doc_lazy_continuation: blank comment line for gap

This change addresses cases where doc comments are separated by blank lines, comments, or non-doc-comment attributes, like this: ```rust /// - first line // not part of doc comment /// second line ``` Before this commit, Clippy gave a pedantically-correct warning about how you needed to indent the second line. This is unlikely to be what the user intends, and has been described as a "false positive" (since Clippy is warning you about a highly unintuitive behavior that Rustdoc actually has, we definitely want it to output *something*, but the suggestion to indent was poor). #12917
rust-lang · Jun 28, 2024 · 6de8782 · 6de8782
1 parent 68a799a
commit 6de8782
Show file tree

Hide file tree

Showing 11 changed files with 246 additions and 68 deletions.
diff --git a/clippy_lints/src/doc/lazy_continuation.rs b/clippy_lints/src/doc/lazy_continuation.rs
@@ -22,6 +22,7 @@ pub(super) fn check(
     range: Range<usize>,
     mut span: Span,
     containers: &[super::Container],
+    line_break_span: Span,
 ) {
     if doc[range.clone()].contains('\t') {
         // We don't do tab stops correctly.
@@ -46,11 +47,35 @@ pub(super) fn check(
         .sum();
     if ccount < blockquote_level || lcount < list_indentation {
         let msg = if ccount < blockquote_level {
-            "doc quote missing `>` marker"
+            "doc quote line without `>` marker"
         } else {
-            "doc list item missing indentation"
+            "doc list item without indentation"
         };
         span_lint_and_then(cx, DOC_LAZY_CONTINUATION, span, msg, |diag| {
+            let snippet = clippy_utils::source::snippet(cx, line_break_span, "");
+            if snippet.chars().filter(|&c| c == '\n').count() > 1
+                && let Some(doc_comment_start) = snippet.rfind('\n')
+                && let doc_comment = snippet[doc_comment_start..].trim()
+                && (doc_comment == "///" || doc_comment == "//!")
+            {
+                // suggest filling in a blank line
+                diag.span_suggestion_with_style(
+                    line_break_span.shrink_to_lo(),
+                    "if this should be its own paragraph, add a blank doc comment line",
+                    format!("\n{doc_comment}"),
+                    Applicability::MaybeIncorrect,
+                    SuggestionStyle::ShowAlways,
+                );
+                if ccount > 0 || blockquote_level > 0 {
+                    diag.help("if this not intended to be a quote at all, escape it with `\\>`");
+                } else {
+                    let indent = list_indentation - lcount;
+                    diag.help(format!(
+                        "if this is intended to be part of the list, indent {indent} spaces"
+                    ));
+                }
+                return;
+            }
             if ccount == 0 && blockquote_level == 0 {
                 // simpler suggestion style for indentation
                 let indent = list_indentation - lcount;

diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs
@@ -762,6 +762,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                         range.end..next_range.start,
                         Span::new(span.hi(), next_span.lo(), span.ctxt(), span.parent()),
                         &containers[..],
+                        span,
                     );
                 }
             },

diff --git a/tests/ui/doc/doc_lazy_blank_line.fixed b/tests/ui/doc/doc_lazy_blank_line.fixed
@@ -0,0 +1,47 @@
+// https://github.com/rust-lang/rust-clippy/issues/12917
+#![warn(clippy::doc_lazy_continuation)]
+
+/// This is a constant.
+///
+/// The meaning of which should not be explained.
+pub const A: i32 = 42;
+
+/// This is another constant, no longer used.
+///
+/// This block of documentation has a long
+/// explanation and derivation to explain
+/// why it is what it is, and how it's used.
+///
+/// It is left here for historical reasons, and
+/// for reference.
+///
+/// Reasons it's great:
+///  - First reason
+///  - Second reason
+///
+//pub const B: i32 = 1337;
+
+/// This is yet another constant.
+///
+/// This has a similar fate as `B`.
+///
+/// Reasons it's useful:
+///  1. First reason
+///  2. Second reason
+///
+//pub const C: i32 = 8008;
+
+/// This is still in use.
+pub const D: i32 = 20;
+
+/// > blockquote code path
+///
+
+/// bottom text
+pub const E: i32 = 20;
+
+/// > blockquote code path
+///
+#[repr(C)]
+/// bottom text
+pub struct Foo(i32);
diff --git a/tests/ui/doc/doc_lazy_blank_line.rs b/tests/ui/doc/doc_lazy_blank_line.rs
@@ -0,0 +1,43 @@
+// https://github.com/rust-lang/rust-clippy/issues/12917
+#![warn(clippy::doc_lazy_continuation)]
+
+/// This is a constant.
+///
+/// The meaning of which should not be explained.
+pub const A: i32 = 42;
+
+/// This is another constant, no longer used.
+///
+/// This block of documentation has a long
+/// explanation and derivation to explain
+/// why it is what it is, and how it's used.
+///
+/// It is left here for historical reasons, and
+/// for reference.
+///
+/// Reasons it's great:
+///  - First reason
+///  - Second reason
+//pub const B: i32 = 1337;
+
+/// This is yet another constant.
+///
+/// This has a similar fate as `B`.
+///
+/// Reasons it's useful:
+///  1. First reason
+///  2. Second reason
+//pub const C: i32 = 8008;
+
+/// This is still in use.
+pub const D: i32 = 20;
+
+/// > blockquote code path
+
+/// bottom text
+pub const E: i32 = 20;
+
+/// > blockquote code path
+#[repr(C)]
+/// bottom text
+pub struct Foo(i32);
diff --git a/tests/ui/doc/doc_lazy_blank_line.stderr b/tests/ui/doc/doc_lazy_blank_line.stderr
@@ -0,0 +1,56 @@
+error: doc list item without indentation
+  --> tests/ui/doc/doc_lazy_blank_line.rs:23:5
+   |
+LL | /// This is yet another constant.
+   |     ^
+   |
+   = help: if this is intended to be part of the list, indent 3 spaces
+   = note: `-D clippy::doc-lazy-continuation` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::doc_lazy_continuation)]`
+help: if this should be its own paragraph, add a blank doc comment line
+   |
+LL ~ ///  - Second reason
+LL + ///
+   |
+
+error: doc list item without indentation
+  --> tests/ui/doc/doc_lazy_blank_line.rs:32:5
+   |
+LL | /// This is still in use.
+   |     ^
+   |
+   = help: if this is intended to be part of the list, indent 4 spaces
+help: if this should be its own paragraph, add a blank doc comment line
+   |
+LL ~ ///  2. Second reason
+LL + ///
+   |
+
+error: doc quote line without `>` marker
+  --> tests/ui/doc/doc_lazy_blank_line.rs:37:5
+   |
+LL | /// bottom text
+   |     ^
+   |
+   = help: if this not intended to be a quote at all, escape it with `\>`
+help: if this should be its own paragraph, add a blank doc comment line
+   |
+LL ~ /// > blockquote code path
+LL + ///
+   |
+
+error: doc quote line without `>` marker
+  --> tests/ui/doc/doc_lazy_blank_line.rs:42:5
+   |
+LL | /// bottom text
+   |     ^
+   |
+   = help: if this not intended to be a quote at all, escape it with `\>`
+help: if this should be its own paragraph, add a blank doc comment line
+   |
+LL ~ /// > blockquote code path
+LL + ///
+   |
+
+error: aborting due to 4 previous errors
+
diff --git a/tests/ui/doc/doc_lazy_blockquote.fixed b/tests/ui/doc/doc_lazy_blockquote.fixed
@@ -2,7 +2,7 @@
 
 /// > blockquote with
 /// > lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn first() {}
 
 /// > blockquote with no
@@ -18,30 +18,30 @@ fn two_nowarn() {}
 /// >
 /// > > nest here
 /// > > lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn two() {}
 
 /// > nest here
 /// >
 /// > > nest here
 /// > > lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn three() {}
 
 /// >   * > nest here
 /// >     > lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn four() {}
 
 /// > * > nest here
 /// >   > lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn four_point_1() {}
 
 /// * > nest here lazy continuation
 fn five() {}
 
 /// 1. > nest here
 ///    > lazy continuation (this results in strange indentation, but still works)
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn six() {}
diff --git a/tests/ui/doc/doc_lazy_blockquote.rs b/tests/ui/doc/doc_lazy_blockquote.rs
@@ -2,7 +2,7 @@
 
 /// > blockquote with
 /// lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn first() {}
 
 /// > blockquote with no
@@ -18,30 +18,30 @@ fn two_nowarn() {}
 /// >
 /// > > nest here
 /// > lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn two() {}
 
 /// > nest here
 /// >
 /// > > nest here
 /// lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn three() {}
 
 /// >   * > nest here
 /// lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn four() {}
 
 /// > * > nest here
 /// lazy continuation
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn four_point_1() {}
 
 /// * > nest here lazy continuation
 fn five() {}
 
 /// 1. > nest here
 ///  lazy continuation (this results in strange indentation, but still works)
-//~^ ERROR: doc quote missing `>` marker
+//~^ ERROR: doc quote line without `>` marker
 fn six() {}
diff --git a/tests/ui/doc/doc_lazy_blockquote.stderr b/tests/ui/doc/doc_lazy_blockquote.stderr
@@ -1,4 +1,4 @@
-error: doc quote missing `>` marker
+error: doc quote line without `>` marker
   --> tests/ui/doc/doc_lazy_blockquote.rs:4:5
    |
 LL | /// lazy continuation
@@ -12,7 +12,7 @@ help: add markers to start of line
 LL | /// > lazy continuation
    |     +
 
-error: doc quote missing `>` marker
+error: doc quote line without `>` marker
   --> tests/ui/doc/doc_lazy_blockquote.rs:20:5
    |
 LL | /// > lazy continuation
@@ -24,7 +24,7 @@ help: add markers to start of line
 LL | /// > > lazy continuation
    |       +
 
-error: doc quote missing `>` marker
+error: doc quote line without `>` marker
   --> tests/ui/doc/doc_lazy_blockquote.rs:27:5
    |
 LL | /// lazy continuation
@@ -36,7 +36,7 @@ help: add markers to start of line
 LL | /// > > lazy continuation
    |     +++
 
-error: doc quote missing `>` marker
+error: doc quote line without `>` marker
   --> tests/ui/doc/doc_lazy_blockquote.rs:32:5
    |
 LL | /// lazy continuation
@@ -48,7 +48,7 @@ help: add markers to start of line
 LL | /// >     > lazy continuation
    |     +++++++
 
-error: doc quote missing `>` marker
+error: doc quote line without `>` marker
   --> tests/ui/doc/doc_lazy_blockquote.rs:37:5
    |
 LL | /// lazy continuation
@@ -60,7 +60,7 @@ help: add markers to start of line
 LL | /// >   > lazy continuation
    |     +++++
 
-error: doc quote missing `>` marker
+error: doc quote line without `>` marker
   --> tests/ui/doc/doc_lazy_blockquote.rs:45:5
    |
 LL | ///  lazy continuation (this results in strange indentation, but still works)