From dd4b5cbaa1d0a768c14f688062abc151d6eedb50 Mon Sep 17 00:00:00 2001
From: shreyymeow <shreyymeoww@gmail.com>
Date: Mon, 8 Dec 2025 20:18:39 +0530
Subject: [PATCH] loops: Fix multi-part counter detection in
 explicit_counter_loop and manual_memcpy

---
 .../src/loops/explicit_counter_loop.rs        |  90 ++++++---
 clippy_lints/src/loops/manual_memcpy.rs       |  18 +-
 clippy_lints/src/loops/utils.rs               |  76 +++----
 tests/ui/explicit_counter_loop.rs             |   3 -
 tests/ui/explicit_counter_loop.stderr         | 187 +++++++++++++++---
 5 files changed, 249 insertions(+), 125 deletions(-)

diff --git a/clippy_lints/src/loops/explicit_counter_loop.rs b/clippy_lints/src/loops/explicit_counter_loop.rs
index 9aa4d2f0adc2..983804713e44 100644
--- a/clippy_lints/src/loops/explicit_counter_loop.rs
+++ b/clippy_lints/src/loops/explicit_counter_loop.rs
@@ -1,5 +1,5 @@
 use super::{EXPLICIT_COUNTER_LOOP, IncrementVisitor, InitializeVisitor, make_iterator_snippet};
-use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{get_enclosing_block, is_integer_const};
 use rustc_ast::Label;
@@ -12,6 +12,8 @@ use rustc_middle::ty::{self, Ty, UintTy};
 // To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be
 // incremented exactly once in the loop body, and initialized to zero
 // at the start of the loop.
