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 committed Jun 18, 2021
1 parent 6bf8772 commit c604688
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 43 deletions.
54 changes: 43 additions & 11 deletions clippy_lints/src/doc.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_note};
use clippy_utils::source::first_line_of_span;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty};
use if_chain::if_chain;
Expand Down Expand Up @@ -37,7 +38,7 @@ declare_clippy_lint! {
/// consider that.
///
/// **Known problems:** Lots of bad docs won’t be fixed, what the lint checks
/// for is limited, and there are still false positives.
/// for is limited, and there are still false positives. HTML is not linted.
///
/// In addition, when writing documentation comments, including `[]` brackets
/// inside a link text would trip the parser. Therfore, documenting link with
Expand Down Expand Up @@ -469,11 +470,11 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
spans: &[(usize, Span)],
) -> DocHeaders {
// true if a safety header was found
use pulldown_cmark::CodeBlockKind;
use pulldown_cmark::Event::{
Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text,
};
use pulldown_cmark::Tag::{CodeBlock, Heading, Link};
use pulldown_cmark::Tag::{CodeBlock, Heading, Link, Paragraph};
use pulldown_cmark::{CodeBlockKind, CowStr};

let mut headers = DocHeaders {
safety: false,
Expand All @@ -485,6 +486,9 @@ 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_unbalanced = false;
let mut text_to_check: Vec<(CowStr<'_>, Span)> = Vec::new();
let mut paragraph_span = spans[0].1; // First line
for (event, range) in events {
match event {
Start(CodeBlock(ref kind)) => {
Expand Down Expand Up @@ -512,24 +516,45 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
End(Link(..)) => in_link = None,
Start(Heading(_)) => in_heading = true,
End(Heading(_)) => in_heading = false,
Start(Paragraph) => {
ticks_unbalanced = false;
let (_, span) = get_current_span(spans, range.start);
paragraph_span = first_line_of_span(cx, span);
},
End(Paragraph) => {
if ticks_unbalanced {
span_lint(
cx,
DOC_MARKDOWN,
paragraph_span,
"backticks are unbalanced; one may be missing a pair",
);
} else {
for (text, span) in text_to_check {
check_text(cx, valid_idents, &text, span);
}
}
text_to_check = Vec::new();
},
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() {
let (begin, span) = get_current_span(spans, range.start);
paragraph_span = paragraph_span.with_hi(span.hi());
if Some(&text) == in_link.as_ref() || ticks_unbalanced {
// 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;
}
if text.contains('`') && !in_code {
ticks_unbalanced = true;
continue;
}
headers.safety |= in_heading && text.trim() == "Safety";
headers.errors |= in_heading && text.trim() == "Errors";
headers.panics |= in_heading && text.trim() == "Panics";
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());
Expand All @@ -538,15 +563,22 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
} 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);
text_to_check.push((text, span));
}
},
}
}
headers
}

fn get_current_span(spans: &[(usize, Span)], idx: usize) -> (usize, Span) {
let index = match spans.binary_search_by(|c| c.0.cmp(&idx)) {
Ok(o) => o,
Err(e) => e - 1,
};
spans[index]
}

fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) {
fn has_needless_main(code: &str, edition: Edition) -> bool {
rustc_driver::catch_fatal_errors(|| {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/if_let_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
}
}

/// Checks if `Mutex::lock` is called in the `if let _ = expr.
/// Checks if `Mutex::lock` is called in the `if let` expr.
pub struct OppVisitor<'a, 'tcx> {
mutex_lock_called: bool,
found_mutex: Option<&'tcx Expr<'tcx>>,
Expand Down
18 changes: 15 additions & 3 deletions tests/ui/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ fn main() {
/// ## CamelCaseThing
/// Talks about `CamelCaseThing`. Titles should be ignored; see issue #897.
///
/// # CamelCaseThing
///
/// Not a title #897 CamelCaseThing
/// be_sure_we_got_to_the_end_of_it
fn issue897() {
Expand Down Expand Up @@ -185,7 +183,6 @@ fn issue_1920() {}

/// Ok: <http://www.unicode.org/reports/tr9/#Reordering_Resolved_Levels>
///
/// Not ok: http://www.unicode.org
/// Not ok: https://www.unicode.org
/// Not ok: http://www.unicode.org/
/// Not ok: http://www.unicode.org/reports/tr9/#Reordering_Resolved_Levels
Expand All @@ -203,6 +200,21 @@ fn issue_2343() {}
/// __|_ _|__||_|
fn pulldown_cmark_crash() {}

/// This has `unbalanced_tick marks so it should_not_lint elsewhere.
fn issue_6753() {}

/// This paragraph has `unbalanced_tick marks and should
/// throw an error.
///
/// This paragraph is fine and should_be linted normally.
fn unbalanced_ticks() {}

/// ```
/// // Unbalanced tick mark in code block shouldn't warn:
/// `
/// ```
fn unbalanced_ticks_in_code() {}

// issue #7033 - const_evaluatable_checked ICE
struct S<T, const N: usize>
where [(); N.checked_next_power_of_two().unwrap()]: {
Expand Down
63 changes: 35 additions & 28 deletions tests/ui/doc.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -85,106 +85,113 @@ LL | /// ## CamelCaseThing
| ^^^^^^^^^^^^^^

error: you should put `CamelCaseThing` between ticks in the documentation
--> $DIR/doc.rs:99:7
|
LL | /// # CamelCaseThing
| ^^^^^^^^^^^^^^

error: you should put `CamelCaseThing` between ticks in the documentation
--> $DIR/doc.rs:101:22
--> $DIR/doc.rs:99:22
|
LL | /// Not a title #897 CamelCaseThing
| ^^^^^^^^^^^^^^

error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation
--> $DIR/doc.rs:102:5
--> $DIR/doc.rs:100:5
|
LL | /// be_sure_we_got_to_the_end_of_it
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation
--> $DIR/doc.rs:109:5
--> $DIR/doc.rs:107:5
|
LL | /// be_sure_we_got_to_the_end_of_it
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation
--> $DIR/doc.rs:122:5
--> $DIR/doc.rs:120:5
|
LL | /// be_sure_we_got_to_the_end_of_it
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you should put `FooBar` between ticks in the documentation
--> $DIR/doc.rs:133:43
--> $DIR/doc.rs:131:43
|
LL | /** E.g., serialization of an empty list: FooBar
| ^^^^^^

error: you should put `BarQuz` between ticks in the documentation
--> $DIR/doc.rs:138:5
--> $DIR/doc.rs:136:5
|
LL | And BarQuz too.
| ^^^^^^

error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation
--> $DIR/doc.rs:139:1
--> $DIR/doc.rs:137:1
|
LL | be_sure_we_got_to_the_end_of_it
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you should put `FooBar` between ticks in the documentation
--> $DIR/doc.rs:144:43
--> $DIR/doc.rs:142:43
|
LL | /** E.g., serialization of an empty list: FooBar
| ^^^^^^

error: you should put `BarQuz` between ticks in the documentation
--> $DIR/doc.rs:149:5
--> $DIR/doc.rs:147:5
|
LL | And BarQuz too.
| ^^^^^^

error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation
--> $DIR/doc.rs:150:1
--> $DIR/doc.rs:148:1
|
LL | be_sure_we_got_to_the_end_of_it
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation
--> $DIR/doc.rs:161:5
--> $DIR/doc.rs:159:5
|
LL | /// be_sure_we_got_to_the_end_of_it
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you should put bare URLs between `<`/`>` or make a proper Markdown link
--> $DIR/doc.rs:188:13
|
LL | /// Not ok: http://www.unicode.org
| ^^^^^^^^^^^^^^^^^^^^^^

error: you should put bare URLs between `<`/`>` or make a proper Markdown link
--> $DIR/doc.rs:189:13
--> $DIR/doc.rs:186:13
|
LL | /// Not ok: https://www.unicode.org
| ^^^^^^^^^^^^^^^^^^^^^^^

error: you should put bare URLs between `<`/`>` or make a proper Markdown link
--> $DIR/doc.rs:190:13
--> $DIR/doc.rs:187:13
|
LL | /// Not ok: http://www.unicode.org/
| ^^^^^^^^^^^^^^^^^^^^^^

error: you should put bare URLs between `<`/`>` or make a proper Markdown link
--> $DIR/doc.rs:191:13
--> $DIR/doc.rs:188:13
|
LL | /// Not ok: http://www.unicode.org/reports/tr9/#Reordering_Resolved_Levels
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you should put `mycrate::Collection` between ticks in the documentation
--> $DIR/doc.rs:194:22
--> $DIR/doc.rs:191:22
|
LL | /// An iterator over mycrate::Collection's values.
| ^^^^^^^^^^^^^^^^^^^

error: aborting due to 31 previous errors
error: backticks are unbalanced; one may be missing a pair
--> $DIR/doc.rs:203:1
|
LL | /// This has `unbalanced_tick marks so it should_not_lint elsewhere.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: backticks are unbalanced; one may be missing a pair
--> $DIR/doc.rs:206:1
|
LL | / /// This paragraph has `unbalanced_tick marks and should
LL | | /// throw an error.
| |___________________^

error: you should put `should_be` between ticks in the documentation
--> $DIR/doc.rs:209:32
|
LL | /// This paragraph is fine and should_be linted normally.
| ^^^^^^^^^

error: aborting due to 32 previous errors

0 comments on commit c604688

Please sign in to comment.