Auto merge of #7690 - Jarcho:while_loop_by_ref, r=xFrednet

Change `while_let_on_iterator` suggestion to use `by_ref()`

It came up in the discussion #7659 that suggesting `iter.by_ref()` is a clearer suggestion than `&mut iter`. I personally think they're equivalent, but if `by_ref()` is clearer to people then that should be the suggestion.

changelog: Change `while_let_on_iterator` suggestion when using `&mut` to use `by_ref()`

Author: bors

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,
     );
 }

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<T: Iterator<Item = u32>> S<T> {
         fn f(&mut self) -> Option<u32> {
             // 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;
         }

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