Fix suggestions in while_let_on_iterator for non-sized traits

Author: sergiogiro

clippy_lints/src/loops/while_let_on_iterator.rs

@@ -12,6 +12,7 @@ use rustc_hir::intravisit::{Visitor, walk_expr};
 use rustc_hir::{Closure, Expr, ExprKind, HirId, LetStmt, Mutability, UnOp};
 use rustc_lint::LateContext;
 use rustc_middle::hir::nested_filter::OnlyBodies;
+use rustc_middle::ty;
 use rustc_middle::ty::adjustment::Adjust;
 use rustc_span::Symbol;
 use rustc_span::symbol::sym;
@@ -43,26 +44,26 @@ pub(super) fn check<'tcx>(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
+        // passed by reference. 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 by_ref = if cx.typeck_results().expr_ty(iter_expr).ref_mutability() == Some(Mutability::Mut)
+        let iterator = snippet_with_applicability(cx, iter_expr.span, "_", &mut applicability);
+        let iterator_by_ref = if cx.typeck_results().expr_ty(iter_expr).ref_mutability() == Some(Mutability::Mut)
             || !iter_expr_struct.can_move
             || !iter_expr_struct.fields.is_empty()
             || needs_mutable_borrow(cx, &iter_expr_struct, expr)
         {
-            ".by_ref()"
+            make_iterator_snippet(cx, iter_expr, iterator)
         } else {
-            ""
+            iterator.to_string()
         };
 
-        let iterator = snippet_with_applicability(cx, iter_expr.span, "_", &mut applicability);
         span_lint_and_sugg(
             cx,
             WHILE_LET_ON_ITERATOR,
             expr.span.with_hi(let_expr.span.hi()),
             "this loop could be written as a `for` loop",
             "try",
-            format!("{loop_label}for {loop_var} in {iterator}{by_ref}"),
+            format!("{loop_label}for {loop_var} in {iterator_by_ref}"),
             applicability,
         );
     }
@@ -355,3 +356,21 @@ fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: &
             .is_break()
     }
 }
+
+/// Constructs the transformed iterator expression for the suggestion.
+/// Returns `iterator.by_ref()` unless the iterator type is a reference to an unsized type,
+/// in which case it returns `&mut *iterator`.
+fn make_iterator_snippet<'tcx>(cx: &LateContext<'tcx>, iter_expr: &Expr<'tcx>, iterator: impl Into<String>) -> String {
+    let iterator = iterator.into();
+    let ty = cx.typeck_results().expr_ty(iter_expr);
+
+    // Check if the type is a reference
+    if let ty::Ref(_, inner_ty, _) = ty.kind()
+        && !inner_ty.is_sized(cx.tcx, cx.typing_env())
+    {
+        return format!("&mut *{iterator}");
+    }
+
+    // Default: use by_ref by appending to the iterator
+    format!("{iterator}.by_ref()")
+}

tests/ui/while_let_on_iterator.fixed

@@ -492,6 +492,37 @@ fn issue13123() {
     }
 }
 
+fn issue16089() {
+    trait CertainTrait: Iterator<Item = u8> {
+        fn iter_over_self(&mut self) {
+            let mut a = 0;
+            for r in &mut *self {
+                //~^ while_let_on_iterator
+                a = r;
+            }
+            self.use_after_iter()
+        }
+
+        fn use_after_iter(&mut self) {}
+    }
+}
+
+fn issue16089_sized_trait_not_reborrowed() {
+    trait CertainTrait: Iterator<Item = u8> + Sized {
+        fn iter_over_self(&mut self) {
+            let mut a = 0;
+            // Check that the suggestion is just "self", since the trait is sized.
+            for r in self.by_ref() {
+                //~^ while_let_on_iterator
+                a = r;
+            }
+            self.use_after_iter()
+        }
+
+        fn use_after_iter(&mut self) {}
+    }
+}
+
 fn main() {
     let mut it = 0..20;
     for _ in it {

tests/ui/while_let_on_iterator.rs

@@ -492,6 +492,37 @@ fn issue13123() {
     }
 }
 
+fn issue16089() {
+    trait CertainTrait: Iterator<Item = u8> {
+        fn iter_over_self(&mut self) {
+            let mut a = 0;
+            while let Some(r) = self.next() {
+                //~^ while_let_on_iterator
+                a = r;
+            }
+            self.use_after_iter()
+        }
+
+        fn use_after_iter(&mut self) {}
+    }
+}
+
+fn issue16089_sized_trait_not_reborrowed() {
+    trait CertainTrait: Iterator<Item = u8> + Sized {
+        fn iter_over_self(&mut self) {
+            let mut a = 0;
+            // Check that the suggestion is just "self", since the trait is sized.
+            while let Some(r) = self.next() {
+                //~^ while_let_on_iterator
+                a = r;
+            }
+            self.use_after_iter()
+        }
+
+        fn use_after_iter(&mut self) {}
+    }
+}
+
 fn main() {
     let mut it = 0..20;
     while let Some(..) = it.next() {

tests/ui/while_let_on_iterator.stderr

@@ -164,10 +164,22 @@ LL |     'label: while let Some(n) = it.next() {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'label: for n in it`
 
 error: this loop could be written as a `for` loop
-  --> tests/ui/while_let_on_iterator.rs:497:5
+  --> tests/ui/while_let_on_iterator.rs:499:13
+   |
+LL |             while let Some(r) = self.next() {
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for r in &mut *self`
+
+error: this loop could be written as a `for` loop
+  --> tests/ui/while_let_on_iterator.rs:515:13
+   |
+LL |             while let Some(r) = self.next() {
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for r in self.by_ref()`
+
+error: this loop could be written as a `for` loop
+  --> tests/ui/while_let_on_iterator.rs:528:5
    |
 LL |     while let Some(..) = it.next() {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it`
 
-error: aborting due to 28 previous errors
+error: aborting due to 30 previous errors