about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-03-24 18:42:20 +0000
committerbors <bors@rust-lang.org>2021-03-24 18:42:20 +0000
commit917b538c6857892a925a4e5d7c7cd448ad3488ab (patch)
treec30a1ae6037bd81bb2460429ddc422407fdaed94
parent919a1a40fe4ec1118eece46b1f4c03c311ab8cda (diff)
parent2f8d71b9dc5cbb2a26ecd88f943c8a225e28108f (diff)
downloadrust-917b538c6857892a925a4e5d7c7cd448ad3488ab.tar.gz
rust-917b538c6857892a925a4e5d7c7cd448ad3488ab.zip
Auto merge of #6962 - TaKO8Ki:fix-false-positive-in-manual-flatten, r=llogiq
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`
-rw-r--r--clippy_lints/src/loops/manual_flatten.rs10
-rw-r--r--clippy_lints/src/methods/utils.rs3
-rw-r--r--tests/ui/manual_flatten.rs22
-rw-r--r--tests/ui/manual_flatten.stderr85
4 files changed, 106 insertions, 14 deletions
diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs
index be87f67d300..8d2b9cccba4 100644
--- 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,6 +55,13 @@ 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,
@@ -61,7 +69,7 @@ pub(super) fn check<'tcx>(
                     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
index 8de23e1bc6e..ac6b55396da 100644
--- 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
index cff68eca933..b5bd35a6878 100644
--- 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
index 855dd9130e2..be5f8a1d818 100644
--- 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,7 +13,7 @@ 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);
@@ -21,7 +21,7 @@ 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,7 +34,7 @@ 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);
@@ -42,7 +42,7 @@ 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,7 +55,7 @@ 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);
@@ -63,7 +63,7 @@ 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,7 +76,7 @@ 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);
@@ -84,7 +84,7 @@ 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