Refactor check_for_loop_explicit_counter to its own module

Author: nahuakang

clippy_lints/src/loops/for_loop_explicit_counter.rs

@@ -0,0 +1,56 @@
+use super::{get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor};
+use crate::utils::{get_enclosing_block, is_integer_const, snippet_with_applicability, span_lint_and_sugg};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::intravisit::{walk_block, walk_expr};
+use rustc_hir::{Expr, Pat};
+use rustc_lint::LateContext;
+
+// 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.
+pub(super) fn check_for_loop_explicit_counter<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &'tcx Pat<'_>,
+    arg: &'tcx Expr<'_>,
+    body: &'tcx Expr<'_>,
+    expr: &'tcx Expr<'_>,
+) {
+    // 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() {
+            let mut initialize_visitor = InitializeVisitor::new(cx, expr, id);
+            walk_block(&mut initialize_visitor, block);
+
+            if_chain! {
+                if let Some((name, initializer)) = initialize_visitor.get_result();
+                if is_integer_const(cx, initializer, 0);
+                then {
+                    let mut applicability = Applicability::MachineApplicable;
+
+                    let for_span = get_span_of_entire_for_loop(expr);
+
+                    span_lint_and_sugg(
+                        cx,
+                        super::EXPLICIT_COUNTER_LOOP,
+                        for_span.with_hi(arg.span.hi()),
+                        &format!("the variable `{}` is used as a loop counter", name),
+                        "consider using",
+                        format!(
+                            "for ({}, {}) in {}.enumerate()",
+                            name,
+                            snippet_with_applicability(cx, pat.span, "item", &mut applicability),
+                            make_iterator_snippet(cx, arg, &mut applicability),
+                        ),
+                        applicability,
+                    );
+                }
+            }
+        }
+    }
+}

clippy_lints/src/loops/mod.rs

@@ -1,4 +1,5 @@
 mod for_loop_arg;
+mod for_loop_explicit_counter;
 mod for_loop_over_map_kv;
 mod for_loop_range;
 mod for_mut_range_bound;
@@ -32,7 +33,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 use rustc_span::symbol::{sym, Ident, Symbol};
 use std::iter::{once, Iterator};
-use utils::make_iterator_snippet;
+use utils::{get_span_of_entire_for_loop, make_iterator_snippet};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for for-loops that manually copy items between
@@ -857,7 +858,7 @@ fn check_for_loop<'tcx>(
     let is_manual_memcpy_triggered = detect_manual_memcpy(cx, pat, arg, body, expr);
     if !is_manual_memcpy_triggered {
         for_loop_range::check_for_loop_range(cx, pat, arg, body, expr);
-        check_for_loop_explicit_counter(cx, pat, arg, body, expr);
+        for_loop_explicit_counter::check_for_loop_explicit_counter(cx, pat, arg, body, expr);
     }
     for_loop_arg::check_for_loop_arg(cx, pat, arg, expr);
     for_loop_over_map_kv::check_for_loop_over_map_kv(cx, pat, arg, body, expr);
@@ -867,17 +868,6 @@ fn check_for_loop<'tcx>(
     manual_flatten::check_manual_flatten(cx, pat, arg, body, span);
 }
 
-// this function assumes the given expression is a `for` loop.
-fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span {
-    // for some reason this is the only way to get the `Span`
-    // of the entire `for` loop
-    if let ExprKind::Match(_, arms, _) = &expr.kind {
-        arms[0].body.span
-    } else {
-        unreachable!()
-    }
-}
-
 /// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`;
 /// and also, it avoids subtracting a variable from the same one by replacing it with `0`.
 /// it exists for the convenience of the overloaded operators while normal functions can do the
@@ -1474,55 +1464,6 @@ fn detect_same_item_push<'tcx>(
     }
 }
 
-// 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.
-fn check_for_loop_explicit_counter<'tcx>(
-    cx: &LateContext<'tcx>,
-    pat: &'tcx Pat<'_>,
-    arg: &'tcx Expr<'_>,
-    body: &'tcx Expr<'_>,
-    expr: &'tcx Expr<'_>,
-) {
-    // 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() {
-            let mut initialize_visitor = InitializeVisitor::new(cx, expr, id);
-            walk_block(&mut initialize_visitor, block);
-
-            if_chain! {
-                if let Some((name, initializer)) = initialize_visitor.get_result();
-                if is_integer_const(cx, initializer, 0);
-                then {
-                    let mut applicability = Applicability::MachineApplicable;
-
-                    let for_span = get_span_of_entire_for_loop(expr);
-
-                    span_lint_and_sugg(
-                        cx,
-                        EXPLICIT_COUNTER_LOOP,
-                        for_span.with_hi(arg.span.hi()),
-                        &format!("the variable `{}` is used as a loop counter", name),
-                        "consider using",
-                        format!(
-                            "for ({}, {}) in {}.enumerate()",
-                            name,
-                            snippet_with_applicability(cx, pat.span, "item", &mut applicability),
-                            make_iterator_snippet(cx, arg, &mut applicability),
-                        ),
-                        applicability,
-                    );
-                }
-            }
-        }
-    }
-}
-
 fn check_for_single_element_loop<'tcx>(
     cx: &LateContext<'tcx>,
     pat: &'tcx Pat<'_>,

clippy_lints/src/loops/utils.rs

@@ -2,6 +2,18 @@ use crate::utils::{get_trait_def_id, has_iter_method, implements_trait, paths, s
 use rustc_errors::Applicability;
 use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability};
 use rustc_lint::LateContext;
+use rustc_span::source_map::Span;
+
+// this function assumes the given expression is a `for` loop.
+pub(super) fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span {
+    // for some reason this is the only way to get the `Span`
+    // of the entire `for` loop
+    if let ExprKind::Match(_, arms, _) = &expr.kind {
+        arms[0].body.span
+    } else {
+        unreachable!()
+    }
+}
 
 /// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the
 /// actual `Iterator` that the loop uses.