From 6de87829da801c10db378e28dd1797434383e651 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 27 Jun 2024 16:35:43 -0700 Subject: [PATCH] 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). https://github.com/rust-lang/rust-clippy/issues/12917 --- clippy_lints/src/doc/lazy_continuation.rs | 29 +++++++++++- clippy_lints/src/doc/mod.rs | 1 + tests/ui/doc/doc_lazy_blank_line.fixed | 47 +++++++++++++++++++ tests/ui/doc/doc_lazy_blank_line.rs | 43 +++++++++++++++++ tests/ui/doc/doc_lazy_blank_line.stderr | 56 +++++++++++++++++++++++ tests/ui/doc/doc_lazy_blockquote.fixed | 12 ++--- tests/ui/doc/doc_lazy_blockquote.rs | 12 ++--- tests/ui/doc/doc_lazy_blockquote.stderr | 12 ++--- tests/ui/doc/doc_lazy_list.fixed | 31 +++++++------ tests/ui/doc/doc_lazy_list.rs | 22 ++++----- tests/ui/doc/doc_lazy_list.stderr | 49 ++++++++++---------- 11 files changed, 246 insertions(+), 68 deletions(-) create mode 100644 tests/ui/doc/doc_lazy_blank_line.fixed create mode 100644 tests/ui/doc/doc_lazy_blank_line.rs create mode 100644 tests/ui/doc/doc_lazy_blank_line.stderr 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, 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, Range 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 | /// ]