Skip to content

Commit

Permalink
check for unbalanced tick pairs in doc-markdown
Browse files Browse the repository at this point in the history
  • Loading branch information
@ebobrow
ebobrow committed Jun 15, 2021
1 parent f1f5ccd commit d194de2
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 67 deletions.
190 changes: 124 additions & 66 deletions clippy_lints/src/doc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,10 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
let mut in_heading = false;
let mut is_rust = false;
let mut edition = None;
let mut ticks_inverted = false;
let mut prev_was_ticks: Option<Span> = None;
for (event, range) in events {
let in_ticks = matches!(event, Code(_));
match event {
Start(CodeBlock(ref kind)) => {
in_code = true;
Expand All @@ -503,43 +506,71 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
}
}
}
prev_was_ticks = None;
},
End(CodeBlock(_)) => {
in_code = false;
is_rust = false;
prev_was_ticks = None;
},
Start(Link(_, url, _)) => in_link = Some(url),
End(Link(..)) => in_link = None,
Start(Heading(_)) => in_heading = true,
End(Heading(_)) => in_heading = false,
Start(_tag) | End(_tag) => (), // We don't care about other tags
Html(_html) => (), // HTML is weird, just ignore it
SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (),
FootnoteReference(text) | Text(text) => {
if Some(&text) == in_link.as_ref() {
// Probably a link of the form `<http://example.com>`
// Which are represented as a link to "http://example.com" with
// text "http://example.com" by pulldown-cmark
continue;
}
headers.safety |= in_heading && text.trim() == "Safety";
headers.errors |= in_heading && text.trim() == "Errors";
headers.panics |= in_heading && text.trim() == "Panics";
Start(Link(_, url, _)) => {
prev_was_ticks = None;
in_link = Some(url);
},
End(Link(..)) => {
prev_was_ticks = None;
in_link = None;
},
Start(Heading(_)) => {
prev_was_ticks = None;
in_heading = true;
},
End(Heading(_)) => {
prev_was_ticks = None;
in_heading = false;
},
Start(_tag) | End(_tag) => {
// We don't care about other tags
prev_was_ticks = None;
},
Html(_html) => {
// HTML is weird, just ignore it
prev_was_ticks = None;
},
SoftBreak | HardBreak | TaskListMarker(_) | Rule => {
prev_was_ticks = None;
},
FootnoteReference(text) | Text(text) | Code(text) => {
let index = match spans.binary_search_by(|c| c.0.cmp(&range.start)) {
Ok(o) => o,
Err(e) => e - 1,
};
let (begin, span) = spans[index];
if in_code {
if is_rust {
let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition());
check_code(cx, &text, edition, span);
}
} else {
// Adjust for the beginning of the current `Event`
if ticks_inverted && !in_ticks || !ticks_inverted && in_ticks {
let span = span.with_lo(span.lo() + BytePos::from_usize(range.start - begin));

check_text(cx, valid_idents, &text, span);
let span = span.with_hi(span.lo() + BytePos::from_usize(1));
prev_was_ticks = Some(span);
} else {
if Some(&text) == in_link.as_ref() {
// Probably a link of the form `<http://example.com>`
// Which are represented as a link to "http://example.com" with
// text "http://example.com" by pulldown-cmark
continue;
}
headers.safety |= in_heading && text.trim() == "Safety";
headers.errors |= in_heading && text.trim() == "Errors";
headers.panics |= in_heading && text.trim() == "Panics";
if in_code {
if is_rust {
let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition());
check_code(cx, &text, edition, span);
}
} else {
// Adjust for the beginning of the current `Event`
let span = span.with_lo(span.lo() + BytePos::from_usize(range.start - begin));
check_text(cx, valid_idents, &text, span, &prev_was_ticks, &mut ticks_inverted);
}
prev_was_ticks = None;
}
},
}
Expand Down Expand Up @@ -618,30 +649,78 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) {
}
}

