Add additional test cases and improve span lint

3 files changed, 108 insertions, 49 deletions

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 5bfdc98bc6a..817230a29c6 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -2034,42 +2034,27 @@ fn check_manual_flatten<'tcx>(
 
                 // Prepare the help message
                 let mut applicability = Applicability::MaybeIncorrect;
-                let arg_snippet = snippet_with_applicability(
-                    cx,
-                    arg.span,
-                    "..",
-                    &mut applicability,
-                );
-                // Determine if `arg` is by reference, an `Iterator`, or implicitly adjusted with `into_iter`
-                let arg_ty = cx.typeck_results().expr_ty(arg);
-                let hint = if arg_ty.is_ref() {
-                    if has_iter_method(cx, arg_ty).is_none() {
-                        return;
-                    } else if let ExprKind::AddrOf(_, _, arg_expr) = arg.kind {
-                        format!("{}.iter().flatten()", snippet(cx, arg_expr.span, ".."))
-                    } else {
-                        return;
-                    }
-                } else if let Some(id) = get_trait_def_id(cx, &paths::ITERATOR) {
-                    let is_iterator = implements_trait(cx, arg_ty, id, &[]);
-                    if is_iterator {
-                        format!("{}.flatten()", arg_snippet)
-                    } else {
-                        format!("{}.into_iter().flatten()", arg_snippet)
-                    }
-                } else {
-                    return
-                };
+                let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
 
-                span_lint_and_sugg(
+                span_lint_and_then(
                     cx,
                     MANUAL_FLATTEN,
                     span,
                     &msg,
-                    "try",
-                    hint,
-                    applicability,
-                )
+                    |diag| {
+                        let sugg = format!("{}.flatten()", arg_snippet);
+                        diag.span_suggestion(
+                            arg.span,
+                            "try",
+                            sugg,
+                            Applicability::MaybeIncorrect,
+                        );
+                        diag.span_help(
+                            inner_expr.span,
+                            "also remove the `if let` statement in the for loop",
+                        );
+                    }
+                );
             }
         }
     }
diff --git a/tests/ui/manual_flatten.rs b/tests/ui/manual_flatten.rs
index b97cceb66f8..ea3440f6da2 100644
--- a/tests/ui/manual_flatten.rs
+++ b/tests/ui/manual_flatten.rs
@@ -4,8 +4,8 @@ fn main() {
     // Test for loop over implicitly adjusted `Iterator` with `if let` expression
     let x = vec![Some(1), Some(2), Some(3)];
     for n in x {
-        if let Some(n) = n {
-            println!("{}", n);
+        if let Some(y) = n {
+            println!("{}", y);
         }
     }
 
@@ -24,12 +24,22 @@ fn main() {
         }
     }
 
+    // Test for loop over an implicit reference
+    // Note: If `clippy::manual_flatten` is made autofixable, this case will
+    // lead to a follow-up lint `clippy::into_iter_on_ref`
+    let z = &y;
+    for n in z {
+        if let Ok(n) = n {
+            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 n in z {
-        if let Some(n) = n {
-            println!("{}", n);
+        if let Some(m) = n {
+            println!("{}", m);
         }
     }
 
diff --git a/tests/ui/manual_flatten.stderr b/tests/ui/manual_flatten.stderr
index 754921eb739..49b8ed0564a 100644
--- a/tests/ui/manual_flatten.stderr
+++ b/tests/ui/manual_flatten.stderr
@@ -1,44 +1,108 @@
 error: unnecessary `if let` since only the `Some` variant of the iterator element is used
   --> $DIR/manual_flatten.rs:6:5
    |
-LL | /     for n in x {
-LL | |         if let Some(n) = n {
-LL | |             println!("{}", n);
+LL |       for n in x {
+   |       ^        - help: try: `x.into_iter().flatten()`
+   |  _____|
+   | |
+LL | |         if let Some(y) = n {
+LL | |             println!("{}", y);
 LL | |         }
 LL | |     }
-   | |_____^ help: try: `x.into_iter().flatten()`
+   | |_____^
    |
    = note: `-D clippy::manual-flatten` implied by `-D warnings`
+help: also remove the `if let` statement in the for loop
+  --> $DIR/manual_flatten.rs:7:9
+   |
+LL | /         if let Some(y) = n {
+LL | |             println!("{}", y);
+LL | |         }
+   | |_________^
 
 error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
   --> $DIR/manual_flatten.rs:14:5
    |
-LL | /     for n in y.clone() {
+LL |       for n in y.clone() {
+   |       ^        --------- help: try: `y.clone().into_iter().flatten()`
+   |  _____|
+   | |
 LL | |         if let Ok(n) = n {
 LL | |             println!("{}", n);
 LL | |         };
 LL | |     }
-   | |_____^ help: try: `y.clone().into_iter().flatten()`
+   | |_____^
+   |
+help: also remove the `if let` statement in the for loop
+  --> $DIR/manual_flatten.rs:15:9
+   |
+LL | /         if let Ok(n) = n {
+LL | |             println!("{}", n);
+LL | |         };
+   | |_________^
 
 error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
   --> $DIR/manual_flatten.rs:21:5
    |
-LL | /     for n in &y {
+LL |       for n in &y {
+   |       ^        -- help: try: `y.iter().flatten()`
+   |  _____|
+   | |
 LL | |         if let Ok(n) = n {
 LL | |             println!("{}", n);
 LL | |         }
 LL | |     }
-   | |_____^ help: try: `y.iter().flatten()`
+   | |_____^
+   |
+help: also remove the `if let` statement in the for loop
+  --> $DIR/manual_flatten.rs:22:9
+   |
+LL | /         if let Ok(n) = n {
+LL | |             println!("{}", n);
+LL | |         }
+   | |_________^
 
-error: unnecessary `if let` since only the `Some` variant of the iterator element is used
-  --> $DIR/manual_flatten.rs:30:5
+error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
+  --> $DIR/manual_flatten.rs:31:5
    |
-LL | /     for n in z {
-LL | |         if let Some(n) = n {
+LL |       for n in z {
+   |       ^        - help: try: `z.into_iter().flatten()`
+   |  _____|
+   | |
+LL | |         if let Ok(n) = n {
 LL | |             println!("{}", n);
 LL | |         }
 LL | |     }
-   | |_____^ help: try: `z.flatten()`
+   | |_____^
+   |
+help: also remove the `if let` statement in the for loop
+  --> $DIR/manual_flatten.rs:32:9
+   |
+LL | /         if let Ok(n) = n {
+LL | |             println!("{}", n);
+LL | |         }
+   | |_________^
+
+error: unnecessary `if let` since only the `Some` variant of the iterator element is used
+  --> $DIR/manual_flatten.rs:40:5
+   |
+LL |       for n in z {
+   |       ^        - help: try: `z.flatten()`
+   |  _____|
+   | |
+LL | |         if let Some(m) = n {
+LL | |             println!("{}", m);
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: also remove the `if let` statement in the for loop
+  --> $DIR/manual_flatten.rs:41:9
+   |
+LL | /         if let Some(m) = n {
+LL | |             println!("{}", m);
+LL | |         }
+   | |_________^
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors