Auto merge of #7262 - Jarcho:while_let_on_iter_closure, r=xFrednet,flip1995

fix `while_let_on_iterator` suggestion in a closure

fixes: #7249

A future improvement would be to check if the closure is being used as `FnOnce`, in which case the original suggestion would be correct.

changelog: Suggest `&mut iter` inside a closure for `while_let_on_iterator`

clippy_lints/src/loops/while_let_on_iterator.rs

@@ -1,7 +1,9 @@
 use super::WHILE_LET_ON_ITERATOR;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::{get_enclosing_loop, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used};
+use clippy_utils::{
+    get_enclosing_loop_or_closure, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used,
+};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor};
@@ -315,9 +317,10 @@ fn needs_mutable_borrow(cx: &LateContext<'tcx>, iter_expr: &IterExpr, loop_expr:
         }
     }
 
-    if let Some(e) = get_enclosing_loop(cx.tcx, loop_expr) {
+    if let Some(e) = get_enclosing_loop_or_closure(cx.tcx, loop_expr) {
-        // The iterator expression will be used on the next iteration unless it is declared within the outer
+        // The iterator expression will be used on the next iteration (for loops), or on the next call (for
-        // loop.
+        // closures) unless it is declared within the enclosing expression. TODO: Check for closures
+        // used where an `FnOnce` type is expected.
         let local_id = match iter_expr.path {
             Res::Local(id) => id,
             _ => return true,

clippy_utils/src/lib.rs

@@ -861,14 +861,16 @@ pub fn get_enclosing_block<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Optio
     })
 }
 
-/// Gets the loop enclosing the given expression, if any.
+/// Gets the loop or closure enclosing the given expression, if any.
-pub fn get_enclosing_loop(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
+pub fn get_enclosing_loop_or_closure(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
     let map = tcx.hir();
     for (_, node) in map.parent_iter(expr.hir_id) {
         match node {
             Node::Expr(
-                e @ Expr {
+                e
-                    kind: ExprKind::Loop(..),
+                @
+                Expr {
+                    kind: ExprKind::Loop(..) | ExprKind::Closure(..),
                     ..
                 },
             ) => return Some(e),

tests/ui/while_let_on_iterator.fixed

@@ -320,6 +320,20 @@ fn issue1924() {
     println!("iterator field {}", it.1);
 }
 
+fn issue7249() {
+    let mut it = 0..10;
+    let mut x = || {
+        // Needs &mut, the closure can be called multiple times
+        for x in &mut it {
+            if x % 2 == 0 {
+                break;
+            }
+        }
+    };
+    x();
+    x();
+}
+
 fn main() {
     let mut it = 0..20;
     for _ in it {

tests/ui/while_let_on_iterator.rs

@@ -320,6 +320,20 @@ fn issue1924() {
     println!("iterator field {}", it.1);
 }
 
+fn issue7249() {
+    let mut it = 0..10;
+    let mut x = || {
+        // Needs &mut, the closure can be called multiple times
+        while let Some(x) = it.next() {
+            if x % 2 == 0 {
+                break;
+            }
+        }
+    };
+    x();
+    x();
+}
+
 fn main() {
     let mut it = 0..20;
     while let Some(..) = it.next() {

tests/ui/while_let_on_iterator.stderr

@@ -105,10 +105,16 @@ LL |     while let Some(n) = it.next() {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in &mut it`
 
 error: this loop could be written as a `for` loop
-  --> $DIR/while_let_on_iterator.rs:325:5
+  --> $DIR/while_let_on_iterator.rs:327:9
+   |
+LL |         while let Some(x) = it.next() {
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it`
+
+error: this loop could be written as a `for` loop
+  --> $DIR/while_let_on_iterator.rs:339:5
    |
 LL |     while let Some(..) = it.next() {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it`
 
-error: aborting due to 18 previous errors
+error: aborting due to 19 previous errors