Reborrow mutable references in explicit_iter_loop

Jarcho · Jarcho · commit 949712c90ac2 · 2023-06-09T21:40:03.000-04:00
diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs
@@ -808,7 +808,7 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msr
 }
 
 fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
-    for item in items.iter() {
+    for item in items {
         if let NestedMetaItem::MetaItem(meta) = item {
             if !meta.has_name(sym::any) && !meta.has_name(sym::all) {
                 continue;
@@ -842,7 +842,7 @@ fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
 }
 
 fn check_nested_misused_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
-    for item in items.iter() {
+    for item in items {
         if let NestedMetaItem::MetaItem(meta) = item {
             if meta.has_name(sym!(features)) && let Some(val) = meta.value_str() {
                 span_lint_and_sugg(
diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs
@@ -161,7 +161,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
                         let fields_def = &variant.fields;
 
                         // Push field type then visit each field expr.
-                        for field in fields.iter() {
+                        for field in *fields {
                             let bound =
                                 fields_def
                                     .iter()
diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs
@@ -562,7 +562,7 @@ fn has_where_lifetimes<'tcx>(cx: &LateContext<'tcx>, generics: &'tcx Generics<'_
                 // if the bounds define new lifetimes, they are fine to occur
                 let allowed_lts = allowed_lts_from(pred.bound_generic_params);
                 // now walk the bounds
-                for bound in pred.bounds.iter() {
+                for bound in pred.bounds {
                     walk_param_bound(&mut visitor, bound);
                 }
                 // and check that all lifetimes are allowed
diff --git a/clippy_lints/src/loops/explicit_into_iter_loop.rs b/clippy_lints/src/loops/explicit_into_iter_loop.rs
@@ -84,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",
-        format!("{}{}", adjust.display(), object.to_string()),
+        format!("{}{object}", adjust.display()),
         applicability,
     );
 }
diff --git a/clippy_lints/src/loops/explicit_iter_loop.rs b/clippy_lints/src/loops/explicit_iter_loop.rs
@@ -33,26 +33,19 @@ pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<
 
     let mut applicability = Applicability::MachineApplicable;
     let object = snippet_with_applicability(cx, self_arg.span, "_", &mut applicability);
-    let prefix = match adjust {
-        AdjustKind::None => "",
-        AdjustKind::Borrow => "&",
-        AdjustKind::BorrowMut => "&mut ",
-        AdjustKind::Deref => "*",
-        AdjustKind::Reborrow => "&*",
-        AdjustKind::ReborrowMut => "&mut *",
-    };
     span_lint_and_sugg(
         cx,
         EXPLICIT_ITER_LOOP,
         call_expr.span,
         "it is more concise to loop over references to containers instead of using explicit \
          iteration methods",
         "to write this more concisely, try",
-        format!("{prefix}{object}"),
+        format!("{}{object}", adjust.display()),
         applicability,
     );
 }
 
+#[derive(Clone, Copy)]
 enum AdjustKind {
     None,
     Borrow,
@@ -76,16 +69,35 @@ impl AdjustKind {
         }
     }
 
-    fn reborrow(mutbl: AutoBorrowMutability) -> Self {
+    fn reborrow(mutbl: Mutability) -> Self {
+        match mutbl {
+            Mutability::Not => Self::Reborrow,
+            Mutability::Mut => Self::ReborrowMut,
+        }
+    }
+
+    fn auto_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::Deref => "*",
+            Self::Reborrow => "&*",
+            Self::ReborrowMut => "&mut *",
+        }
+    }
 }
 
 /// Checks if an `iter` or `iter_mut` call returns `IntoIterator::IntoIter`. Returns how the
 /// argument needs to be adjusted.
+#[expect(clippy::too_many_lines)]
 fn is_ref_iterable<'tcx>(
     cx: &LateContext<'tcx>,
     self_arg: &Expr<'_>,
@@ -108,27 +120,50 @@ fn is_ref_iterable<'tcx>(
         let self_is_copy = is_copy(cx, self_ty);
 
         if adjustments.is_empty() && self_is_copy {
+            // Exact type match, already checked earlier
             return Some((AdjustKind::None, self_ty));
         }
 
-        let res_ty = cx.tcx.erase_regions(EarlyBinder::bind(req_res_ty).subst(cx.tcx, typeck.node_substs(call_expr.hir_id)));
-        if !adjustments.is_empty() && self_is_copy {
-            if implements_trait(cx, self_ty, trait_id, &[])
-                && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [self_ty])
-                && ty == res_ty
-            {
-                return Some((AdjustKind::None, self_ty));
-            }
-        }
-
+        let res_ty = cx.tcx.erase_regions(EarlyBinder::bind(req_res_ty)
+            .subst(cx.tcx, typeck.node_substs(call_expr.hir_id)));
         let mutbl = if let ty::Ref(_, _, mutbl) = *req_self_ty.kind() {
             Some(mutbl)
         } else {
             None
         };
+
+        if !adjustments.is_empty() {
+            if self_is_copy {
+                // Using by value won't consume anything
+                if implements_trait(cx, self_ty, trait_id, &[])
+                    && let Some(ty) =
+                        make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [self_ty])
+                    && ty == res_ty
+                {
+                    return Some((AdjustKind::None, self_ty));
+                }
+            } else if let ty::Ref(region, ty, Mutability::Mut) = *self_ty.kind()
+                && let Some(mutbl) = mutbl
+            {
+                // Attempt to reborrow the mutable reference
+                let self_ty = if mutbl.is_mut() {
+                    self_ty
+                } else {
+                    cx.tcx.mk_ref(region, TypeAndMut { ty, mutbl })
+                };
+                if implements_trait(cx, self_ty, trait_id, &[])
+                    && let Some(ty) =
+                        make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [self_ty])
+                    && ty == res_ty
+                {
+                    return Some((AdjustKind::reborrow(mutbl), self_ty));
+                }
+            }
+        }
         if let Some(mutbl) = mutbl
             && !self_ty.is_ref()
         {
+            // Attempt to borrow
             let self_ty = cx.tcx.mk_ref(cx.tcx.lifetimes.re_erased, TypeAndMut {
                 ty: self_ty,
                 mutbl,
@@ -157,7 +192,7 @@ fn is_ref_iterable<'tcx>(
                         make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
                     && ty == res_ty
                 {
-                    Some((AdjustKind::reborrow(mutbl), target))
+                    Some((AdjustKind::auto_reborrow(mutbl), target))
                 } else {
                     None
                 }
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
@@ -687,6 +687,8 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
             manual_while_let_some::check(cx, condition, body, span);
         }
     }
+
+    extract_msrv_attr!(LateContext);
 }
 
 impl Loops {
diff --git a/clippy_lints/src/loops/same_item_push.rs b/clippy_lints/src/loops/same_item_push.rs
@@ -148,7 +148,7 @@ impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> {
     }
 
     fn visit_block(&mut self, b: &'tcx Block<'_>) {
-        for stmt in b.stmts.iter() {
+        for stmt in b.stmts {
             self.visit_stmt(stmt);
         }
     }
diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs
@@ -101,7 +101,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
             });
             if !id.is_local();
             then {
-                for kid in cx.tcx.module_children(id).iter() {
+                for kid in cx.tcx.module_children(id) {
                     if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res {
                         let span = mac_attr.span;
                         let def_path = cx.tcx.def_path_str(mac_id);
diff --git a/clippy_lints/src/matches/match_wild_err_arm.rs b/clippy_lints/src/matches/match_wild_err_arm.rs
@@ -25,7 +25,7 @@ pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm<'
                     let mut ident_bind_name = kw::Underscore;
                     if !matching_wild {
                         // Looking for unused bindings (i.e.: `_e`)
-                        for pat in inner.iter() {
+                        for pat in inner {
                             if let PatKind::Binding(_, id, ident, None) = pat.kind {
                                 if ident.as_str().starts_with('_') && !is_local_used(cx, arm.body, id) {
                                     ident_bind_name = ident.name;
diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs
@@ -140,7 +140,7 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
                     }
                 }
 
-                for generic_arg in b.iter() {
+                for generic_arg in *b {
                     if let GenericArgKind::Type(ty) = generic_arg.unpack() {
                         if self.has_sig_drop_attr(cx, ty) {
                             return true;
diff --git a/clippy_lints/src/matches/try_err.rs b/clippy_lints/src/matches/try_err.rs
@@ -81,7 +81,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine
 /// Finds function return type by examining return expressions in match arms.
 fn find_return_type<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx ExprKind<'_>) -> Option<Ty<'tcx>> {
     if let ExprKind::Match(_, arms, MatchSource::TryDesugar) = expr {
-        for arm in arms.iter() {
+        for arm in *arms {
             if let ExprKind::Ret(Some(ret)) = arm.body.kind {
                 return Some(cx.typeck_results().expr_ty(ret));
             }
diff --git a/clippy_lints/src/misc_early/mixed_case_hex_literals.rs b/clippy_lints/src/misc_early/mixed_case_hex_literals.rs
@@ -13,7 +13,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, lit_span: Span, suffix: &str, lit_sni
         return;
     }
     let mut seen = (false, false);
-    for ch in lit_snip.as_bytes()[2..=maybe_last_sep_idx].iter() {
+    for ch in &lit_snip.as_bytes()[2..=maybe_last_sep_idx] {
         match ch {
             b'a'..=b'f' => seen.0 = true,
             b'A'..=b'F' => seen.1 = true,
diff --git a/clippy_lints/src/mismatching_type_param_order.rs b/clippy_lints/src/mismatching_type_param_order.rs
@@ -59,7 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeParamMismatch {
             then {
                 // get the name and span of the generic parameters in the Impl
                 let mut impl_params = Vec::new();
-                for p in generic_args.args.iter() {
+                for p in generic_args.args {
                     match p {
                         GenericArg::Type(Ty {kind: TyKind::Path(QPath::Resolved(_, path)), ..}) =>
                             impl_params.push((path.segments[0].ident.to_string(), path.span)),
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
@@ -292,7 +292,7 @@ fn check_final_expr<'tcx>(
         // (except for unit type functions) so we don't match it
         ExprKind::Match(_, arms, MatchSource::Normal) => {
             let match_ty = cx.typeck_results().expr_ty(peeled_drop_expr);
-            for arm in arms.iter() {
+            for arm in *arms {
                 check_final_expr(cx, arm.body, semi_spans.clone(), RetReplacement::Unit, Some(match_ty));
             }
         },
diff --git a/clippy_lints/src/significant_drop_tightening.rs b/clippy_lints/src/significant_drop_tightening.rs
@@ -333,7 +333,7 @@ impl<'cx, 'sdt, 'tcx> SigDropChecker<'cx, 'sdt, 'tcx> {
                         return true;
                     }
                 }
-                for generic_arg in b.iter() {
+                for generic_arg in *b {
                     if let GenericArgKind::Type(ty) = generic_arg.unpack() {
                         if self.has_sig_drop_attr(ty) {
                             return true;
diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs
@@ -139,7 +139,7 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
                     ) = cx.tcx.hir().get_if_local(*def_id);
                 then {
                     if self_bounds_map.is_empty() {
-                        for bound in self_bounds.iter() {
+                        for bound in *self_bounds {
                             let Some((self_res, self_segments, _)) = get_trait_info_from_bound(bound) else { continue };
                             self_bounds_map.insert(self_res, self_segments);
                         }
@@ -184,7 +184,7 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
 
                 // Iterate the bounds and add them to our seen hash
                 // If we haven't yet seen it, add it to the fixed traits
-                for bound in bounds.iter() {
+                for bound in bounds {
                     let Some(def_id) = bound.trait_ref.trait_def_id() else { continue; };
 
                     let new_trait = seen_def_ids.insert(def_id);
diff --git a/clippy_lints/src/utils/internal_lints/interning_defined_symbol.rs b/clippy_lints/src/utils/internal_lints/interning_defined_symbol.rs
@@ -75,7 +75,7 @@ impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
 
         for &module in &[&paths::KW_MODULE, &paths::SYM_MODULE] {
             for def_id in def_path_def_ids(cx, module) {
-                for item in cx.tcx.module_children(def_id).iter() {
+                for item in cx.tcx.module_children(def_id) {
                     if_chain! {
                         if let Res::Def(DefKind::Const, item_def_id) = item.res;
                         let ty = cx.tcx.type_of(item_def_id).subst_identity();
diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs
@@ -61,7 +61,7 @@ pub fn is_min_const_fn<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, msrv: &Msrv)
         body.local_decls.iter().next().unwrap().source_info.span,
     )?;
 
-    for bb in body.basic_blocks.iter() {
+    for bb in &*body.basic_blocks {
         check_terminator(tcx, body, bb.terminator(), msrv)?;
         for stmt in &bb.statements {
             check_statement(tcx, body, def_id, stmt)?;
@@ -89,7 +89,7 @@ fn check_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, span: Span) -> McfResult {
                 return Err((span, "function pointers in const fn are unstable".into()));
             },
             ty::Dynamic(preds, _, _) => {
-                for pred in preds.iter() {
+                for pred in *preds {
                     match pred.skip_binder() {
                         ty::ExistentialPredicate::AutoTrait(_) | ty::ExistentialPredicate::Projection(_) => {
                             return Err((
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
@@ -277,7 +277,7 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
             false
         },
         ty::Dynamic(binder, _, _) => {
-            for predicate in binder.iter() {
+            for predicate in *binder {
                 if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder() {
                     if cx.tcx.has_attr(trait_ref.def_id, sym::must_use) {
                         return true;
diff --git a/clippy_utils/src/usage.rs b/clippy_utils/src/usage.rs
@@ -82,7 +82,7 @@ pub struct ParamBindingIdCollector {
 impl<'tcx> ParamBindingIdCollector {
     fn collect_binding_hir_ids(body: &'tcx hir::Body<'tcx>) -> Vec<hir::HirId> {
         let mut hir_ids: Vec<hir::HirId> = Vec::new();
-        for param in body.params.iter() {
+        for param in body.params {
             let mut finder = ParamBindingIdCollector {
                 binding_hir_ids: Vec::new(),
             };
diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs
@@ -474,7 +474,7 @@ fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) {
             });
         } else if let Some(ref versions) = tk.versions {
             // if we have multiple versions, save each one
-            for ver in versions.iter() {
+            for ver in versions {
                 crate_sources.push(CrateSource::CratesIo {
                     name: tk.name.clone(),
                     version: ver.to_string(),
diff --git a/tests/ui/explicit_iter_loop.fixed b/tests/ui/explicit_iter_loop.fixed
@@ -17,6 +17,13 @@ fn main() {
     for _ in &vec {}
     for _ in &mut vec {}
 
+    let rvec = &vec;
+    for _ in rvec {}
+
+    let rmvec = &mut vec;
+    for _ in &*rmvec {}
+    for _ in &mut *rmvec {}
+
     for _ in &vec {} // these are fine
     for _ in &mut vec {} // these are fine
 
@@ -29,9 +36,13 @@ fn main() {
 
     let ll: LinkedList<()> = LinkedList::new();
     for _ in &ll {}
+    let rll = &ll;
+    for _ in rll {}
 
     let vd: VecDeque<()> = VecDeque::new();
     for _ in &vd {}
+    let rvd = &vd;
+    for _ in rvd {}
 
     let bh: BinaryHeap<()> = BinaryHeap::new();
     for _ in &bh {}
@@ -137,4 +148,7 @@ fn main() {
     let mut x = CustomType;
     for _ in &x {}
     for _ in &mut x {}
+
+    let r = &x;
+    for _ in r {}
 }
diff --git a/tests/ui/explicit_iter_loop.rs b/tests/ui/explicit_iter_loop.rs
diff --git a/tests/ui/explicit_iter_loop.stderr b/tests/ui/explicit_iter_loop.stderr

Original file line number	Diff line number	Diff line change
`@@ -562,7 +562,7 @@ fn has_where_lifetimes<'tcx>(cx: &LateContext<'tcx>, generics: &'tcx Generics<'_`
`562`	`562`	`// if the bounds define new lifetimes, they are fine to occur`
`563`	`563`	`let allowed_lts = allowed_lts_from(pred.bound_generic_params);`
`564`	`564`	`// now walk the bounds`
`565`		`- for bound in pred.bounds.iter() {`
	`565`	`+ for bound in pred.bounds {`
`566`	`566`	`walk_param_bound(&mut visitor, bound);`
`567`	`567`	`}`
`568`	`568`	`// and check that all lifetimes are allowed`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<`
`84`	`84`	`"it is more concise to loop over containers instead of using explicit \`
`85`	`85`	`iteration methods",`
`86`	`86`	`"to write this more concisely, try",`
`87`		`- format!("{}{}", adjust.display(), object.to_string()),`
	`87`	`+ format!("{}{object}", adjust.display()),`
`88`	`88`	`applicability,`
`89`	`89`	`);`
`90`	`90`	`}`
Original file line number	Diff line number	Diff line change
`@@ -687,6 +687,8 @@ impl<'tcx> LateLintPass<'tcx> for Loops {`
`687`	`687`	`manual_while_let_some::check(cx, condition, body, span);`
`688`	`688`	`}`
`689`	`689`	`}`
	`690`	`+`
	`691`	`+ extract_msrv_attr!(LateContext);`
`690`	`692`	`}`
`691`	`693`
`692`	`694`	`impl Loops {`
Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> {`
`148`	`148`	`}`
`149`	`149`
`150`	`150`	`fn visit_block(&mut self, b: &'tcx Block<'_>) {`
`151`		`- for stmt in b.stmts.iter() {`
	`151`	`+ for stmt in b.stmts {`
`152`	`152`	`self.visit_stmt(stmt);`
`153`	`153`	`}`
`154`	`154`	`}`
Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,7 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {`
`140`	`140`	`}`
`141`	`141`	`}`
`142`	`142`
`143`		`- for generic_arg in b.iter() {`
	`143`	`+ for generic_arg in *b {`
`144`	`144`	`if let GenericArgKind::Type(ty) = generic_arg.unpack() {`
`145`	`145`	`if self.has_sig_drop_attr(cx, ty) {`
`146`	`146`	`return true;`