Skip to content

Commit 2e43d0c

Browse files
Centri3Centri3
committed
Improve suggestion and add more tests
1 parent 885ce61 commit 2e43d0c

7 files changed

+97
-50
lines changed

‎clippy_lints/src/four_forward_slashes.rs

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_opt};
1+
use clippy_utils::diagnostics::span_lint_and_then;
22
use rustc_errors::Applicability;
33
use rustc_hir::Item;
44
use rustc_lint::{LateContext, LateLintPass, LintContext};
55
use rustc_session::{declare_lint_pass, declare_tool_lint};
6-
use rustc_span::{Span, SyntaxContext};
6+
use rustc_span::Span;
77

88
declare_clippy_lint! {
99
/// ### What it does
@@ -40,43 +40,60 @@ impl<'tcx> LateLintPass<'tcx> for FourForwardSlashes {
4040
if item.span.from_expansion() {
4141
return;
4242
}
43-
let src = cx.sess().source_map();
44-
let item_and_attrs_span = cx
43+
let sm = cx.sess().source_map();
44+
let mut span = cx
4545
.tcx
4646
.hir()
4747
.attrs(item.hir_id())
4848
.iter()
4949
.fold(item.span.shrink_to_lo(), |span, attr| span.to(attr.span));
50-
let (Some(file), _, _, end_line, _) = src.span_to_location_info(item_and_attrs_span) else {
50+
let (Some(file), _, _, end_line, _) = sm.span_to_location_info(span) else {
5151
return;
5252
};
53+
let mut bad_comments = vec![];
5354
for line in (0..end_line.saturating_sub(1)).rev() {
54-
let Some(contents) = file.get_line(line) else {
55-
continue;
55+
let Some(contents) = file.get_line(line).map(|c| c.trim().to_owned()) else {
56+
return;
5657
};
57-
let contents = contents.trim();
58-
if contents.is_empty() {
58+
// Keep searching until we find the next item
59+
if !contents.is_empty() && !contents.starts_with("//") && !contents.starts_with("#[") {
5960
break;
6061
}
61-
if contents.starts_with("////") {
62+
63+
if contents.starts_with("////") && !matches!(contents.chars().nth(4), Some('/' | '!')) {
6264
let bounds = file.line_bounds(line);
63-
let span = Span::new(bounds.start, bounds.end, SyntaxContext::root(), None);
65+
let line_span = Span::with_root_ctxt(bounds.start, bounds.end);
66+
span = line_span.to(span);
67+
bad_comments.push((line_span, contents));
68+
}
69+
}
6470

65-
if snippet_opt(cx, span).is_some_and(|s| s.trim().starts_with("////")) {
66-
span_lint_and_sugg(
67-
cx,
68-
FOUR_FORWARD_SLASHES,
69-
span,
70-
"comment with 4 forward slashes (`////`). This looks like a doc comment, but it isn't",
71-
"make this a doc comment by removing one `/`",
72-
// It's a little unfortunate but the span includes the `\n` yet the contents
73-
// do not, so we must add it back. If some codebase uses `\r\n` instead they
74-
// will need normalization but it should be fine
75-
contents.replacen("////", "///", 1) + "\n",
71+
if !bad_comments.is_empty() {
72+
span_lint_and_then(
73+
cx,
74+
FOUR_FORWARD_SLASHES,
75+
span,
76+
"this item has comments with 4 forward slashes (`////`). These look like doc comments, but they aren't",
77+
|diag| {
78+
let msg = if bad_comments.len() == 1 {
79+
"make this a doc comment by removing one `/`"
80+
} else {
81+
"turn these into doc comments by removing one `/`"
82+
};
83+
84+
diag.multipart_suggestion(
85+
msg,
86+
bad_comments
87+
.into_iter()
88+
// It's a little unfortunate but the span includes the `\n` yet the contents
89+
// do not, so we must add it back. If some codebase uses `\r\n` instead they
90+
// will need normalization but it should be fine
91+
.map(|(span, c)| (span, c.replacen("////", "///", 1) + "\n"))
92+
.collect(),
7693
Applicability::MachineApplicable,
7794
);
78-
}
79-
}
95+
},
96+
);
8097
}
8198
}
8299
}

‎tests/ui/four_forward_slashes.fixed

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
//// first line borked doc comment. doesn't combust!
21
//@run-rustfix
32
//@aux-build:proc_macros.rs:proc-macro
43
#![feature(custom_inner_attributes)]
@@ -32,6 +31,11 @@ fn g() {}
3231
/// not very start of contents
3332
fn h() {}
3433

34+
fn i() {
35+
//// don't lint me bozo
36+
todo!()
37+
}
38+
3539
external! {
3640
//// don't lint me bozo
3741
fn e() {}

‎tests/ui/four_forward_slashes.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
//// first line borked doc comment. doesn't combust!
21
//@run-rustfix
32
//@aux-build:proc_macros.rs:proc-macro
43
#![feature(custom_inner_attributes)]
@@ -32,6 +31,11 @@ fn g() {}
3231
//// not very start of contents
3332
fn h() {}
3433

34+
fn i() {
35+
//// don't lint me bozo
36+
todo!()
37+
}
38+
3539
external! {
3640
//// don't lint me bozo
3741
fn e() {}

‎tests/ui/four_forward_slashes.stderr

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
error: comment with 4 forward slashes (`////`). This looks like a doc comment, but it isn't
2-
--> $DIR/four_forward_slashes.rs:13:1
1+
error: this item has comments with 4 forward slashes (`////`). These look like doc comments, but they aren't
2+
--> $DIR/four_forward_slashes.rs:12:1
33
|
44
LL | / //// whoops
55
LL | | fn a() {}
@@ -11,56 +11,49 @@ help: make this a doc comment by removing one `/`
1111
LL + /// whoops
1212
|
1313

14-
error: comment with 4 forward slashes (`////`). This looks like a doc comment, but it isn't
15-
--> $DIR/four_forward_slashes.rs:16:1
14+
error: this item has comments with 4 forward slashes (`////`). These look like doc comments, but they aren't
15+
--> $DIR/four_forward_slashes.rs:15:1
1616
|
1717
LL | / //// whoops
1818
LL | | #[allow(dead_code)]
19+
LL | | fn b() {}
1920
| |_
2021
|
2122
help: make this a doc comment by removing one `/`
2223
|
2324
LL + /// whoops
2425
|
2526

26-
error: comment with 4 forward slashes (`////`). This looks like a doc comment, but it isn't
27-
--> $DIR/four_forward_slashes.rs:21:1
28-
|
29-
LL | / //// two borked comments!
30-
LL | | #[track_caller]
31-
| |_
32-
|
33-
help: make this a doc comment by removing one `/`
34-
|
35-
LL + /// two borked comments!
36-
|
37-
38-
error: comment with 4 forward slashes (`////`). This looks like a doc comment, but it isn't
39-
--> $DIR/four_forward_slashes.rs:20:1
27+
error: this item has comments with 4 forward slashes (`////`). These look like doc comments, but they aren't
28+
--> $DIR/four_forward_slashes.rs:19:1
4029
|
4130
LL | / //// whoops
4231
LL | | //// two borked comments!
32+
LL | | #[track_caller]
33+
LL | | fn c() {}
4334
| |_
4435
|
45-
help: make this a doc comment by removing one `/`
36+
help: turn these into doc comments by removing one `/`
4637
|
4738
LL + /// whoops
39+
LL ~ /// two borked comments!
4840
|
4941

50-
error: comment with 4 forward slashes (`////`). This looks like a doc comment, but it isn't
51-
--> $DIR/four_forward_slashes.rs:28:1
42+
error: this item has comments with 4 forward slashes (`////`). These look like doc comments, but they aren't
43+
--> $DIR/four_forward_slashes.rs:27:1
5244
|
5345
LL | / //// between attributes
5446
LL | | #[allow(dead_code)]
47+
LL | | fn g() {}
5548
| |_
5649
|
5750
help: make this a doc comment by removing one `/`
5851
|
5952
LL + /// between attributes
6053
|
6154

62-
error: comment with 4 forward slashes (`////`). This looks like a doc comment, but it isn't
63-
--> $DIR/four_forward_slashes.rs:32:1
55+
error: this item has comments with 4 forward slashes (`////`). These look like doc comments, but they aren't
56+
--> $DIR/four_forward_slashes.rs:31:1
6457
|
6558
LL | / //// not very start of contents
6659
LL | | fn h() {}
@@ -71,5 +64,5 @@ help: make this a doc comment by removing one `/`
7164
LL + /// not very start of contents
7265
|
7366

74-
error: aborting due to 6 previous errors
67+
error: aborting due to 5 previous errors
7568

‎tests/ui/four_forward_slashes_first_line.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/// borked doc comment on the first line. doesn't combust!
2+
fn a() {}
3+
4+
//@run-rustfix
5+
// This test's entire purpose is to make sure we don't panic if the comment with four slashes
6+
// extends to the first line of the file. This is likely pretty rare in production, but an ICE is an
7+
// ICE.

‎tests/ui/four_forward_slashes_first_line.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
//// borked doc comment on the first line. doesn't combust!
2+
fn a() {}
3+
4+
//@run-rustfix
5+
// This test's entire purpose is to make sure we don't panic if the comment with four slashes
6+
// extends to the first line of the file. This is likely pretty rare in production, but an ICE is an
7+
// ICE.

‎tests/ui/four_forward_slashes_first_line.stderr

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: this item has comments with 4 forward slashes (`////`). These look like doc comments, but they aren't
2+
--> $DIR/four_forward_slashes_first_line.rs:1:1
3+
|
4+
LL | / //// borked doc comment on the first line. doesn't combust!
5+
LL | | fn a() {}
6+
| |_
7+
|
8+
= note: `-D clippy::four-forward-slashes` implied by `-D warnings`
9+
help: make this a doc comment by removing one `/`
10+
|
11+
LL + /// borked doc comment on the first line. doesn't combust!
12+
|
13+
14+
error: aborting due to previous error
15+

0 commit comments

Comments
 (0)