Fixes manual_flatten removes the useless if let

clippy_lints/src/loops/manual_flatten.rs

@@ -2,8 +2,9 @@ use super::MANUAL_FLATTEN;
 use super::utils::make_iterator_snippet;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::{HasSession, indent_of, reindent_multiline, snippet_with_applicability};
 use clippy_utils::visitors::is_local_used;
-use clippy_utils::{higher, path_to_local_id, peel_blocks_with_stmt};
+use clippy_utils::{higher, path_to_local_id, peel_blocks_with_stmt, span_contains_comment};
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::{Expr, Pat, PatKind};
@@ -28,7 +29,7 @@ pub(super) fn check<'tcx>(
         && let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind
         && path_to_local_id(let_expr, pat_hir_id)
         // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
-        && let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind
+        && let PatKind::TupleStruct(ref qpath, let_pats, _) = let_pat.kind
         && let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id)
         && let Some(variant_id) = cx.tcx.opt_parent(ctor_id)
         && let some_ctor = cx.tcx.lang_items().option_some_variant() == Some(variant_id)
@@ -38,13 +39,19 @@ pub(super) fn check<'tcx>(
         && !is_local_used(cx, if_then, pat_hir_id)
         && msrv.meets(cx, msrvs::ITER_FLATTEN)
     {
+        if arg.span.from_expansion() || if_then.span.from_expansion() {
+            return;
+        }
         let if_let_type = if some_ctor { "Some" } else { "Ok" };
         // Prepare the error message
         let msg =
             format!("unnecessary `if let` since only the `{if_let_type}` variant of the iterator element is used");
 
-        // Prepare the help message
-        let mut applicability = Applicability::MaybeIncorrect;
+        let mut applicability = if span_contains_comment(cx.sess().source_map(), body.span) {
+            Applicability::MaybeIncorrect
+        } else {
+            Applicability::MachineApplicable
+        };
         let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
         let copied = match cx.typeck_results().expr_ty(let_expr).kind() {
             ty::Ref(_, inner, _) => match inner.kind() {
@@ -54,20 +61,35 @@ pub(super) fn check<'tcx>(
             _ => "",
         };
 
-        let sugg = format!("{arg_snippet}{copied}.flatten()");
+        let help_msg = "try `.flatten()` and remove the `if let` statement in the for loop";
 
-        // 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..."
+        let pat_snippet = if let_pats.is_empty() {
+            "_".to_string()
         } else {
-            "...and remove the `if let` statement in the for loop"
+            snippet_with_applicability(
+                cx,
+                let_pats.first().unwrap().span.source_callsite(),
+                "_",
+                &mut applicability,
+            )
+            .to_string()
         };
+        let body_snippet =
+            snippet_with_applicability(cx, if_then.span.source_callsite(), "[body]", &mut applicability).to_string();
+        let suggestions = vec![
+            // flatten the iterator
+            (arg.span, format!("{arg_snippet}{copied}.flatten()")),
+            (pat.span, pat_snippet),
+            // remove the `if let` statement
+            (
+                body.span,
+                reindent_multiline(&body_snippet, true, indent_of(cx, body.span)),
+            ),
+        ];
 
         span_lint_and_then(cx, MANUAL_FLATTEN, span, msg, |diag| {
-            diag.span_suggestion(arg.span, "try", sugg, applicability);
             diag.span_help(inner_expr.span, help_msg);
+            diag.multipart_suggestion("try", suggestions, applicability);
         });
     }
 }

tests/ui/manual_flatten.fixed

