diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index 79527e3bfa9..9a3475c977e 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -8,7 +8,7 @@ use clippy_utils::{ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; -use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, Mutability, PatKind, QPath, UnOp}; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, PatKind, QPath, UnOp}; use rustc_lint::LateContext; use rustc_span::{symbol::sym, Span, Symbol}; @@ -47,13 +47,8 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // If the iterator is a field or the iterator is accessed after the loop is complete it needs to be // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used // afterwards a mutable borrow of a field isn't necessary. - let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { - if cx.typeck_results().node_type(iter_expr.hir_id).ref_mutability() == Some(Mutability::Mut) { - // Reborrow for mutable references. It may not be possible to get a mutable reference here. - "&mut *" - } else { - "&mut " - } + let by_ref = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { + ".by_ref()" } else { "" }; @@ -65,7 +60,7 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr.span.with_hi(scrutinee_expr.span.hi()), "this loop could be written as a `for` loop", "try", - format!("for {} in {}{}", loop_var, ref_mut, iterator), + format!("for {} in {}{}", loop_var, iterator, by_ref), applicability, ); } diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed index cdcdd808c94..f5a34190902 100644 --- a/tests/ui/while_let_on_iterator.fixed +++ b/tests/ui/while_let_on_iterator.fixed @@ -188,7 +188,7 @@ fn issue6491() { // Used in outer loop, needs &mut let mut it = 1..40; while let Some(n) = it.next() { - for m in &mut it { + for m in it.by_ref() { if m % 10 == 0 { break; } @@ -219,7 +219,7 @@ fn issue6491() { // Used after the loop, needs &mut. let mut it = 1..40; - for m in &mut it { + for m in it.by_ref() { if m % 10 == 0 { break; } @@ -236,7 +236,7 @@ fn issue6231() { let mut it = 1..40; let mut opt = Some(0); while let Some(n) = opt.take().or_else(|| it.next()) { - for m in &mut it { + for m in it.by_ref() { if n % 10 == 0 { break; } @@ -251,7 +251,7 @@ fn issue1924() { impl> S { fn f(&mut self) -> Option { // Used as a field. - for i in &mut self.0 { + for i in self.0.by_ref() { if !(3..=7).contains(&i) { return Some(i); } @@ -283,7 +283,7 @@ fn issue1924() { } } // This one is fine, a different field is borrowed - for i in &mut self.0.0.0 { + for i in self.0.0.0.by_ref() { if i == 1 { return self.0.1.take(); } else { @@ -312,7 +312,7 @@ fn issue1924() { // Needs &mut, field of the iterator is accessed after the loop let mut it = S2(1..40, 0); - for n in &mut it { + for n in it.by_ref() { if n == 0 { break; } @@ -324,7 +324,7 @@ fn issue7249() { let mut it = 0..10; let mut x = || { // Needs &mut, the closure can be called multiple times - for x in &mut it { + for x in it.by_ref() { if x % 2 == 0 { break; } @@ -338,7 +338,7 @@ fn issue7510() { let mut it = 0..10; let it = &mut it; // Needs to reborrow `it` as the binding isn't mutable - for x in &mut *it { + for x in it.by_ref() { if x % 2 == 0 { break; } @@ -349,7 +349,7 @@ fn issue7510() { let mut it = 0..10; let it = S(&mut it); // Needs to reborrow `it.0` as the binding isn't mutable - for x in &mut *it.0 { + for x in it.0.by_ref() { if x % 2 == 0 { break; } diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr index ff9b08996da..5e2fce4491a 100644 --- a/tests/ui/while_let_on_iterator.stderr +++ b/tests/ui/while_let_on_iterator.stderr @@ -46,7 +46,7 @@ error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:191:9 | LL | while let Some(m) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:202:5 @@ -70,19 +70,19 @@ error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:222:9 | LL | while let Some(m) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:239:9 | LL | while let Some(m) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:254:13 | LL | while let Some(i) = self.0.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in &mut self.0` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in self.0.by_ref()` error: manual `!RangeInclusive::contains` implementation --> $DIR/while_let_on_iterator.rs:255:20 @@ -96,31 +96,31 @@ error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:286:13 | LL | while let Some(i) = self.0.0.0.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in &mut self.0.0.0` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in self.0.0.0.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:315:5 | LL | while let Some(n) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:327:9 | LL | while let Some(x) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:341:5 | LL | while let Some(x) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:352:5 | LL | while let Some(x) = it.0.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it.0` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.0.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:371:5