Auto merge of #6962 - TaKO8Ki:fix-false-positive-in-manual-flatten, r=llogiq

bors · bors · commit 917b538c6857 · 2021-03-24T18:42:20.000Z
Fix false positive in `manual_flatten` This pull request fixes false positive in `manual_flatten` in case using a slice of references . closes: #6893 changelog: fix false positive in `manual_flatten`
diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs
@@ -6,6 +6,7 @@ use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, MatchSource, Pat, PatKind, QPath, StmtKind};
 use rustc_lint::LateContext;
+use rustc_middle::ty;
 use rustc_span::source_map::Span;
 
 /// Check for unnecessary `if let` usage in a for loop where only the `Some` or `Ok` variant of the
@@ -54,14 +55,21 @@ pub(super) fn check<'tcx>(
                 // Prepare the help message
                 let mut applicability = Applicability::MaybeIncorrect;
                 let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
+                let copied = match cx.typeck_results().expr_ty(match_expr).kind() {
+                    ty::Ref(_, inner, _) => match inner.kind() {
+                        ty::Ref(..) => ".copied()",
+                        _ => ""
+                    }
+                    _ => ""
+                };
 
                 span_lint_and_then(
                     cx,
                     MANUAL_FLATTEN,
                     span,
                     &msg,
                     |diag| {
-                        let sugg = format!("{}.flatten()", arg_snippet);
+                        let sugg = format!("{}{}.flatten()", arg_snippet, copied);
                         diag.span_suggestion(
                             arg.span,
                             "try",
diff --git a/clippy_lints/src/methods/utils.rs b/clippy_lints/src/methods/utils.rs
@@ -5,8 +5,7 @@ use rustc_ast::ast;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
-use rustc_middle::ty;
-use rustc_middle::ty::Ty;
+use rustc_middle::ty::{self, Ty};
 use rustc_span::symbol::sym;
 
 pub(super) fn derefs_to_slice<'tcx>(
diff --git a/tests/ui/manual_flatten.rs b/tests/ui/manual_flatten.rs
@@ -1,4 +1,5 @@
 #![warn(clippy::manual_flatten)]
+#![allow(clippy::useless_vec)]
 
 fn main() {
     // Test for loop over implicitly adjusted `Iterator` with `if let` expression
@@ -69,6 +70,27 @@ fn main() {
         }
     }
 
+    let vec_of_ref = vec![&Some(1)];
+    for n in &vec_of_ref {
+        if let Some(n) = n {
+            println!("{:?}", n);
+        }
+    }
+
+    let vec_of_ref = &vec_of_ref;
+    for n in vec_of_ref {
+        if let Some(n) = n {
+            println!("{:?}", n);
+        }
+    }
+
+    let slice_of_ref = &[&Some(1)];
+    for n in slice_of_ref {
+        if let Some(n) = n {
+            println!("{:?}", n);
+        }
+    }
+
     // Using manual flatten should not trigger the lint
     for n in vec![Some(1), Some(2), Some(3)].iter().flatten() {
         println!("{}", n);
diff --git a/tests/ui/manual_flatten.stderr b/tests/ui/manual_flatten.stderr
@@ -1,5 +1,5 @@
 error: unnecessary `if let` since only the `Some` variant of the iterator element is used
-  --> $DIR/manual_flatten.rs:6:5
+  --> $DIR/manual_flatten.rs:7:5
    |
 LL |       for n in x {
    |       ^        - help: try: `x.into_iter().flatten()`
@@ -13,15 +13,15 @@ LL | |     }
    |
    = note: `-D clippy::manual-flatten` implied by `-D warnings`
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:7:9
+  --> $DIR/manual_flatten.rs:8: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
+  --> $DIR/manual_flatten.rs:15:5
    |
 LL |       for n in y.clone() {
    |       ^        --------- help: try: `y.clone().into_iter().flatten()`
@@ -34,15 +34,15 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:15:9
+  --> $DIR/manual_flatten.rs:16: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
+  --> $DIR/manual_flatten.rs:22:5
    |
 LL |       for n in &y {
    |       ^        -- help: try: `y.iter().flatten()`
@@ -55,15 +55,15 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:22:9
+  --> $DIR/manual_flatten.rs:23: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:31:5
+  --> $DIR/manual_flatten.rs:32:5
    |
 LL |       for n in z {
    |       ^        - help: try: `z.into_iter().flatten()`
@@ -76,15 +76,15 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:32:9
+  --> $DIR/manual_flatten.rs:33: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
+  --> $DIR/manual_flatten.rs:41:5
    |
 LL |       for n in z {
    |       ^        - help: try: `z.flatten()`
@@ -97,12 +97,75 @@ LL | |     }
    | |_____^
    |
 help: ...and remove the `if let` statement in the for loop
-  --> $DIR/manual_flatten.rs:41:9
+  --> $DIR/manual_flatten.rs:42:9
    |
 LL | /         if let Some(m) = n {
 LL | |             println!("{}", m);
 LL | |         }
    | |_________^
 
-error: aborting due to 5 previous errors
+error: unnecessary `if let` since only the `Some` variant of the iterator element is used
+  --> $DIR/manual_flatten.rs:74:5
+   |
+LL |       for n in &vec_of_ref {
+   |       ^        ----------- help: try: `vec_of_ref.iter().copied().flatten()`
+   |  _____|
+   | |
+LL | |         if let Some(n) = n {
+LL | |             println!("{:?}", n);
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: ...and remove the `if let` statement in the for loop
+  --> $DIR/manual_flatten.rs:75:9
+   |
+LL | /         if let Some(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:81:5
+   |
+LL |       for n in vec_of_ref {
+   |       ^        ---------- help: try: `vec_of_ref.into_iter().copied().flatten()`
+   |  _____|
+   | |
+LL | |         if let Some(n) = n {
+LL | |             println!("{:?}", n);
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: ...and remove the `if let` statement in the for loop
+  --> $DIR/manual_flatten.rs:82:9
+   |
+LL | /         if let Some(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:88:5
+   |
+LL |       for n in slice_of_ref {
+   |       ^        ------------ help: try: `slice_of_ref.into_iter().copied().flatten()`
+   |  _____|
+   | |
+LL | |         if let Some(n) = n {
+LL | |             println!("{:?}", n);
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: ...and remove the `if let` statement in the for loop
+  --> $DIR/manual_flatten.rs:89:9
+   |
+LL | /         if let Some(n) = n {
+LL | |             println!("{:?}", n);
+LL | |         }
+   | |_________^
+
+error: aborting due to 8 previous errors
 
