diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index 63560047578..d57588716a5 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/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) { - // The iterator expression will be used on the next iteration unless it is declared within the outer - // loop. + if let Some(e) = get_enclosing_loop_or_closure(cx.tcx, loop_expr) { + // The iterator expression will be used on the next iteration (for loops), or on the next call (for + // 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, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 769836aaf18..93212f97cb7 100644 --- a/clippy_utils/src/lib.rs +++ b/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. -pub fn get_enclosing_loop(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> { +/// Gets the loop or closure enclosing the given expression, if any. +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 { - kind: ExprKind::Loop(..), + e + @ + Expr { + kind: ExprKind::Loop(..) | ExprKind::Closure(..), .. }, ) => return Some(e), diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed index c3e2cf0c4a4..52e80ceee83 100644 --- a/tests/ui/while_let_on_iterator.fixed +++ b/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 { diff --git a/tests/ui/while_let_on_iterator.rs b/tests/ui/while_let_on_iterator.rs index 1717006a449..5078a3c9028 100644 --- a/tests/ui/while_let_on_iterator.rs +++ b/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() { diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr index eff559bef7e..cb0afeae15e 100644 --- a/tests/ui/while_let_on_iterator.stderr +++ b/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