Skip to content

Commit

Permalink
Auto merge of rust-lang#13002 - notriddle:notriddle/blank-line, r=Man…
Browse files Browse the repository at this point in the history
…ishearth

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 rust-lang#12917

```
changelog: [`doc_lazy_continuation`]: suggest blank line for likely-unintended lazy continuations
```
  • Loading branch information
bors committed Jun 28, 2024
2 parents 68a799a + 6de8782 commit 2f80536
Show file tree
Hide file tree
Showing 11 changed files with 246 additions and 68 deletions.
29 changes: 27 additions & 2 deletions clippy_lints/src/doc/lazy_continuation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/doc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
},
Expand Down
47 changes: 47 additions & 0 deletions tests/ui/doc/doc_lazy_blank_line.fixed
Original file line number Diff line number Diff line change
@@ -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);
43 changes: 43 additions & 0 deletions tests/ui/doc/doc_lazy_blank_line.rs
Original file line number Diff line number Diff line change
@@ -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);
56 changes: 56 additions & 0 deletions tests/ui/doc/doc_lazy_blank_line.stderr
Original file line number Diff line number Diff line change
@@ -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

12 changes: 6 additions & 6 deletions tests/ui/doc/doc_lazy_blockquote.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/// > blockquote with
/// > lazy continuation
//~^ ERROR: doc quote missing `>` marker
//~^ ERROR: doc quote line without `>` marker
fn first() {}

/// > blockquote with no
Expand All @@ -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() {}
12 changes: 6 additions & 6 deletions tests/ui/doc/doc_lazy_blockquote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/// > blockquote with
/// lazy continuation
//~^ ERROR: doc quote missing `>` marker
//~^ ERROR: doc quote line without `>` marker
fn first() {}

/// > blockquote with no
Expand All @@ -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() {}
12 changes: 6 additions & 6 deletions tests/ui/doc/doc_lazy_blockquote.stderr
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
Loading

0 comments on commit 2f80536

Please sign in to comment.