while_let_on_iterator: consider all deref adjustments for by_ref

Author: sergiogiro

clippy_lints/src/loops/while_let_on_iterator.rs

@@ -12,7 +12,6 @@ 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;
@@ -52,9 +51,9 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
             || !iter_expr_struct.fields.is_empty()
             || needs_mutable_borrow(cx, &iter_expr_struct, expr)
         {
-            make_iterator_snippet(cx, iter_expr, iterator)
+            make_iterator_snippet(cx, iter_expr, &iterator)
         } else {
-            iterator.to_string()
+            iterator.into_owned()
         };
 
         span_lint_and_sugg(
@@ -358,17 +357,20 @@ fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: &
 }
 
 /// 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);
-
-    if let ty::Ref(_, inner_ty, _) = ty.kind()
-        && !inner_ty.is_sized(cx.tcx, cx.typing_env())
+/// Returns `iterator.by_ref()` unless the last deref adjustment targets an unsized type,
+/// in which case it applies all derefs (e.g., `&mut **iterator` or `&mut ***iterator`).
+fn make_iterator_snippet<'tcx>(cx: &LateContext<'tcx>, iter_expr: &Expr<'tcx>, iterator: &str) -> String {
+    if let Some((n, adjust)) = cx
+        .typeck_results()
+        .expr_adjustments(iter_expr)
+        .iter()
+        .take_while(|x| matches!(x.kind, Adjust::Deref(_)))
+        .enumerate()
+        .last()
+        && !adjust.target.is_sized(cx.tcx, cx.typing_env())
     {
-        return format!("&mut *{iterator}");
+        format!("&mut {:*<n$}{iterator}", '*', n = n + 1)
+    } else {
+        format!("{iterator}.by_ref()")
     }
-
-    format!("{iterator}.by_ref()")
 }

tests/ui/while_let_on_iterator.fixed

@@ -523,6 +523,79 @@ fn issue16089_sized_trait_not_reborrowed() {
     }
 }
 
+fn issue16089_nested_derefs() {
+    struct S<T>(T);
+    impl<T> core::ops::Deref for S<T> {
+        type Target = T;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T> core::ops::DerefMut for S<T> {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            &mut self.0
+        }
+    }
+
+    fn f(mut x: S<S<&mut dyn Iterator<Item = u32>>>) {
+        for _ in &mut ***x {}
+        //~^ while_let_on_iterator
+    }
+}
+
+fn issue16089_nested_derefs_last_not_sized() {
+    struct WithSize<T>(T);
+    impl<T> core::ops::Deref for WithSize<T> {
+        type Target = T;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T> core::ops::DerefMut for WithSize<T> {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            &mut self.0
+        }
+    }
+    // The suggestion must use `&mut **x`. Using `x.by_ref()` doesn't work in this
+    // case, since the last type adjustment for `x` in the expression `x.next()` is
+    // to dereference a `?Sized` trait.
+    fn f(mut x: WithSize<&mut dyn Iterator<Item = u32>>) {
+        for _ in &mut **x {}
+        //~^ while_let_on_iterator
+    }
+}
+
+fn issue16089_nested_derefs_last_sized() {
+    struct NoSize<T: ?Sized>(T);
+    impl<T: ?Sized> core::ops::Deref for NoSize<T> {
+        type Target = T;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T: ?Sized> core::ops::DerefMut for NoSize<T> {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            &mut self.0
+        }
+    }
+
+    struct SizedIter {}
+
+    impl Iterator for SizedIter {
+        type Item = u32;
+        fn next(&mut self) -> Option<u32> {
+            Some(0)
+        }
+    }
+
+    // We want the suggestion to be `x.by_ref()`. It works in this case since the last type
+    // adjustment for `x` in the expression `x.next()` is to dereference a Sized type.
+    fn f(mut x: NoSize<NoSize<SizedIter>>) {
+        for _ in x.by_ref() {}
+        //~^ while_let_on_iterator
+    }
+}
+
 fn main() {
     let mut it = 0..20;
     for _ in it {

tests/ui/while_let_on_iterator.rs

@@ -523,6 +523,79 @@ fn issue16089_sized_trait_not_reborrowed() {
     }
 }
 
+fn issue16089_nested_derefs() {
+    struct S<T>(T);
+    impl<T> core::ops::Deref for S<T> {
+        type Target = T;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T> core::ops::DerefMut for S<T> {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            &mut self.0
+        }
+    }
+
+    fn f(mut x: S<S<&mut dyn Iterator<Item = u32>>>) {
+        while let Some(_) = x.next() {}
+        //~^ while_let_on_iterator
+    }
+}
+
+fn issue16089_nested_derefs_last_not_sized() {
+    struct WithSize<T>(T);
+    impl<T> core::ops::Deref for WithSize<T> {
+        type Target = T;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T> core::ops::DerefMut for WithSize<T> {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            &mut self.0
+        }
+    }
+    // The suggestion must use `&mut **x`. Using `x.by_ref()` doesn't work in this
+    // case, since the last type adjustment for `x` in the expression `x.next()` is
+    // to dereference a `?Sized` trait.
+    fn f(mut x: WithSize<&mut dyn Iterator<Item = u32>>) {
+        while let Some(_) = x.next() {}
+        //~^ while_let_on_iterator
+    }
+}
+
+fn issue16089_nested_derefs_last_sized() {
+    struct NoSize<T: ?Sized>(T);
+    impl<T: ?Sized> core::ops::Deref for NoSize<T> {
+        type Target = T;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T: ?Sized> core::ops::DerefMut for NoSize<T> {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            &mut self.0
+        }
+    }
+
+    struct SizedIter {}
+
+    impl Iterator for SizedIter {
+        type Item = u32;
+        fn next(&mut self) -> Option<u32> {
+            Some(0)
+        }
+    }
+
+    // We want the suggestion to be `x.by_ref()`. It works in this case since the last type
+    // adjustment for `x` in the expression `x.next()` is to dereference a Sized type.
+    fn f(mut x: NoSize<NoSize<SizedIter>>) {
+        while let Some(_) = x.next() {}
+        //~^ while_let_on_iterator
+    }
+}
+
 fn main() {
     let mut it = 0..20;
     while let Some(..) = it.next() {

tests/ui/while_let_on_iterator.stderr

@@ -176,10 +176,28 @@ 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
+  --> tests/ui/while_let_on_iterator.rs:541:9
+   |
+LL |         while let Some(_) = x.next() {}
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in &mut ***x`
+
+error: this loop could be written as a `for` loop
+  --> tests/ui/while_let_on_iterator.rs:563:9
+   |
+LL |         while let Some(_) = x.next() {}
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in &mut **x`
+
+error: this loop could be written as a `for` loop
+  --> tests/ui/while_let_on_iterator.rs:594:9
+   |
+LL |         while let Some(_) = x.next() {}
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in x.by_ref()`
+
+error: this loop could be written as a `for` loop
+  --> tests/ui/while_let_on_iterator.rs:601:5
    |
 LL |     while let Some(..) = it.next() {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it`
 
-error: aborting due to 30 previous errors
+error: aborting due to 33 previous errors