From 3b8e85a5dccd47c7284b30ad29d3144d5c152235 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Thu, 25 Mar 2021 00:15:15 +0900 Subject: [PATCH] fix false positive in manual_flatten --- clippy_lints/src/loops/manual_flatten.rs | 10 ++- tests/ui/manual_flatten.rs | 22 ++++++ tests/ui/manual_flatten.stderr | 85 +++++++++++++++++++++--- 3 files changed, 105 insertions(+), 12 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/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