Skip to content

Commit

Permalink
Auto merge of rust-lang#6962 - TaKO8Ki:fix-false-positive-in-manual-f…
Browse files Browse the repository at this point in the history
…latten, 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: rust-lang#6893

changelog: fix false positive in `manual_flatten`
  • Loading branch information
bors committed Mar 24, 2021
2 parents 919a1a4 + 2f8d71b commit 917b538
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 14 deletions.
10 changes: 9 additions & 1 deletion clippy_lints/src/loops/manual_flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/methods/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down
22 changes: 22 additions & 0 deletions tests/ui/manual_flatten.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
Expand Down
85 changes: 74 additions & 11 deletions tests/ui/manual_flatten.stderr
Original file line number Diff line number Diff line change
@@ -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()`
Expand All @@ -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()`
Expand All @@ -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()`
Expand All @@ -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()`
Expand All @@ -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()`
Expand All @@ -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

0 comments on commit 917b538

Please sign in to comment.