Auto merge of #3901 - rail-rain:issue_1670, r=flip1995

Fix `explicit_counter_loop` suggestion

#1670

This code seems to me to work, but I have two question.
* Because range expression desugared in hir, `Sugg::hir` doesn't add parenthesis to range expression.  Which function is better to check range do you think, `check_for_loop_explicit_counter` or `hir_from_snippet`?
* Do you think we need to distinguish between range expression and struct expression that creates `std::ops::Range*`?

Author: bors

clippy_lints/src/loops.rs

@@ -777,7 +777,7 @@ fn check_for_loop<'a, 'tcx>(
     check_for_loop_range(cx, pat, arg, body, expr);
     check_for_loop_reverse_range(cx, arg, expr);
     check_for_loop_arg(cx, pat, arg, expr);
-    check_for_loop_explicit_counter(cx, arg, body, expr);
+    check_for_loop_explicit_counter(cx, pat, arg, body, expr);
     check_for_loop_over_map_kv(cx, pat, arg, body, expr);
     check_for_mut_range_bound(cx, arg, body);
     detect_manual_memcpy(cx, pat, arg, body, expr);
@@ -1453,6 +1453,7 @@ fn check_arg_type(cx: &LateContext<'_, '_>, pat: &Pat, arg: &Expr) {
 
 fn check_for_loop_explicit_counter<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
+    pat: &'tcx Pat,
     arg: &'tcx Expr,
     body: &'tcx Expr,
     expr: &'tcx Expr,
@@ -1489,16 +1490,31 @@ fn check_for_loop_explicit_counter<'a, 'tcx>(
 
                 if visitor2.state == VarState::Warn {
                     if let Some(name) = visitor2.name {
-                        span_lint(
+                        let mut applicability = Applicability::MachineApplicable;
+                        span_lint_and_sugg(
                             cx,
                             EXPLICIT_COUNTER_LOOP,
                             expr.span,
-                            &format!(
-                                "the variable `{0}` is used as a loop counter. Consider using `for ({0}, \
-                                 item) in {1}.enumerate()` or similar iterators",
+                            &format!("the variable `{}` is used as a loop counter.", name),
+                            "consider using",
+                            format!(
+                                "for ({}, {}) in {}.enumerate()",
                                 name,
-                                snippet(cx, arg.span, "_")
+                                snippet_with_applicability(cx, pat.span, "item", &mut applicability),
+                                if higher::range(cx, arg).is_some() {
+                                    format!(
+                                        "({})",
+                                        snippet_with_applicability(cx, arg.span, "_", &mut applicability)
+                                    )
+                                } else {
+                                    format!(
+                                        "{}",
+                                        sugg::Sugg::hir_with_applicability(cx, arg, "_", &mut applicability)
+                                            .maybe_par()
+                                    )
+                                }
                             ),
+                            applicability,
                         );
                     }
                 }

tests/ui/explicit_counter_loop.rs

@@ -113,3 +113,12 @@ mod issue_3308 {
         }
     }
 }
+
+mod issue_1670 {
+    pub fn test() {
+        let mut count = 0;
+        for _i in 3..10 {
+            count += 1;
+        }
+    }
+}

tests/ui/explicit_counter_loop.stderr

@@ -1,28 +1,34 @@
-error: the variable `_index` is used as a loop counter. Consider using `for (_index, item) in &vec.enumerate()` or similar iterators
+error: the variable `_index` is used as a loop counter.
   --> $DIR/explicit_counter_loop.rs:6:15
    |
 LL |     for _v in &vec {
-   |               ^^^^
+   |               ^^^^ help: consider using: `for (_index, _v) in (&vec).enumerate()`
    |
    = note: `-D clippy::explicit-counter-loop` implied by `-D warnings`
 
-error: the variable `_index` is used as a loop counter. Consider using `for (_index, item) in &vec.enumerate()` or similar iterators
+error: the variable `_index` is used as a loop counter.
   --> $DIR/explicit_counter_loop.rs:12:15
    |
 LL |     for _v in &vec {
-   |               ^^^^
+   |               ^^^^ help: consider using: `for (_index, _v) in (&vec).enumerate()`
 
-error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators
+error: the variable `count` is used as a loop counter.
   --> $DIR/explicit_counter_loop.rs:51:19
    |
 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. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators
+error: the variable `count` is used as a loop counter.
   --> $DIR/explicit_counter_loop.rs:62:19
    |
 LL |         for ch in text.chars() {
-   |                   ^^^^^^^^^^^^
+   |                   ^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`
 
-error: aborting due to 4 previous errors
+error: the variable `count` is used as a loop counter.
+  --> $DIR/explicit_counter_loop.rs:120:19
+   |
+LL |         for _i in 3..10 {
+   |                   ^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()`
+
+error: aborting due to 5 previous errors