Skip to content

Commit 852c38c

Browse files
committed
fix for manual_flatten help texts order
Whenever suggestion for this lint does not fit in one line, lint will generate two help messages. The second help message will always contain the suggestion. The first help message refers to suggestion message, and it should adapt depending on the location of the suggestion: - inline suggestion within the error/warning message - suggestion separated into second help text
1 parent 7c1598b commit 852c38c

File tree

4 files changed

+59
-4
lines changed

4 files changed

+59
-4
lines changed

clippy_lints/src/loops/manual_flatten.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,32 @@ pub(super) fn check<'tcx>(
5151
_ => ""
5252
};
5353

54+
let sugg = format!("{arg_snippet}{copied}.flatten()");
55+
56+
// If suggestion is not a one-liner, it won't be shown inline within the error message. In that case,
57+
// it will be shown in the extra `help` message at the end, which is why the first `help_msg` needs
58+
// to refer to the correct relative position of the suggestion.
59+
let help_msg = if sugg.contains('\n') {
60+
"remove the `if let` statement in the for loop and then..."
61+
} else {
62+
"...and remove the `if let` statement in the for loop"
63+
};
64+
5465
span_lint_and_then(
5566
cx,
5667
MANUAL_FLATTEN,
5768
span,
5869
&msg,
5970
|diag| {
60-
let sugg = format!("{}{}.flatten()", arg_snippet, copied);
6171
diag.span_suggestion(
6272
arg.span,
6373
"try",
6474
sugg,
65-
Applicability::MaybeIncorrect,
75+
applicability,
6676
);
6777
diag.span_help(
6878
inner_expr.span,
69-
"...and remove the `if let` statement in the for loop",
79+
help_msg,
7080
);
7181
}
7282
);

tests/lint_message_convention.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ impl Message {
4242
r".*remove .*the return type...",
4343
r"note: Clippy version: .*",
4444
r"the compiler unexpectedly panicked. this is a bug.",
45+
r"remove the `if let` statement in the for loop and then...",
4546
])
4647
.unwrap();
4748

tests/ui/manual_flatten.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,4 +106,20 @@ fn main() {
106106
for n in vec![Some(1), Some(2), Some(3)].iter().flatten() {
107107
println!("{}", n);
108108
}
109+
110+
run_unformatted_tests();
111+
}
112+
113+
#[rustfmt::skip]
114+
fn run_unformatted_tests() {
115+
// Skip rustfmt here on purpose so the suggestion does not fit in one line
116+
for n in vec![
117+
Some(1),
118+
Some(2),
119+
Some(3)
120+
].iter() {
121+
if let Some(n) = n {
122+
println!("{:?}", n);
123+
}
124+
}
109125
}

tests/ui/manual_flatten.stderr

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,5 +167,33 @@ LL | | println!("{:?}", n);
167167
LL | | }
168168
| |_________^
169169

170-
error: aborting due to 8 previous errors
170+
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
171+
--> $DIR/manual_flatten.rs:116:5
172+
|
173+
LL | / for n in vec![
174+
LL | | Some(1),
175+
LL | | Some(2),
176+
LL | | Some(3)
177+
... |
178+
LL | | }
179+
LL | | }
180+
| |_____^
181+
|
182+
help: remove the `if let` statement in the for loop and then...
183+
--> $DIR/manual_flatten.rs:121:9
184+
|
185+
LL | / if let Some(n) = n {
186+
LL | | println!("{:?}", n);
187+
LL | | }
188+
| |_________^
189+
help: try
190+
|
191+
LL ~ for n in vec![
192+
LL + Some(1),
193+
LL + Some(2),
194+
LL + Some(3)
195+
LL ~ ].iter().flatten() {
196+
|
197+
198+
error: aborting due to 9 previous errors
171199

0 commit comments

Comments
 (0)