Add lint for markdown lazy paragraph continuations

notriddle · notriddle · commit 0617c0d4eb65 · 2024-05-06T15:53:16.000-07:00
This is a follow-up for rust-lang/rust#121659, since most cases of unintended block quotes are lazy continuations. The lint is designed to be more generally useful than that, though, because it will also catch unintended list items and unintended block quotes that didn't coincidentally hit a pulldown-cmark bug.
diff --git a/clippy_lints/src/doc/lazy_continuation.rs b/clippy_lints/src/doc/lazy_continuation.rs
@@ -16,7 +16,13 @@ fn map_container_to_text(c: &super::Container) -> &'static str {
 }
 
 // TODO: Adjust the parameters as necessary
-pub(super) fn check(cx: &LateContext<'_>, doc: &str, range: Range<usize>, mut span: Span, containers: &[super::Container]) {
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    doc: &str,
+    range: Range<usize>,
+    mut span: Span,
+    containers: &[super::Container],
+) {
     if doc[range.clone()].contains('\t') {
         // We don't do tab stops correctly.
         return;
@@ -30,10 +36,20 @@ pub(super) fn check(cx: &LateContext<'_>, doc: &str, range: Range<usize>, mut sp
     let lcount = doc[range.clone()].chars().filter(|c| *c == ' ').count();
     let list_indentation = containers
         .iter()
-        .map(|c| if let super::Container::List(indent) = c { *indent } else { 0 })
+        .map(|c| {
+            if let super::Container::List(indent) = c {
+                *indent
+            } else {
+                0
+            }
+        })
         .sum();
     if ccount < blockquote_level || lcount < list_indentation {
-        let msg = if ccount < blockquote_level { "doc quote missing `>` marker" } else { "doc list item missing indentation" };
+        let msg = if ccount < blockquote_level {
+            "doc quote missing `>` marker"
+        } else {
+            "doc list item missing indentation"
+        };
         span_lint_and_then(cx, DOC_LAZY_CONTINUATION, span, msg, |diag| {
             if ccount == 0 && blockquote_level == 0 {
                 // simpler suggestion style for indentation
@@ -54,10 +70,14 @@ pub(super) fn check(cx: &LateContext<'_>, doc: &str, range: Range<usize>, mut sp
                 let text = map_container_to_text(c);
                 if doc_start_range.starts_with(text) {
                     doc_start_range = &doc_start_range[text.len()..];
-                    span = span.with_lo(span.lo() + BytePos(u32::try_from(text.len()).expect("text is not 2**32 or bigger")));
-                } else if matches!(c, super::Container::Blockquote) && let Some(i) = doc_start_range.find('>') {
+                    span = span
+                        .with_lo(span.lo() + BytePos(u32::try_from(text.len()).expect("text is not 2**32 or bigger")));
+                } else if matches!(c, super::Container::Blockquote)
+                    && let Some(i) = doc_start_range.find('>')
+                {
                     doc_start_range = &doc_start_range[i + 1..];
-                    span = span.with_lo(span.lo() + BytePos(u32::try_from(i).expect("text is not 2**32 or bigger") + 1));
+                    span =
+                        span.with_lo(span.lo() + BytePos(u32::try_from(i).expect("text is not 2**32 or bigger") + 1));
                 } else {
                     suggested.push_str(text);
                 }
diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs
@@ -366,20 +366,56 @@ declare_clippy_lint! {
 declare_clippy_lint! {
     /// ### What it does
     ///
+    /// In CommonMark Markdown, the language used to write doc comments, a
+    /// paragraph nested within a list or block quote does not need any line
+    /// after the first one to be indented or marked. The specification calls
+    /// this a "lazy paragraph continuation."
+    ///
     /// ### Why is this bad?
     ///
+    /// This is easy to write but hard to read. Lazy continuations makes
+    /// unintended markers hard to see, and make it harder to deduce the
+    /// document's intended structure.
+    ///
     /// ### Example
+    ///
+    /// This table is probably intended to have two rows,
+    /// but it does not. It has zero rows, and is followed by
+    /// a block quote.
     /// ```no_run
-    /// // example code where clippy issues a warning
+    /// /// Range | Description
+    /// /// ----- | -----------
+    /// /// >= 1  | fully opaque
+    /// /// < 1   | partially see-through
+    /// fn set_opacity(opacity: f32) {}
     /// ```
-    /// Use instead:
+    ///
+    /// Fix it by escaping the marker:
+    /// ```no_run
+    /// /// Range | Description
+    /// /// ----- | -----------
+    /// /// \>= 1 | fully opaque
+    /// /// < 1   | partially see-through
+    /// fn set_opacity(opacity: f32) {}
+    /// ```
+    ///
+    /// This example is actually intended to be a list:
+    /// ```no_run
+    /// /// * Do nothing.
+    /// /// * Then do something. Whatever it is needs done,
+    /// /// it should be done right now.
+    /// ```
+    ///
+    /// Fix it by indenting the list contents:
     /// ```no_run
-    /// // example code which does not raise clippy warning
+    /// /// * Do nothing.
+    /// /// * Then do something. Whatever it is needs done,
+    /// ///   it should be done right now.
     /// ```
     #[clippy::version = "1.80.0"]
     pub DOC_LAZY_CONTINUATION,
     style,
-    "default lint description"
+    "require every line of a paragraph to be indented and marked"
 }
 
 #[derive(Clone)]
@@ -711,20 +747,19 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
             End(FootnoteDefinition(..)) => in_footnote_definition = false,
             Start(_tag) | End(_tag) => (), // We don't care about other tags
             SoftBreak | HardBreak => {
-                if !containers.is_empty() {
-                    if let Some((_next_event, next_range)) = events.peek()
-                        && let Some(next_span) = fragments.span(cx, next_range.clone())
-                        && let Some(span) = fragments.span(cx, range.clone())
-                        && !in_footnote_definition // not implemented
-                    {
-                        lazy_continuation::check(
-                            cx,
-                            doc,
-                            range.end..next_range.start,
-                            Span::new(span.hi(), next_span.lo(), span.ctxt(), span.parent()),
-                            &containers[..],
-                        );
-                    }
+                if !containers.is_empty()
+                    && let Some((_next_event, next_range)) = events.peek()
+                    && let Some(next_span) = fragments.span(cx, next_range.clone())
+                    && let Some(span) = fragments.span(cx, range.clone())
+                    && !in_footnote_definition
+                {
+                    lazy_continuation::check(
+                        cx,
+                        doc,
+                        range.end..next_range.start,
+                        Span::new(span.hi(), next_span.lo(), span.ctxt(), span.parent()),
+                        &containers[..],
+                    );
                 }
             },
             TaskListMarker(_) | Code(_) | Rule => (),
diff --git a/tests/ui/doc/doc_lazy_blockquote.fixed b/tests/ui/doc/doc_lazy_blockquote.fixed
@@ -38,9 +38,7 @@ fn four() {}
 //~^ ERROR: doc quote missing `>` marker
 fn four_point_1() {}
 
-/// * > nest here
-///   > lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+/// * > nest here lazy continuation
 fn five() {}
 
 /// 1. > nest here
diff --git a/tests/ui/doc/doc_lazy_blockquote.rs b/tests/ui/doc/doc_lazy_blockquote.rs
@@ -38,9 +38,7 @@ fn four() {}
 //~^ ERROR: doc quote missing `>` marker
 fn four_point_1() {}
 
-/// * > nest here
-///   lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+/// * > nest here lazy continuation
 fn five() {}
 
 /// 1. > nest here
diff --git a/tests/ui/doc/doc_lazy_blockquote.stderr b/tests/ui/doc/doc_lazy_blockquote.stderr
@@ -61,19 +61,7 @@ LL | /// >   > lazy continuation
    |     +++++
 
 error: doc quote missing `>` marker
-  --> tests/ui/doc/doc_lazy_blockquote.rs:42:5
-   |
-LL | ///   lazy continuation
-   |     ^^
-   |
-   = help: if this not intended to be a quote at all, escape it with `\>`
-help: add markers to start of line
-   |
-LL | ///   > lazy continuation
-   |       +
-
-error: doc quote missing `>` marker
-  --> tests/ui/doc/doc_lazy_blockquote.rs:47:5
+  --> tests/ui/doc/doc_lazy_blockquote.rs:45:5
    |
 LL | ///  lazy continuation (this results in strange indentation, but still works)
    |     ^
@@ -84,5 +72,5 @@ help: add markers to start of line
 LL | ///    > lazy continuation (this results in strange indentation, but still works)
    |        +
 
-error: aborting due to 7 previous errors
+error: aborting due to 6 previous errors
 
diff --git a/tests/ui/doc/doc_lazy_list.fixed b/tests/ui/doc/doc_lazy_list.fixed
@@ -8,7 +8,7 @@ fn one() {}
 /// 1. first line
 ///    lazy list continuations don't make warnings with this lint
 //~^ ERROR: doc list item missing indentation
-///    because they don't have the 
+///    because they don't have the
 //~^ ERROR: doc list item missing indentation
 fn two() {}
 
@@ -20,7 +20,7 @@ fn three() {}
 ///   - first line
 ///     lazy list continuations don't make warnings with this lint
 //~^ ERROR: doc list item missing indentation
-///     because they don't have the 
+///     because they don't have the
 //~^ ERROR: doc list item missing indentation
 fn four() {}
 
@@ -29,14 +29,14 @@ fn four() {}
 //~^ ERROR: doc list item missing indentation
 fn five() {}
 
-///   -   - first line
-///         this will warn on the lazy continuation
+///   - - first line
+///       this will warn on the lazy continuation
 //~^ ERROR: doc list item missing indentation
-///         and so should this
+///       and so should this
 //~^ ERROR: doc list item missing indentation
 fn six() {}
 
-///   -   - first line
+///   - - first line
 ///
 ///     this is not a lazy continuation
 fn seven() {}
diff --git a/tests/ui/doc/doc_lazy_list.rs b/tests/ui/doc/doc_lazy_list.rs
@@ -8,7 +8,7 @@ fn one() {}
 /// 1. first line
 /// lazy list continuations don't make warnings with this lint
 //~^ ERROR: doc list item missing indentation
-/// because they don't have the 
+/// because they don't have the
 //~^ ERROR: doc list item missing indentation
 fn two() {}
 
@@ -20,7 +20,7 @@ fn three() {}
 ///   - first line
 /// lazy list continuations don't make warnings with this lint
 //~^ ERROR: doc list item missing indentation
-/// because they don't have the 
+/// because they don't have the
 //~^ ERROR: doc list item missing indentation
 fn four() {}
 
@@ -29,14 +29,14 @@ fn four() {}
 //~^ ERROR: doc list item missing indentation
 fn five() {}
 
-///   -   - first line
+///   - - first line
 /// this will warn on the lazy continuation
 //~^ ERROR: doc list item missing indentation
 ///     and so should this
 //~^ ERROR: doc list item missing indentation
 fn six() {}
 
-///   -   - first line
+///   - - first line
 ///
 ///     this is not a lazy continuation
 fn seven() {}
diff --git a/tests/ui/doc/doc_lazy_list.stderr b/tests/ui/doc/doc_lazy_list.stderr
@@ -27,13 +27,13 @@ LL | ///    lazy list continuations don't make warnings with this lint
 error: doc list item missing indentation
   --> tests/ui/doc/doc_lazy_list.rs:11:5
    |
-LL | /// because they don't have the 
+LL | /// because they don't have the
    |     ^
    |
    = help: if this is supposed to be its own paragraph, add a blank line
 help: indent this line
    |
-LL | ///    because they don't have the 
+LL | ///    because they don't have the
    |     +++
 
 error: doc list item missing indentation
@@ -63,13 +63,13 @@ LL | ///     lazy list continuations don't make warnings with this lint
 error: doc list item missing indentation
   --> tests/ui/doc/doc_lazy_list.rs:23:5
    |
-LL | /// because they don't have the 
+LL | /// because they don't have the
    |     ^
    |
    = help: if this is supposed to be its own paragraph, add a blank line
 help: indent this line
    |
-LL | ///     because they don't have the 
+LL | ///     because they don't have the
    |     ++++
 
 error: doc list item missing indentation
@@ -93,8 +93,8 @@ LL | /// this will warn on the lazy continuation
    = help: if this is supposed to be its own paragraph, add a blank line
 help: indent this line
    |
-LL | ///         this will warn on the lazy continuation
-   |     ++++++++
+LL | ///       this will warn on the lazy continuation
+   |     ++++++
 
 error: doc list item missing indentation
   --> tests/ui/doc/doc_lazy_list.rs:35:5
@@ -105,8 +105,8 @@ LL | ///     and so should this
    = help: if this is supposed to be its own paragraph, add a blank line
 help: indent this line
    |
-LL | ///         and so should this
-   |         ++++
+LL | ///       and so should this
+   |         ++
 
 error: aborting due to 9 previous errors
 
