Fix `explicit_into_iter_loop` with mutable references

6 files changed, 188 insertions, 77 deletions

diff --git a/clippy_lints/src/loops/explicit_into_iter_loop.rs b/clippy_lints/src/loops/explicit_into_iter_loop.rs
index 175e2b382e3..bf5e018ce92 100644
--- a/clippy_lints/src/loops/explicit_into_iter_loop.rs
+++ b/clippy_lints/src/loops/explicit_into_iter_loop.rs
@@ -5,15 +5,76 @@ use clippy_utils::source::snippet_with_applicability;
 use rustc_errors::Applicability;
 use rustc_hir::Expr;
 use rustc_lint::LateContext;
+use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
 use rustc_span::symbol::sym;
 
+#[derive(Clone, Copy)]
+enum AdjustKind {
+    None,
+    Borrow,
+    BorrowMut,
+    Reborrow,
+    ReborrowMut,
+}
+impl AdjustKind {
+    fn borrow(mutbl: AutoBorrowMutability) -> Self {
+        match mutbl {
+            AutoBorrowMutability::Not => Self::Borrow,
+            AutoBorrowMutability::Mut { .. } => Self::BorrowMut,
+        }
+    }
+
+    fn reborrow(mutbl: AutoBorrowMutability) -> Self {
+        match mutbl {
+            AutoBorrowMutability::Not => Self::Reborrow,
+            AutoBorrowMutability::Mut { .. } => Self::ReborrowMut,
+        }
+    }
+
+    fn display(self) -> &'static str {
+        match self {
+            Self::None => "",
+            Self::Borrow => "&",
+            Self::BorrowMut => "&mut ",
+            Self::Reborrow => "&*",
+            Self::ReborrowMut => "&mut *",
+        }
+    }
+}
+
 pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<'_>) {
-    let self_ty = cx.typeck_results().expr_ty(self_arg);
-    let self_ty_adjusted = cx.typeck_results().expr_ty_adjusted(self_arg);
-    if !(self_ty == self_ty_adjusted && is_trait_method(cx, call_expr, sym::IntoIterator)) {
+    if !is_trait_method(cx, call_expr, sym::IntoIterator) {
         return;
     }
 
+    let typeck = cx.typeck_results();
+    let self_ty = typeck.expr_ty(self_arg);
+    let adjust = match typeck.expr_adjustments(self_arg) {
+        [] => AdjustKind::None,
+        &[
+            Adjustment {
+                kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)),
+                ..
+            },
+        ] => AdjustKind::borrow(mutbl),
+        &[
+            Adjustment {
+                kind: Adjust::Deref(_), ..
+            },
+            Adjustment {
+                kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)),
+                target,
+            },
+        ] => {
+            if self_ty == target && matches!(mutbl, AutoBorrowMutability::Not) {
+                AdjustKind::None
+            } else {
+                AdjustKind::reborrow(mutbl)
+            }
+        },
+        _ => return,
+    };
+
     let mut applicability = Applicability::MachineApplicable;
     let object = snippet_with_applicability(cx, self_arg.span, "_", &mut applicability);
     span_lint_and_sugg(
@@ -23,7 +84,7 @@ pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<
         "it is more concise to loop over containers instead of using explicit \
             iteration methods",
         "to write this more concisely, try",
-        object.to_string(),
+        format!("{}{}", adjust.display(), object.to_string()),
         applicability,
     );
 }
diff --git a/clippy_lints/src/loops/explicit_iter_loop.rs b/clippy_lints/src/loops/explicit_iter_loop.rs
index ba66af62375..621f0e0adca 100644
--- a/clippy_lints/src/loops/explicit_iter_loop.rs
+++ b/clippy_lints/src/loops/explicit_iter_loop.rs
@@ -1,58 +1,39 @@
 use super::EXPLICIT_ITER_LOOP;
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::is_trait_method;
 use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::ty::{implements_trait_with_env, make_normalized_projection_with_regions, normalize_with_regions,
-    make_normalized_projection, implements_trait, is_copy};
+use clippy_utils::ty::{
+    implements_trait, implements_trait_with_env, is_copy, make_normalized_projection,
+    make_normalized_projection_with_regions, normalize_with_regions,
+};
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, Mutability};
 use rustc_lint::LateContext;
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
-use rustc_middle::ty::{self, EarlyBinder, TypeAndMut, Ty};
+use rustc_middle::ty::{self, EarlyBinder, Ty, TypeAndMut};
 use rustc_span::sym;
 
-pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<'_>, method_name: &str, msrv: &Msrv) {
-    let borrow_kind = match method_name {
-        "iter" | "iter_mut" => match is_ref_iterable(cx, self_arg, call_expr) {
-            Some((kind, ty)) => {
-                if let ty::Array(_, count) = *ty.peel_refs().kind() {
-                    if !ty.is_ref() {
-                        if !msrv.meets(msrvs::ARRAY_INTO_ITERATOR) {
-                            return;
-                        }
-                    } else if count.try_eval_target_usize(cx.tcx, cx.param_env).map_or(true, |x| x > 32)
-                        && !msrv.meets(msrvs::ARRAY_IMPL_ANY_LEN)
-                    {
-                        return
-                    }
-                }
-                kind
-            },
-            None => return,
-        },
-        "into_iter" if is_trait_method(cx, call_expr, sym::IntoIterator) => {
-            let receiver_ty = cx.typeck_results().expr_ty(self_arg);
-            let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(self_arg);
-            let ref_receiver_ty = cx.tcx.mk_ref(
-                cx.tcx.lifetimes.re_erased,
-                ty::TypeAndMut {
-                    ty: receiver_ty,
-                    mutbl: Mutability::Not,
-                },
-            );
-            if receiver_ty_adjusted == ref_receiver_ty {
-                AdjustKind::None
-            } else {
+pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<'_>, msrv: &Msrv) {
+    let Some((adjust, ty)) = is_ref_iterable(cx, self_arg, call_expr) else {
+        return;
+    };
+    if let ty::Array(_, count) = *ty.peel_refs().kind() {
+        if !ty.is_ref() {
+            if !msrv.meets(msrvs::ARRAY_INTO_ITERATOR) {
                 return;
             }
-        },
-        _ => return,
-    };
+        } else if count
+            .try_eval_target_usize(cx.tcx, cx.param_env)
+            .map_or(true, |x| x > 32)
+            && !msrv.meets(msrvs::ARRAY_IMPL_ANY_LEN)
+        {
+            return;
+        }
+    }
 
     let mut applicability = Applicability::MachineApplicable;
     let object = snippet_with_applicability(cx, self_arg.span, "_", &mut applicability);
-    let prefix = match borrow_kind {
+    let prefix = match adjust {
         AdjustKind::None => "",
         AdjustKind::Borrow => "&",
         AdjustKind::BorrowMut => "&mut ",
@@ -105,7 +86,11 @@ impl AdjustKind {
 
 /// Checks if an `iter` or `iter_mut` call returns `IntoIterator::IntoIter`. Returns how the
 /// argument needs to be adjusted.
-fn is_ref_iterable<'tcx>(cx: &LateContext<'tcx>, self_arg: &Expr<'_>, call_expr: &Expr<'_>) -> Option<(AdjustKind, Ty<'tcx>)> {
+fn is_ref_iterable<'tcx>(
+    cx: &LateContext<'tcx>,
+    self_arg: &Expr<'_>,
+    call_expr: &Expr<'_>,
+) -> Option<(AdjustKind, Ty<'tcx>)> {
     let typeck = cx.typeck_results();
     if let Some(trait_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
         && let Some(fn_id) = typeck.type_dependent_def_id(call_expr.hir_id)
@@ -158,10 +143,18 @@ fn is_ref_iterable<'tcx>(cx: &LateContext<'tcx>, self_arg: &Expr<'_>, call_expr:
 
         match adjustments {
             [] => Some((AdjustKind::None, self_ty)),
-            &[Adjustment { kind: Adjust::Deref(_), ..}, Adjustment { kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)), target }, ..] => {
+            &[
+                Adjustment { kind: Adjust::Deref(_), ..},
+                Adjustment {
+                    kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)),
+                    target,
+                },
+                ..
+            ] => {
                 if target != self_ty
                     && implements_trait(cx, target, trait_id, &[])
-                    && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
+                    && let Some(ty) =
+                        make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
                     && ty == res_ty
                 {
                     Some((AdjustKind::reborrow(mutbl), target))
@@ -172,7 +165,8 @@ fn is_ref_iterable<'tcx>(cx: &LateContext<'tcx>, self_arg: &Expr<'_>, call_expr:
             &[Adjustment { kind: Adjust::Deref(_), target }, ..] => {
                 if is_copy(cx, target)
                     && implements_trait(cx, target, trait_id, &[])
-                    && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
+                    && let Some(ty) =
+                        make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
                     && ty == res_ty
                 {
                     Some((AdjustKind::Deref, target))
@@ -180,10 +174,17 @@ fn is_ref_iterable<'tcx>(cx: &LateContext<'tcx>, self_arg: &Expr<'_>, call_expr:
                     None
                 }
             }
-            &[Adjustment { kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)), target }, ..] => {
+            &[
+                Adjustment {
+                    kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)),
+                    target,
+                },
+                ..
+            ] => {
                 if self_ty.is_ref()
                     && implements_trait(cx, target, trait_id, &[])
-                    && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
+                    && let Some(ty) =
+                        make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
                     && ty == res_ty
                 {
                     Some((AdjustKind::auto_borrow(mutbl), target))
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
index 068166bdeb2..1231fefb180 100644
--- a/clippy_lints/src/loops/mod.rs
+++ b/clippy_lints/src/loops/mod.rs
@@ -715,14 +715,11 @@ impl Loops {
 
     fn check_for_loop_arg(&self, cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {
         if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind {
-            let method_name = method.ident.as_str();
-            // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
-            match method_name {
+            match method.ident.as_str() {
                 "iter" | "iter_mut" => {
-                    explicit_iter_loop::check(cx, self_arg, arg, method_name, &self.msrv);
+                    explicit_iter_loop::check(cx, self_arg, arg, &self.msrv);
                 },
                 "into_iter" => {
-                    explicit_iter_loop::check(cx, self_arg, arg, method_name, &self.msrv);
                     explicit_into_iter_loop::check(cx, self_arg, arg);
                 },
                 "next" => {
diff --git a/tests/ui/explicit_into_iter_loop.fixed b/tests/ui/explicit_into_iter_loop.fixed
index bc9ecb1e05a..dcef6340311 100644
--- a/tests/ui/explicit_into_iter_loop.fixed
+++ b/tests/ui/explicit_into_iter_loop.fixed
@@ -7,9 +7,7 @@ fn main() {
     where
         for<'a> &'a T: IntoIterator<Item = &'a String>,
     {
-        for i in iterator {
-            println!("{}", i);
-        }
+        for _ in iterator {}
     }
 
     struct T;
@@ -17,23 +15,39 @@ fn main() {
         type Item = ();
         type IntoIter = std::vec::IntoIter<Self::Item>;
         fn into_iter(self) -> Self::IntoIter {
-            vec![].into_iter()
+            unimplemented!()
         }
     }
 
-    let t = T;
-    let r = &t;
-    let rr = &&t;
-
-    // This case is handled by `explicit_iter_loop`. No idea why.
-    for _ in t.into_iter() {}
+    let mut t = T;
+    for _ in &t {}
 
+    let r = &t;
     for _ in r {}
 
     // No suggestion for this.
     // We'd have to suggest `for _ in *rr {}` which is less clear.
+    let rr = &&t;
     for _ in rr.into_iter() {}
 
+    let mr = &mut t;
+    for _ in &*mr {}
+
+    struct U;
+    impl IntoIterator for &mut U {
+        type Item = ();
+        type IntoIter = std::vec::IntoIter<Self::Item>;
+        fn into_iter(self) -> Self::IntoIter {
+            unimplemented!()
+        }
+    }
+
+    let mut u = U;
+    for _ in &mut u {}
+
+    let mr = &mut u;
+    for _ in &mut *mr {}
+
     // Issue #6900
     struct S;
     impl S {
diff --git a/tests/ui/explicit_into_iter_loop.rs b/tests/ui/explicit_into_iter_loop.rs
index 9b610fe3d11..bc048ed302b 100644
--- a/tests/ui/explicit_into_iter_loop.rs
+++ b/tests/ui/explicit_into_iter_loop.rs
@@ -7,9 +7,7 @@ fn main() {
     where
         for<'a> &'a T: IntoIterator<Item = &'a String>,
     {
-        for i in iterator.into_iter() {
-            println!("{}", i);
-        }
+        for _ in iterator.into_iter() {}
     }
 
     struct T;
@@ -17,23 +15,39 @@ fn main() {
         type Item = ();
         type IntoIter = std::vec::IntoIter<Self::Item>;
         fn into_iter(self) -> Self::IntoIter {
-            vec![].into_iter()
+            unimplemented!()
         }
     }
 
-    let t = T;
-    let r = &t;
-    let rr = &&t;
-
-    // This case is handled by `explicit_iter_loop`. No idea why.
+    let mut t = T;
     for _ in t.into_iter() {}
 
+    let r = &t;
     for _ in r.into_iter() {}
 
     // No suggestion for this.
     // We'd have to suggest `for _ in *rr {}` which is less clear.
+    let rr = &&t;
     for _ in rr.into_iter() {}
 
+    let mr = &mut t;
+    for _ in mr.into_iter() {}
+
+    struct U;
+    impl IntoIterator for &mut U {
+        type Item = ();
+        type IntoIter = std::vec::IntoIter<Self::Item>;
+        fn into_iter(self) -> Self::IntoIter {
+            unimplemented!()
+        }
+    }
+
+    let mut u = U;
+    for _ in u.into_iter() {}
+
+    let mr = &mut u;
+    for _ in mr.into_iter() {}
+
     // Issue #6900
     struct S;
     impl S {
diff --git a/tests/ui/explicit_into_iter_loop.stderr b/tests/ui/explicit_into_iter_loop.stderr
index 1bd2b38a0e7..fa89b884fa0 100644
--- a/tests/ui/explicit_into_iter_loop.stderr
+++ b/tests/ui/explicit_into_iter_loop.stderr
@@ -1,16 +1,40 @@
 error: it is more concise to loop over containers instead of using explicit iteration methods
   --> $DIR/explicit_into_iter_loop.rs:10:18
    |
-LL |         for i in iterator.into_iter() {
+LL |         for _ in iterator.into_iter() {}
    |                  ^^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `iterator`
    |
    = note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings`
 
 error: it is more concise to loop over containers instead of using explicit iteration methods
-  --> $DIR/explicit_into_iter_loop.rs:31:14
+  --> $DIR/explicit_into_iter_loop.rs:23:14
+   |
+LL |     for _ in t.into_iter() {}
+   |              ^^^^^^^^^^^^^ help: to write this more concisely, try: `&t`
+
+error: it is more concise to loop over containers instead of using explicit iteration methods
+  --> $DIR/explicit_into_iter_loop.rs:26:14
    |
 LL |     for _ in r.into_iter() {}
    |              ^^^^^^^^^^^^^ help: to write this more concisely, try: `r`
 
-error: aborting due to 2 previous errors
+error: it is more concise to loop over containers instead of using explicit iteration methods
+  --> $DIR/explicit_into_iter_loop.rs:34:14
+   |
+LL |     for _ in mr.into_iter() {}
+   |              ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&*mr`
+
+error: it is more concise to loop over containers instead of using explicit iteration methods
+  --> $DIR/explicit_into_iter_loop.rs:46:14
+   |
+LL |     for _ in u.into_iter() {}
+   |              ^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut u`
+
+error: it is more concise to loop over containers instead of using explicit iteration methods
+  --> $DIR/explicit_into_iter_loop.rs:49:14
+   |
+LL |     for _ in mr.into_iter() {}
+   |              ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut *mr`
+
+error: aborting due to 6 previous errors