+// ... (imports and prelude)
+
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     pat: &'tcx Pat<'_>,
@@ -20,66 +22,90 @@ pub(super) fn check<'tcx>(
     expr: &'tcx Expr<'_>,
     label: Option<Label>,
 ) {
-    // Look for variables that are incremented once per loop iteration.
     let mut increment_visitor = IncrementVisitor::new(cx);
     walk_expr(&mut increment_visitor, body);
 
-    // For each candidate, check the parent block to see if
-    // it's initialized to zero at the start of the loop.
     if let Some(block) = get_enclosing_block(cx, expr.hir_id) {
-        for id in increment_visitor.into_results() {
+        for (id, increment_span) in increment_visitor.into_results() {
             let mut initialize_visitor = InitializeVisitor::new(cx, expr, id);
             walk_block(&mut initialize_visitor, block);
 
-            if let Some((name, ty, initializer)) = initialize_visitor.get_result()
+            if let Some((name, ty, initializer, init_span)) = initialize_visitor.get_result()
                 && is_integer_const(cx, initializer, 0)
             {
                 let mut applicability = Applicability::MaybeIncorrect;
                 let span = expr.span.with_hi(arg.span.hi());
                 let loop_label = label.map_or(String::new(), |l| format!("{}: ", l.ident.name));
-                let int_name = match ty.map(Ty::kind) {
-                    // usize or inferred
+
+                // Determine the suggestion and optional note based on the variable's type
+                let suggestion_info = match ty.map(Ty::kind) {
                     Some(ty::Uint(UintTy::Usize)) | None => {
-                        span_lint_and_sugg(
-                            cx,
-                            EXPLICIT_COUNTER_LOOP,
-                            span,
-                            format!("the variable `{name}` is used as a loop counter"),
-                            "consider using",
+                        // usize or inferred (uses enumerate)
+                        Some((
                             format!(
                                 "{loop_label}for ({name}, {}) in {}.enumerate()",
                                 snippet_with_applicability(cx, pat.span, "item", &mut applicability),
                                 make_iterator_snippet(cx, arg, &mut applicability),
                             ),
-                            applicability,
-                        );
-                        return;
+                            None,
+                        ))
+                    },
+                    Some(ty::Int(int_ty)) => {
+                        // Signed integer types (uses (0_type..).zip)
+                        let int_name = int_ty.name_str();
+                        Some((
+                            format!(
+                                "{loop_label}for ({name}, {}) in (0_{int_name}..).zip({})",
+                                snippet_with_applicability(cx, pat.span, "item", &mut applicability),
+                                make_iterator_snippet(cx, arg, &mut applicability),
+                            ),
+                            Some(format!(
+                                "`{name}` is of type `{int_name}`, making it ineligible for `Iterator::enumerate`"
+                            )),
+                        ))
+                    },
+                    Some(ty::Uint(uint_ty)) => {
+                        // Other unsigned integer types (uses (0_type..).zip)
+                        let uint_name = uint_ty.name_str();
+                        Some((
+                            format!(
+                                "{loop_label}for ({name}, {}) in (0_{uint_name}..).zip({})",
+                                snippet_with_applicability(cx, pat.span, "item", &mut applicability),
+                                make_iterator_snippet(cx, arg, &mut applicability),
+                            ),
+                            Some(format!(
+                                "`{name}` is of type `{uint_name}`, making it ineligible for `Iterator::enumerate`"
+                            )),
+                        ))
                     },
-                    Some(ty::Int(int_ty)) => int_ty.name_str(),
-                    Some(ty::Uint(uint_ty)) => uint_ty.name_str(),
-                    _ => return,
+                    // Anything else (e.g., f32, struct) is ineligible
+                    _ => None,
+                };
+
+                // If ineligible, stop processing this counter variable
+                let Some((suggestion, note)) = suggestion_info else {
+                    continue;
                 };
 
                 span_lint_and_then(
                     cx,
                     EXPLICIT_COUNTER_LOOP,
-                    span,
+                    expr.span,
                     format!("the variable `{name}` is used as a loop counter"),
                     |diag| {
-                        diag.span_suggestion(
-                            span,
-                            "consider using",
-                            format!(
-                                "{loop_label}for ({name}, {}) in (0_{int_name}..).zip({})",
-                                snippet_with_applicability(cx, pat.span, "item", &mut applicability),
-                                make_iterator_snippet(cx, arg, &mut applicability),
-                            ),
+                        diag.multipart_suggestion(
+                            "consider rewriting the loop to use an iterator adapter",
+                            vec![
+                                (span, suggestion),
+                                (init_span, String::new()),
+                                (increment_span, String::new()),
+                            ],
                             applicability,
                         );
 
-                        diag.note(format!(
-                            "`{name}` is of type `{int_name}`, making it ineligible for `Iterator::enumerate`"
-                        ));
+                        if let Some(note_text) = note {
+                            diag.note(note_text);
+                        }
                     },
                 );
             }
diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs
index e5c37377e857..0de8b6116d18 100644
--- a/clippy_lints/src/loops/manual_memcpy.rs
+++ b/clippy_lints/src/loops/manual_memcpy.rs
@@ -442,30 +442,28 @@ fn get_assignments<'a, 'tcx>(
         .map(get_assignment)
 }
 
-fn get_loop_counters<'a, 'tcx>(
-    cx: &'a LateContext<'tcx>,
+fn get_loop_counters<'tcx>(
+    cx: &LateContext<'tcx>,
     body: &'tcx Block<'tcx>,
     expr: &'tcx Expr<'_>,
-) -> Option<impl Iterator<Item = Start<'tcx>> + 'a> {
-    // Look for variables that are incremented once per loop iteration.
+) -> Option<Vec<Start<'tcx>>> {
     let mut increment_visitor = IncrementVisitor::new(cx);
     walk_block(&mut increment_visitor, body);
 
-    // For each candidate, check the parent block to see if
-    // it's initialized to zero at the start of the loop.
-    get_enclosing_block(cx, expr.hir_id).and_then(|block| {
+    get_enclosing_block(cx, expr.hir_id).map(move |block| {
         increment_visitor
             .into_results()
-            .filter_map(move |var_id| {
+            .filter_map(move |(var_id, _)| {
+                // var_id is now the HirId struct
                 let mut initialize_visitor = InitializeVisitor::new(cx, expr, var_id);
                 walk_block(&mut initialize_visitor, block);
 
-                initialize_visitor.get_result().map(|(_, _, initializer)| Start {
+                initialize_visitor.get_result().map(|(_, _, initializer, _)| Start {
                     id: var_id,
                     kind: StartKind::Counter { initializer },
                 })
             })
-            .into()
+            .collect()
     })
 }
 
diff --git a/clippy_lints/src/loops/utils.rs b/clippy_lints/src/loops/utils.rs
index 56d535c4f262..d6fedd0713a0 100644
--- a/clippy_lints/src/loops/utils.rs
+++ b/clippy_lints/src/loops/utils.rs
@@ -1,20 +1,21 @@
 use clippy_utils::res::MaybeResPath;
 use clippy_utils::ty::{has_iter_method, implements_trait};
 use clippy_utils::{get_parent_expr, is_integer_const, sugg};
-use rustc_ast::ast::{LitIntType, LitKind};
+
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::{Visitor, walk_expr, walk_local};
 use rustc_hir::{AssignOpKind, BorrowKind, Expr, ExprKind, HirId, HirIdMap, LetStmt, Mutability, PatKind};
 use rustc_lint::LateContext;
 use rustc_middle::hir::nested_filter;
 use rustc_middle::ty::{self, Ty};
-use rustc_span::source_map::Spanned;
+
+use rustc_span::Span;
 use rustc_span::symbol::{Symbol, sym};
 
 #[derive(Debug, PartialEq, Eq)]
 enum IncrementVisitorVarState {
-    Initial,  // Not examined yet
-    IncrOnce, // Incremented exactly once, may be a loop counter
+    Initial,        // Not examined yet
+    IncrOnce(Span), // Incremented exactly once, may be a loop counter
     DontWarn,
 }
 
@@ -34,10 +35,10 @@ impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
         }
     }
 
-    pub(super) fn into_results(self) -> impl Iterator<Item = HirId> {
+    pub(super) fn into_results(self) -> impl Iterator<Item = (HirId, Span)> {
         self.states.into_iter().filter_map(|(id, state)| {
-            if state == IncrementVisitorVarState::IncrOnce {
-                Some(id)
+            if let IncrementVisitorVarState::IncrOnce(span) = state {
+                Some((id, span))
             } else {
                 None
             }
@@ -51,7 +52,7 @@ impl<'tcx> Visitor<'tcx> for IncrementVisitor<'_, 'tcx> {
         if let Some(def_id) = expr.res_local_id() {
             if let Some(parent) = get_parent_expr(self.cx, expr) {
                 let state = self.states.entry(def_id).or_insert(IncrementVisitorVarState::Initial);
-                if *state == IncrementVisitorVarState::IncrOnce {
+                if matches!(*state, IncrementVisitorVarState::IncrOnce(_)) {
                     *state = IncrementVisitorVarState::DontWarn;
                     return;
                 }
@@ -61,10 +62,10 @@ impl<'tcx> Visitor<'tcx> for IncrementVisitor<'_, 'tcx> {
                         if lhs.hir_id == expr.hir_id {
                             *state = if op.node == AssignOpKind::AddAssign
                                 && is_integer_const(self.cx, rhs, 1)
-                                && *state == IncrementVisitorVarState::Initial
+                                && matches!(*state, IncrementVisitorVarState::Initial)
                                 && self.depth == 0
                             {
-                                IncrementVisitorVarState::IncrOnce
+                                IncrementVisitorVarState::IncrOnce(parent.span)
                             } else {
                                 // Assigned some other value or assigned multiple times
                                 IncrementVisitorVarState::DontWarn
@@ -97,12 +98,14 @@ impl<'tcx> Visitor<'tcx> for IncrementVisitor<'_, 'tcx> {
 }
 
 enum InitializeVisitorState<'hir> {
-    Initial,                            // Not examined yet
+    Initial, // Not examined yet
+    #[allow(dead_code)]
     Declared(Symbol, Option<Ty<'hir>>), // Declared but not (yet) initialized
     Initialized {
         name: Symbol,
         ty: Option<Ty<'hir>>,
         initializer: &'hir Expr<'hir>,
+        stmt_span: Span,
     },
     DontWarn,
 }
@@ -130,9 +133,15 @@ impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> {
         }
     }
 
-    pub(super) fn get_result(&self) -> Option<(Symbol, Option<Ty<'tcx>>, &'tcx Expr<'tcx>)> {
-        if let InitializeVisitorState::Initialized { name, ty, initializer } = self.state {
-            Some((name, ty, initializer))
+    pub(super) fn get_result(&self) -> Option<(Symbol, Option<Ty<'tcx>>, &'tcx Expr<'tcx>, Span)> {
+        if let InitializeVisitorState::Initialized {
+            name,
+            ty,
+            initializer,
+            stmt_span,
+        } = self.state
+        {
+            Some((name, ty, initializer, stmt_span))
         } else {
             None
         }
@@ -154,6 +163,7 @@ impl<'tcx> Visitor<'tcx> for InitializeVisitor<'_, 'tcx> {
                     initializer: init,
                     ty,
                     name: ident.name,
+                    stmt_span: l.span,
                 }
             });
         }
@@ -175,6 +185,7 @@ impl<'tcx> Visitor<'tcx> for InitializeVisitor<'_, 'tcx> {
             return;
         }
 
+        // If node is the desired variable, see how it's used
         // If node is the desired variable, see how it's used
         if expr.res_local_id() == Some(self.var_id) {
             if self.past_loop {
@@ -187,40 +198,9 @@ impl<'tcx> Visitor<'tcx> for InitializeVisitor<'_, 'tcx> {
                     ExprKind::AssignOp(_, lhs, _) if lhs.hir_id == expr.hir_id => {
                         self.state = InitializeVisitorState::DontWarn;
                     },
-                    ExprKind::Assign(lhs, rhs, _) if lhs.hir_id == expr.hir_id => {
-                        self.state = if self.depth == 0 {
-                            match self.state {
-                                InitializeVisitorState::Declared(name, mut ty) => {
-                                    if ty.is_none() {
-                                        if let ExprKind::Lit(Spanned {
-                                            node: LitKind::Int(_, LitIntType::Unsuffixed),
-                                            ..
-                                        }) = rhs.kind
-                                        {
-                                            ty = None;
-                                        } else {
-                                            ty = self.cx.typeck_results().expr_ty_opt(rhs);
-                                        }
-                                    }
-
-                                    InitializeVisitorState::Initialized {
-                                        initializer: rhs,
-                                        ty,
-                                        name,
-                                    }
-                                },
-                                InitializeVisitorState::Initialized { ty, name, .. } => {
-                                    InitializeVisitorState::Initialized {
-                                        initializer: rhs,
-                                        ty,
-                                        name,
-                                    }
-                                },
-                                _ => InitializeVisitorState::DontWarn,
-                            }
-                        } else {
-                            InitializeVisitorState::DontWarn
-                        }
+                    // If the variable is reassigned (`i = value;`), it invalidates the simple counter pattern.
+                    ExprKind::Assign(lhs, _, _) if lhs.hir_id == expr.hir_id => {
+                        self.state = InitializeVisitorState::DontWarn;
                     },
                     ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _) => {
                         self.state = InitializeVisitorState::DontWarn;
diff --git a/tests/ui/explicit_counter_loop.rs b/tests/ui/explicit_counter_loop.rs
index ec4cecf37766..f9ded959898b 100644
--- a/tests/ui/explicit_counter_loop.rs
+++ b/tests/ui/explicit_counter_loop.rs
@@ -13,11 +13,8 @@ fn main() {
     let mut _index = 1;
     _index = 0;
     for _v in &vec {
-        //~^ explicit_counter_loop
-
         _index += 1
     }
-
     let mut _index = 0;
     for _v in &mut vec {
         //~^ explicit_counter_loop
diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr
index a73516558c20..6cb6168fd8fc 100644
--- a/tests/ui/explicit_counter_loop.stderr
+++ b/tests/ui/explicit_counter_loop.stderr
@@ -1,67 +1,190 @@
 error: the variable `_index` is used as a loop counter
   --> tests/ui/explicit_counter_loop.rs:7:5
    |
-LL |     for _v in &vec {
-   |     ^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter().enumerate()`
+LL | /     for _v in &vec {
+LL | |
+LL | |
+LL | |         _index += 1
+LL | |     }
+   | |_____^
    |
    = note: `-D clippy::explicit-counter-loop` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::explicit_counter_loop)]`
-
-error: the variable `_index` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:15:5
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~     
+LL ~     for (_index, _v) in vec.iter().enumerate() {
+LL |
+LL |
+LL ~         
    |
-LL |     for _v in &vec {
-   |     ^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter().enumerate()`
 
 error: the variable `_index` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:22:5
+  --> tests/ui/explicit_counter_loop.rs:19:5
+   |
+LL | /     for _v in &mut vec {
+LL | |
+LL | |
+LL | |         _index += 1;
+LL | |     }
+   | |_____^
+   |
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~     
+LL ~     for (_index, _v) in vec.iter_mut().enumerate() {
+LL |
+LL |
+LL ~         ;
    |
-LL |     for _v in &mut vec {
-   |     ^^^^^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter_mut().enumerate()`
 
 error: the variable `_index` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:29:5
+  --> tests/ui/explicit_counter_loop.rs:26:5
+   |
+LL | /     for _v in vec {
+LL | |
+LL | |
+LL | |         _index += 1;
+LL | |     }
+   | |_____^
+   |
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~     
+LL ~     for (_index, _v) in vec.into_iter().enumerate() {
+LL |
+LL |
+LL ~         ;
    |
-LL |     for _v in vec {
-   |     ^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.into_iter().enumerate()`
 
 error: the variable `count` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:118:9
+  --> tests/ui/explicit_counter_loop.rs:115:9
+   |
+LL | /         for ch in text.chars() {
+LL | |
+LL | |
+LL | |             println!("{count}");
+...  |
+LL | |         }
+   | |_________^
+   |
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~         
+LL ~         for (count, ch) in text.chars().enumerate() {
+LL |
+LL |
+LL |             println!("{count}");
+LL ~             ;
    |
-LL |         for ch in text.chars() {
-   |         ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`
 
 error: the variable `count` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:131:9
+  --> tests/ui/explicit_counter_loop.rs:128:9
+   |
+LL | /         for ch in text.chars() {
+LL | |
+LL | |
+LL | |             println!("{count}");
+...  |
+LL | |         }
+   | |_________^
+   |
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~         
+LL ~         for (count, ch) in text.chars().enumerate() {
+LL |
+LL |
+LL |             println!("{count}");
+LL ~             ;
    |
-LL |         for ch in text.chars() {
-   |         ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`
 
 error: the variable `count` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:191:9
+  --> tests/ui/explicit_counter_loop.rs:188:9
+   |
+LL | /         for _i in 3..10 {
+LL | |
+LL | |
+LL | |             count += 1;
+LL | |         }
+   | |_________^
+   |
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~         
+LL ~         for (count, _i) in (3..10).enumerate() {
+LL |
+LL |
+LL ~             ;
    |
-LL |         for _i in 3..10 {
-   |         ^^^^^^^^^^^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()`
 
 error: the variable `idx_usize` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:233:9
+  --> tests/ui/explicit_counter_loop.rs:230:9
+   |
+LL | /         for _item in slice {
+LL | |
+LL | |
+LL | |             if idx_usize == index_usize {
+...  |
+LL | |             idx_usize += 1;
+LL | |         }
+   | |_________^
+   |
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~         
+LL |
+LL |         // should suggest `enumerate`
+LL ~         for (idx_usize, _item) in slice.iter().enumerate() {
+LL |
+...
+LL |
+LL ~             ;
    |
-LL |         for _item in slice {
-   |         ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_usize, _item) in slice.iter().enumerate()`
 
 error: the variable `idx_u32` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:247:9
+  --> tests/ui/explicit_counter_loop.rs:244:9
    |
-LL |         for _item in slice {
-   |         ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_u32, _item) in (0_u32..).zip(slice.iter())`
+LL | /         for _item in slice {
+LL | |
+LL | |
+LL | |             if idx_u32 == index_u32 {
+...  |
+LL | |             idx_u32 += 1;
+LL | |         }
+   | |_________^
    |
    = note: `idx_u32` is of type `u32`, making it ineligible for `Iterator::enumerate`
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~         
+LL |
+LL |         // should suggest `zip`
+LL ~         for (idx_u32, _item) in (0_u32..).zip(slice.iter()) {
+LL |
+...
+LL |
+LL ~             ;
+   |
 
 error: the variable `_index` is used as a loop counter
-  --> tests/ui/explicit_counter_loop.rs:293:9
+  --> tests/ui/explicit_counter_loop.rs:290:9
+   |
+LL | /         'label: for v in vec {
+LL | |
+LL | |             _index += 1;
+LL | |             if v == 1 {
+...  |
+LL | |         }
+   | |_________^
+   |
+help: consider rewriting the loop to use an iterator adapter
+   |
+LL ~         
+LL ~         'label: for (_index, v) in vec.into_iter().enumerate() {
+LL |
+LL ~             ;
    |
-LL |         'label: for v in vec {
-   |         ^^^^^^^^^^^^^^^^^^^^ help: consider using: `'label: for (_index, v) in vec.into_iter().enumerate()`
 
-error: aborting due to 10 previous errors
+error: aborting due to 9 previous errors