about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-06-28 16:37:43 +0000
committerbors <bors@rust-lang.org>2024-06-28 16:37:43 +0000
commit2f80536e8353f7fbbfe72725d25a7feada3d2eb3 (patch)
tree68b1c9c8246b04e995fc19ce367312bd74c798fa
parent68a799aea9b65e2444fbecfe32217ce7d5a3604f (diff)
parent6de87829da801c10db378e28dd1797434383e651 (diff)
downloadrust-2f80536e8353f7fbbfe72725d25a7feada3d2eb3.tar.gz
rust-2f80536e8353f7fbbfe72725d25a7feada3d2eb3.zip
Auto merge of #13002 - notriddle:notriddle/blank-line, r=Manishearth
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](https://notriddle.com/rustdoc-html-demo-11/lazy-continuation-bad/test_dingus_2024/constant.D.html), we definitely want it to output *something*, but the suggestion to indent was poor.

Fixes #12917

```
changelog: [`doc_lazy_continuation`]: suggest blank line for likely-unintended lazy continuations
```
-rw-r--r--clippy_lints/src/doc/lazy_continuation.rs29
-rw-r--r--clippy_lints/src/doc/mod.rs1
-rw-r--r--tests/ui/doc/doc_lazy_blank_line.fixed47
-rw-r--r--tests/ui/doc/doc_lazy_blank_line.rs43
-rw-r--r--tests/ui/doc/doc_lazy_blank_line.stderr56
-rw-r--r--tests/ui/doc/doc_lazy_blockquote.fixed12
-rw-r--r--tests/ui/doc/doc_lazy_blockquote.rs12
-rw-r--r--tests/ui/doc/doc_lazy_blockquote.stderr12
-rw-r--r--tests/ui/doc/doc_lazy_list.fixed31
-rw-r--r--tests/ui/doc/doc_lazy_list.rs22
-rw-r--r--tests/ui/doc/doc_lazy_list.stderr49
11 files changed, 246 insertions, 68 deletions
diff --git a/clippy_lints/src/doc/lazy_continuation.rs b/clippy_lints/src/doc/lazy_continuation.rs
index 38bc58a5501..bd1cc46e185 100644
--- 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
index 3d875e7ac2d..5faa00b7c97 100644
--- 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
new file mode 100644
index 00000000000..1aaa26afe7f
--- /dev/null
+++ 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
new file mode 100644
index 00000000000..e1ab8fc8389
--- /dev/null
+++ 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
new file mode 100644
index 00000000000..854906a7474
--- /dev/null
+++ 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
index 9877991f183..9d6e8637608 100644
--- 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,24 +18,24 @@ 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
@@ -43,5 +43,5 @@ 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
index 587b2fdd533..0323a1b44e7 100644
--- 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,24 +18,24 @@ 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
@@ -43,5 +43,5 @@ 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
index 975184a01c3..d3390efdff3 100644
--- 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)
diff --git a/tests/ui/doc/doc_lazy_list.fixed b/tests/ui/doc/doc_lazy_list.fixed
index 409e6b0bc22..ea59ae4c01c 100644
--- a/tests/ui/doc/doc_lazy_list.fixed
+++ b/tests/ui/doc/doc_lazy_list.fixed
@@ -2,38 +2,41 @@
 
 /// 1. nest here
 ///    lazy continuation
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 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
-//~^ ERROR: doc list item missing indentation
+///
+//~^ ERROR: doc list item without indentation
+/// because they don't have the
+//~^ ERROR: doc list item without indentation
 fn two() {}
 
 ///   - nest here
 ///     lazy continuation
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 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
-//~^ ERROR: doc list item missing indentation
+///
+//~^ ERROR: doc list item without indentation
+/// because they don't have the
+//~^ ERROR: doc list item without indentation
 fn four() {}
 
 ///   - nest here
 ///     lazy continuation
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn five() {}
 
 ///   - - 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
+///
+//~^ ERROR: doc list item without indentation
+///     and so should this
+//~^ ERROR: doc list item without indentation
 fn six() {}
 
 ///   - - first line
@@ -54,7 +57,7 @@ fn seven() {}
 /// * `protocol_descriptors`: A Json Representation of the ProtocolDescriptors
 ///     to set up. Example:
 ///   'protocol_descriptors': [
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 ///      {
 ///          'protocol': 25,  # u64 Representation of ProtocolIdentifier::AVDTP
 ///          'params': [
