Fix explicit_counter_loop FN when loop counter starts at non-zero (rust-lang#16620)

Closes rust-lang#16612

Previous implementation hardcoded `0` as loop counter start, this PR
lifts that restriction.

changelog: [`explicit_counter_loop`] fix FN when loop counter starts at
non-zero

Author: Jarcho

clippy_lints/src/loops/explicit_counter_loop.rs

@@ -1,17 +1,19 @@
+use std::borrow::Cow;
+
 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;
+use clippy_utils::sugg::{EMPTY, Sugg};
+use clippy_utils::{get_enclosing_block, is_integer_const, is_integer_literal_untyped};
+use rustc_ast::{Label, RangeLimits};
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::{walk_block, walk_expr};
 use rustc_hir::{Expr, Pat};
 use rustc_lint::LateContext;
 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.
+// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be incremented exactly once in the
+// loop body.
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     pat: &'tcx Pat<'_>,
@@ -31,55 +33,64 @@ pub(super) fn check<'tcx>(
             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()
-                && is_integer_const(cx, initializer, 0)
-            {
+            if let Some((name, ty, initializer)) = initialize_visitor.get_result() {
+                let is_zero = 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
-                    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",
-                            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;
-                    },
-                    Some(ty::Int(int_ty)) => int_ty.name_str(),
-                    Some(ty::Uint(uint_ty)) => uint_ty.name_str(),
-                    _ => return,
-                };
 
                 span_lint_and_then(
                     cx,
                     EXPLICIT_COUNTER_LOOP,
                     span,
                     format!("the variable `{name}` is used as a loop counter"),
                     |diag| {
+                        let pat_snippet = snippet_with_applicability(cx, pat.span, "item", &mut applicability);
+                        let iter_snippet = make_iterator_snippet(cx, arg, &mut applicability);
+                        let int_name = match ty.map(Ty::kind) {
+                            Some(ty::Uint(UintTy::Usize)) | None => {
+                                if is_zero {
+                                    diag.span_suggestion(
+                                        span,
+                                        "consider using",
+                                        format!(
+                                            "{loop_label}for ({name}, {pat_snippet}) in {iter_snippet}.enumerate()",
+                                        ),
+                                        applicability,
+                                    );
+                                    return;
+                                }
+                                None
+                            },
+                            Some(ty::Int(int_ty)) => Some(int_ty.name_str()),
+                            Some(ty::Uint(uint_ty)) => Some(uint_ty.name_str()),
+                            _ => None,
+                        }
+                        .filter(|_| is_integer_literal_untyped(initializer));
+
+                        let initializer = Sugg::hir_from_snippet(cx, initializer, |span| {
+                            let snippet = snippet_with_applicability(cx, span, "..", &mut applicability);
+                            if let Some(int_name) = int_name {
+                                return Cow::Owned(format!("{snippet}_{int_name}"));
+                            }
+                            snippet
+                        });
+
                         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),
+                                "{loop_label}for ({name}, {pat_snippet}) in ({}).zip({iter_snippet})",
+                                initializer.range(&EMPTY, RangeLimits::HalfOpen)
                             ),
                             applicability,
                         );
 
-                        diag.note(format!(
-                            "`{name}` is of type `{int_name}`, making it ineligible for `Iterator::enumerate`"
-                        ));
+                        if is_zero && let Some(int_name) = int_name {
+                            diag.note(format!(
+                                "`{name}` is of type `{int_name}`, making it ineligible for `Iterator::enumerate`"
+                            ));
+                        }
                     },
                 );
             }

tests/ui/explicit_counter_loop.rs

@@ -33,10 +33,10 @@ fn main() {
     }
 
     let vec = [1, 2, 3, 4];
-    // Potential false positives
     let mut _index = 0;
     _index = 1;
     for _v in &vec {
+        //~^ explicit_counter_loop
         _index += 1
     }
 
@@ -299,3 +299,21 @@ mod issue_13123 {
         }
     }
 }
+
+fn issue16612(v: Vec<u8>, s: i64) {
+    use std::hint::black_box;
+
+    let mut i = 1;
+    for item in &v {
+        //~^ explicit_counter_loop
+        black_box((i, *item));
+        i += 1;
+    }
+
+    let mut j = s + 1;
+    for item in &v {
+        //~^ explicit_counter_loop
+        black_box((j, *item));
+        j += 1;
+    }
+}

tests/ui/explicit_counter_loop.stderr

@@ -25,6 +25,12 @@ error: the variable `_index` is used as a loop counter
 LL |     for _v in vec {
    |     ^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.into_iter().enumerate()`
 
+error: the variable `_index` is used as a loop counter
+  --> tests/ui/explicit_counter_loop.rs:38:5
+   |
+LL |     for _v in &vec {
+   |     ^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in (1..).zip(vec.iter())`
+
 error: the variable `count` is used as a loop counter
   --> tests/ui/explicit_counter_loop.rs:118:9
    |
@@ -63,5 +69,17 @@ error: the variable `_index` is used as a loop counter
 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: the variable `i` is used as a loop counter
+  --> tests/ui/explicit_counter_loop.rs:307:5
+   |
+LL |     for item in &v {
+   |     ^^^^^^^^^^^^^^ help: consider using: `for (i, item) in (1..).zip(v.iter())`
+
+error: the variable `j` is used as a loop counter
+  --> tests/ui/explicit_counter_loop.rs:314:5
+   |
+LL |     for item in &v {
+   |     ^^^^^^^^^^^^^^ help: consider using: `for (j, item) in (s + 1..).zip(v.iter())`
+
+error: aborting due to 13 previous errors