Auto merge of #5525 - flip1995:issue_1654, r=phansch

 Don't trigger while_let_on_iterator when the iterator is recreated every iteration

r? @phansch

Fixes #1654

changelog: Fix false positive in [`while_let_on_iterator`]

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)

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,11 +131,30 @@ 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();
     nested_loops();
     issue1121();
     issue2965();
     issue3670();
+    issue1654();
 }

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,11 +131,30 @@ 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();
     nested_loops();
     issue1121();
     issue2965();
     issue3670();
+    issue1654();
 }

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