check for unbalanced tick pairs in doc-markdown

Author: ebobrow

clippy_lints/src/doc.rs

@@ -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;
@@ -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
@@ -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,
@@ -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)) => {
@@ -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());
@@ -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(|| {

clippy_lints/src/if_let_mutex.rs

@@ -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>>,

tests/ui/doc.rs

@@ -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() {
@@ -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
@@ -203,6 +200,20 @@ 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()]: {

tests/ui/doc.stderr

@@ -85,106 +85,112 @@ 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 throw an error.
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: you should put `should_be` between ticks in the documentation
+  --> $DIR/doc.rs:208:32
+   |
+LL | /// This paragraph is fine and should_be linted normally.
+   |                                ^^^^^^^^^
+
+error: aborting due to 32 previous errors