@@ -73,5 +76,5 @@ fn seven() {}
 ///          }]
 ///      }
 ///   ]
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn eight() {}
diff --git a/tests/ui/doc/doc_lazy_list.rs b/tests/ui/doc/doc_lazy_list.rs
index 30ab448a113..3cc18e35780 100644
--- a/tests/ui/doc/doc_lazy_list.rs
+++ b/tests/ui/doc/doc_lazy_list.rs
@@ -2,38 +2,38 @@
 
 /// 1. nest here
 /// lazy continuation
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn one() {}
 
 /// 1. first line
 /// lazy list continuations don't make warnings with this lint
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 /// because they don't have the
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn two() {}
 
 ///   - nest here
 /// lazy continuation
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn three() {}
 
 ///   - first line
 /// lazy list continuations don't make warnings with this lint
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 /// because they don't have the
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn four() {}
 
 ///   - nest here
 /// lazy continuation
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn five() {}
 
 ///   - - first line
 /// this will warn on the lazy continuation
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 ///     and so should this
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn six() {}
 
 ///   - - first line
@@ -54,7 +54,7 @@ fn seven() {}
 /// * `protocol_descriptors`: A Json Representation of the ProtocolDescriptors
 ///     to set up. Example:
 ///  'protocol_descriptors': [
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 ///      {
 ///          'protocol': 25,  # u64 Representation of ProtocolIdentifier::AVDTP
 ///          'params': [
@@ -73,5 +73,5 @@ fn seven() {}
 ///          }]
 ///      }
 ///  ]
-//~^ ERROR: doc list item missing indentation
+//~^ ERROR: doc list item without indentation
 fn eight() {}
diff --git a/tests/ui/doc/doc_lazy_list.stderr b/tests/ui/doc/doc_lazy_list.stderr
index ddfdc49340c..52aa74df894 100644
--- a/tests/ui/doc/doc_lazy_list.stderr
+++ b/tests/ui/doc/doc_lazy_list.stderr
@@ -1,4 +1,4 @@
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:4:5
    |
 LL | /// lazy continuation
@@ -12,7 +12,7 @@ help: indent this line
 LL | ///    lazy continuation
    |     +++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:9:5
    |
 LL | /// lazy list continuations don't make warnings with this lint
@@ -24,19 +24,20 @@ help: indent this line
 LL | ///    lazy list continuations don't make warnings with this lint
    |     +++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:11:5
    |
 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
+   = help: if this is intended to be part of the list, indent 3 spaces
+help: if this should be its own paragraph, add a blank doc comment line
+   |
+LL ~ /// lazy list continuations don't make warnings with this lint
+LL + ///
    |
-LL | ///    because they don't have the
-   |     +++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:16:5
    |
 LL | /// lazy continuation
@@ -48,7 +49,7 @@ help: indent this line
 LL | ///     lazy continuation
    |     ++++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:21:5
    |
 LL | /// lazy list continuations don't make warnings with this lint
@@ -60,19 +61,20 @@ help: indent this line
 LL | ///     lazy list continuations don't make warnings with this lint
    |     ++++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:23:5
    |
 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
+   = 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 ~ /// lazy list continuations don't make warnings with this lint
+LL + ///
    |
-LL | ///     because they don't have the
-   |     ++++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:28:5
    |
 LL | /// lazy continuation
@@ -84,7 +86,7 @@ help: indent this line
 LL | ///     lazy continuation
    |     ++++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:33:5
    |
 LL | /// this will warn on the lazy continuation
@@ -96,19 +98,20 @@ help: indent this line
 LL | ///       this will warn on the lazy continuation
    |     ++++++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:35:5
    |
 LL | ///     and so should this
    |     ^^^^
    |
-   = help: if this is supposed to be its own paragraph, add a blank line
-help: indent this line
+   = help: if this is intended to be part of the list, indent 2 spaces
+help: if this should be its own paragraph, add a blank doc comment line
+   |
+LL ~ /// this will warn on the lazy continuation
+LL + ///
    |
-LL | ///       and so should this
-   |         ++
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:56:5
    |
 LL | ///  'protocol_descriptors': [
@@ -120,7 +123,7 @@ help: indent this line
 LL | ///   'protocol_descriptors': [
    |      +
 
-error: doc list item missing indentation
+error: doc list item without indentation
   --> tests/ui/doc/doc_lazy_list.rs:75:5
    |
 LL | ///  ]