about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-07-13 12:52:08 +0000
committerbors <bors@rust-lang.org>2022-07-13 12:52:08 +0000
commitf78292a4ad8dc0aa625bfd87bf473fa5d744730d (patch)
treeec29a427a415ed40ab74a92b758fb6cf58d30fcd
parent07ab0e87417e8cea018a34c7cd46fde0693d32a5 (diff)
parent852c38c543d4b2cdea4af9fe8d5ddf229ebd1410 (diff)
downloadrust-f78292a4ad8dc0aa625bfd87bf473fa5d744730d.tar.gz
rust-f78292a4ad8dc0aa625bfd87bf473fa5d744730d.zip
Auto merge of #9156 - Rqnsom:manual_flatten, r=Alexendoo
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
-rw-r--r--clippy_lints/src/loops/manual_flatten.rs16
-rw-r--r--tests/lint_message_convention.rs1
-rw-r--r--tests/ui/manual_flatten.rs16
-rw-r--r--tests/ui/manual_flatten.stderr30
4 files changed, 59 insertions, 4 deletions
diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs
index d276c901059..1d6ddf4b99f 100644
--- a/clippy_lints/src/loops/manual_flatten.rs
+++ b/clippy_lints/src/loops/manual_flatten.rs
@@ -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,
                     );
                 }
             );
diff --git a/tests/lint_message_convention.rs b/tests/lint_message_convention.rs
index dd1d4412036..9519c2e9379 100644
--- a/tests/lint_message_convention.rs
+++ b/tests/lint_message_convention.rs
@@ -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();
 
diff --git a/tests/ui/manual_flatten.rs b/tests/ui/manual_flatten.rs
index 6c5232ec5f5..d922593bc6f 100644
--- a/tests/ui/manual_flatten.rs
+++ b/tests/ui/manual_flatten.rs
@@ -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);
+        }
+    }
 }
diff --git a/tests/ui/manual_flatten.stderr b/tests/ui/manual_flatten.stderr
index 392e1a39393..da053c05668 100644
--- a/tests/ui/manual_flatten.stderr
+++ b/tests/ui/manual_flatten.stderr
@@ -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