fn check_text(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, text: &str, span: Span) {
for word in text.split(|c: char| c.is_whitespace() || c == '\'') {
// Trim punctuation as in `some comment (see foo::bar).`
// ^^
// Or even as in `_foo bar_` which is emphasized.
let word = word.trim_matches(|c: char| !c.is_alphanumeric());
fn check_text(
cx: &LateContext<'_>,
valid_idents: &FxHashSet<String>,
text: &str,
span: Span,
prev_was_ticks: &Option<Span>,
ticks_inverted: &mut bool,
) {
if_chain! {
if let Some(prev_span) = prev_was_ticks;
let words = text.split(char::is_whitespace).collect_vec();
if words.len() == 1;
if should_have_backticks(text);
then {
span_lint(
cx,
DOC_MARKDOWN,
*prev_span,
"backtick may be missing a pair",
);
*ticks_inverted = !*ticks_inverted;
} else {
for word in text.split(|c: char| c.is_whitespace() || c == '\'') {
// Trim punctuation as in `some comment (see foo::bar).`
// ^^
// Or even as in `_foo bar_` which is emphasized.
let word = word.trim_matches(|c: char| !c.is_alphanumeric());

if valid_idents.contains(word) {
continue;
}
if valid_idents.contains(word) {
continue;
}

// Adjust for the current word
let offset = word.as_ptr() as usize - text.as_ptr() as usize;
let span = Span::new(
span.lo() + BytePos::from_usize(offset),
span.lo() + BytePos::from_usize(offset + word.len()),
span.ctxt(),
);
// Adjust for the current word
let offset = word.as_ptr() as usize - text.as_ptr() as usize;
let span = Span::new(
span.lo() + BytePos::from_usize(offset),
span.lo() + BytePos::from_usize(offset + word.len()),
span.ctxt(),
);

check_word(cx, word, span);
check_word(cx, word, span);
}
}
}
}

fn check_word(cx: &LateContext<'_>, word: &str, span: Span) {
if let Ok(url) = Url::parse(word) {
// try to get around the fact that `foo::bar` parses as a valid URL
if !url.cannot_be_a_base() {
span_lint(
cx,
DOC_MARKDOWN,
span,
"you should put bare URLs between `<`/`>` or make a proper Markdown link",
);

return;
}
}

if should_have_backticks(word) {
span_lint(
cx,
DOC_MARKDOWN,
span,
&format!("you should put `{}` between ticks in the documentation", word),
);
}
}

fn should_have_backticks(word: &str) -> bool {
/// Checks if a string is camel-case, i.e., contains at least two uppercase
/// letters (`Clippy` is ok) and one lower-case letter (`NASA` is ok).
/// Plurals are also excluded (`IDs` is ok).
Expand All @@ -665,33 +744,12 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span) {
s != "-" && s.contains('-')
}

if let Ok(url) = Url::parse(word) {
// try to get around the fact that `foo::bar` parses as a valid URL
if !url.cannot_be_a_base() {
span_lint(
cx,
DOC_MARKDOWN,
span,
"you should put bare URLs between `<`/`>` or make a proper Markdown link",
);

return;
}
}

// We assume that mixed-case words are not meant to be put inside bacticks. (Issue #2343)
if has_underscore(word) && has_hyphen(word) {
return;
return false;
}

if has_underscore(word) || word.contains("::") || is_camel_case(word) {
span_lint(
cx,
DOC_MARKDOWN,
span,
&format!("you should put `{}` between ticks in the documentation", word),
);
}
has_underscore(word) || word.contains("::") || is_camel_case(word)
}

struct FindPanicUnwrap<'a, 'tcx> {
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/doc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,12 @@ fn issue_2343() {}
/// __|_ _|__||_|
fn pulldown_cmark_crash() {}

/// This is a doc comment with `unbalanced_tick marks and several words that
/// should be `encompassed_by` tick marks because they `contain_underscores`.
/// Because of the initial unbalanced_tick pair, the error message is
/// very `confusing_and_misleading`.
pub fn issue_6753() {}

// issue #7033 - const_evaluatable_checked ICE
struct S<T, const N: usize>
where [(); N.checked_next_power_of_two().unwrap()]: {
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/doc.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,5 +186,11 @@ error: you should put `mycrate::Collection` between ticks in the documentation
LL | /// An iterator over mycrate::Collection's values.
| ^^^^^^^^^^^^^^^^^^^

error: aborting due to 31 previous errors
error: backtick may be missing a pair
--> $DIR/doc.rs:206:32
|
LL | /// This is a doc comment with `unbalanced_tick marks and several words that
| ^

error: aborting due to 32 previous errors

0 comments on commit d194de2

Please sign in to comment.