@@ -0,0 +1,141 @@
+#![warn(clippy::manual_flatten)]
+#![allow(clippy::useless_vec, clippy::uninlined_format_args)]
+
+fn main() {
+    // Test for loop over implicitly adjusted `Iterator` with `if let` expression
+    let x = vec![Some(1), Some(2), Some(3)];
+    for y in x.into_iter().flatten() {
+        println!("{}", y);
+    }
+
+    // Test for loop over implicitly adjusted `Iterator` with `if let` statement
+    let y: Vec<Result<i32, i32>> = vec![];
+    for n in y.clone().into_iter().flatten() {
+        println!("{}", n);
+    }
+
+    // Test for loop over by reference
+    for n in y.iter().flatten() {
+        println!("{}", n);
+    }
+
+    // Test for loop over an implicit reference
+    let z = &y;
+    for n in z.iter().flatten() {
+        println!("{}", n);
+    }
+
+    // Test for loop over `Iterator` with `if let` expression
+    let z = vec![Some(1), Some(2), Some(3)];
+    let z = z.iter();
+    for m in z.flatten() {
+        println!("{}", m);
+    }
+
+    // Using the `None` variant should not trigger the lint
+    // Note: for an autofixable suggestion, the binding in the for loop has to take the
+    // name of the binding in the `if let`
+    let z = vec![Some(1), Some(2), Some(3)];
+    for n in z {
+        if n.is_none() {
+            println!("Nada.");
+        }
+    }
+
+    // Using the `Err` variant should not trigger the lint
+    for n in y.clone() {
+        if let Err(e) = n {
+            println!("Oops: {}!", e);
+        }
+    }
+
+    // Having an else clause should not trigger the lint
+    for n in y.clone() {
+        if let Ok(n) = n {
+            println!("{}", n);
+        } else {
+            println!("Oops!");
+        }
+    }
+
+    let vec_of_ref = vec![&Some(1)];
+    for n in vec_of_ref.iter().copied().flatten() {
+        println!("{:?}", n);
+    }
+
+    let vec_of_ref = &vec_of_ref;
+    for n in vec_of_ref.iter().copied().flatten() {
+        println!("{:?}", n);
+    }
+
+    let slice_of_ref = &[&Some(1)];
+    for n in slice_of_ref.iter().copied().flatten() {
+        println!("{:?}", n);
+    }
+
+    struct Test {
+        a: usize,
+    }
+
+    let mut vec_of_struct = [Some(Test { a: 1 }), None];
+
+    // Usage of `if let` expression should not trigger lint
+    for n in vec_of_struct.iter_mut() {
+        if let Some(z) = n {
+            *n = None;
+        }
+    }
+
+    // Using manual flatten should not trigger the lint
+    for n in vec![Some(1), Some(2), Some(3)].iter().flatten() {
+        println!("{}", n);
+    }
+
+    macro_rules! inner {
+        ($id:ident / $new:pat => $action:expr) => {
+            if let Some($new) = $id {
+                $action;
+            }
+        };
+    }
+
+    // Usage of `if let` expression with macro should not trigger lint
+    for ab in [Some((1, 2)), Some((3, 4))] {
+        inner!(ab / (c, d) => println!("{c}-{d}"));
+    }
+
+    macro_rules! args {
+        ($($arg:expr),*) => {
+            vec![$(Some($arg)),*]
+        };
+    }
+
+    // Usage of `if let` expression with macro should not trigger lint
+    for n in args!(1, 2, 3) {
+        if let Some(n) = n {
+            println!("{:?}", n);
+        }
+    }
+
+    // This should trigger the lint, but the applicability is `MaybeIncorrect`
+    let z = vec![Some(1), Some(2), Some(3)];
+    for n in z.into_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![
+    //~^ manual_flatten
+
+        Some(1),
+        Some(2),
+        Some(3)
+    ].iter().flatten() {
+        println!("{:?}", n);
+    }
+}

tests/ui/manual_flatten.rs

@@ -1,6 +1,6 @@
 #![warn(clippy::manual_flatten)]
 #![allow(clippy::useless_vec, clippy::uninlined_format_args)]
-//@no-rustfix
+
 fn main() {
     // Test for loop over implicitly adjusted `Iterator` with `if let` expression
     let x = vec![Some(1), Some(2), Some(3)];
@@ -123,6 +123,43 @@ fn main() {
         println!("{}", n);
     }
 
+    macro_rules! inner {
+        ($id:ident / $new:pat => $action:expr) => {
+            if let Some($new) = $id {
+                $action;
+            }
+        };
+    }
+
+    // Usage of `if let` expression with macro should not trigger lint
+    for ab in [Some((1, 2)), Some((3, 4))] {
+        inner!(ab / (c, d) => println!("{c}-{d}"));
+    }
+
+    macro_rules! args {
+        ($($arg:expr),*) => {
+            vec![$(Some($arg)),*]
+        };
+    }
+
+    // Usage of `if let` expression with macro should not trigger lint
+    for n in args!(1, 2, 3) {
+        if let Some(n) = n {
+            println!("{:?}", n);
+        }
+    }
+
+    // This should trigger the lint, but the applicability is `MaybeIncorrect`
+    let z = vec![Some(1), Some(2), Some(3)];
+    for n in z {
+        //~^ manual_flatten
+
+        if let Some(n) = n {
+            println!("{:?}", n);
+        }
+        // foo
+    }
+
     run_unformatted_tests();
 }