From eadd9d24dcd7c84c96dcd17234b8aadf9a113bd3 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 25 Apr 2020 20:51:02 +0200 Subject: [PATCH 1/2] Don't trigger while_let_on_iterator when the iterator is recreated every iteration --- clippy_lints/src/loops.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 56dd2795c609..6e9b7f685eb1 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -566,6 +566,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops { ) = (pat, &match_expr.kind) { let iter_expr = &method_args[0]; + + // Don't lint when the iterator is recreated on every iteration + if_chain! { + if let ExprKind::MethodCall(..) | ExprKind::Call(..) = iter_expr.kind; + if let Some(iter_def_id) = get_trait_def_id(cx, &paths::ITERATOR); + if implements_trait(cx, cx.tables.expr_ty(iter_expr), iter_def_id, &[]); + then { + return; + } + } + let lhs_constructor = last_path_segment(qpath); if method_path.ident.name == sym!(next) && match_trait_method(cx, match_expr, &paths::ITERATOR) From a1826222cf5f0298353cc9f9893e86babdaac770 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 25 Apr 2020 20:51:23 +0200 Subject: [PATCH 2/2] Add tests for #1654 --- tests/ui/while_let_on_iterator.fixed | 24 ++++++++++++++++++++---- tests/ui/while_let_on_iterator.rs | 24 ++++++++++++++++++++---- tests/ui/while_let_on_iterator.stderr | 8 +------- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed index 447b36048eed..f5fcabf63fd3 100644 --- a/tests/ui/while_let_on_iterator.fixed +++ b/tests/ui/while_let_on_iterator.fixed @@ -119,10 +119,7 @@ fn issue2965() { let mut values = HashSet::new(); values.insert(1); - for _ in values.iter() { - // FIXME(flip1995): Linting this with the following line uncommented is a FP, see #1654 - // values.remove(&1); - } + while let Some(..) = values.iter().next() {} } fn issue3670() { @@ -134,6 +131,24 @@ fn issue3670() { } } +fn issue1654() { + // should not lint if the iterator is generated on every iteration + use std::collections::HashSet; + let mut values = HashSet::new(); + values.insert(1); + + while let Some(..) = values.iter().next() { + values.remove(&1); + } + + while let Some(..) = values.iter().map(|x| x + 1).next() {} + + let chars = "Hello, World!".char_indices(); + while let Some((i, ch)) = chars.clone().next() { + println!("{}: {}", i, ch); + } +} + fn main() { base(); refutable(); @@ -141,4 +156,5 @@ fn main() { issue1121(); issue2965(); issue3670(); + issue1654(); } diff --git a/tests/ui/while_let_on_iterator.rs b/tests/ui/while_let_on_iterator.rs index 56a245aa8c7d..04dce8a02898 100644 --- a/tests/ui/while_let_on_iterator.rs +++ b/tests/ui/while_let_on_iterator.rs @@ -119,10 +119,7 @@ fn issue2965() { let mut values = HashSet::new(); values.insert(1); - while let Some(..) = values.iter().next() { - // FIXME(flip1995): Linting this with the following line uncommented is a FP, see #1654 - // values.remove(&1); - } + while let Some(..) = values.iter().next() {} } fn issue3670() { @@ -134,6 +131,24 @@ fn issue3670() { } } +fn issue1654() { + // should not lint if the iterator is generated on every iteration + use std::collections::HashSet; + let mut values = HashSet::new(); + values.insert(1); + + while let Some(..) = values.iter().next() { + values.remove(&1); + } + + while let Some(..) = values.iter().map(|x| x + 1).next() {} + + let chars = "Hello, World!".char_indices(); + while let Some((i, ch)) = chars.clone().next() { + println!("{}: {}", i, ch); + } +} + fn main() { base(); refutable(); @@ -141,4 +156,5 @@ fn main() { issue1121(); issue2965(); issue3670(); + issue1654(); } diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr index 94caedb43a51..6de138d7227b 100644 --- a/tests/ui/while_let_on_iterator.stderr +++ b/tests/ui/while_let_on_iterator.stderr @@ -24,11 +24,5 @@ error: this loop could be written as a `for` loop LL | while let Some(_) = y.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in y` -error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:122:5 - | -LL | while let Some(..) = values.iter().next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in values.iter()` - -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors