Skip to content

Commit

Permalink
Auto merge of #9156 - Rqnsom:manual_flatten, r=Alexendoo
Browse files Browse the repository at this point in the history
fix [`manual_flatten`] help texts order

fixes  #8948

Whenever suggestion for this lint does not fit in one line,
legacy solution has some unexpected/unhandled behavior:
lint will then generate two help messages which seem to be shown in the wrong order.
The second help message in that case will contain the suggestion.

The first help message always refers to a suggestion message,
and **it should adapt** depending on the location of the suggestion:
- inline suggestion within the error/warning message
- suggestion separated into a second help text

This is my first contribution here, so I hope I didn't miss anything for creating this PR.

changelog: fix [`manual_flatten`] help texts order
  • Loading branch information
bors committed Jul 13, 2022
2 parents 07ab0e8 + 852c38c commit f78292a
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 4 deletions.
16 changes: 13 additions & 3 deletions clippy_lints/src/loops/manual_flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,32 @@ pub(super) fn check<'tcx>(
_ => ""
};

let sugg = format!("{arg_snippet}{copied}.flatten()");

// If suggestion is not a one-liner, it won't be shown inline within the error message. In that case,
// it will be shown in the extra `help` message at the end, which is why the first `help_msg` needs
// to refer to the correct relative position of the suggestion.
let help_msg = if sugg.contains('\n') {
"remove the `if let` statement in the for loop and then..."
} else {
"...and remove the `if let` statement in the for loop"
};

span_lint_and_then(
cx,
MANUAL_FLATTEN,
span,
&msg,
|diag| {
let sugg = format!("{}{}.flatten()", arg_snippet, copied);
diag.span_suggestion(
arg.span,
"try",
sugg,
Applicability::MaybeIncorrect,
applicability,
);
diag.span_help(
inner_expr.span,
"...and remove the `if let` statement in the for loop",
help_msg,
);
}
);
Expand Down
1 change: 1 addition & 0 deletions tests/lint_message_convention.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ impl Message {
r".*remove .*the return type...",
r"note: Clippy version: .*",
r"the compiler unexpectedly panicked. this is a bug.",
r"remove the `if let` statement in the for loop and then...",
])
.unwrap();

Expand Down
16 changes: 16 additions & 0 deletions tests/ui/manual_flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,20 @@ fn main() {
for n in vec![Some(1), Some(2), Some(3)].iter().flatten() {
println!("{}", n);
}

run_unformatted_tests();
}

#[rustfmt::skip]
fn run_unformatted_tests() {
// Skip rustfmt here on purpose so the suggestion does not fit in one line
for n in vec![
Some(1),
Some(2),
Some(3)
].iter() {
if let Some(n) = n {
println!("{:?}", n);
}
}
}
30 changes: 29 additions & 1 deletion tests/ui/manual_flatten.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -167,5 +167,33 @@ LL | | println!("{:?}", n);
LL | | }
| |_________^

error: aborting due to 8 previous errors
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
--> $DIR/manual_flatten.rs:116:5
|
LL | / for n in vec![
LL | | Some(1),
LL | | Some(2),
LL | | Some(3)
... |
LL | | }
LL | | }
| |_____^
|
help: remove the `if let` statement in the for loop and then...
--> $DIR/manual_flatten.rs:121:9
|
LL | / if let Some(n) = n {
LL | | println!("{:?}", n);
LL | | }
| |_________^
help: try
|
LL ~ for n in vec![
LL + Some(1),
LL + Some(2),
LL + Some(3)
LL ~ ].iter().flatten() {
|

error: aborting due to 9 previous errors

0 comments on commit f78292a

Please sign in to comment.