Suggest calling `iter` if needed in `explicit_counter_loop`

3 files changed, 67 insertions, 21 deletions

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 22821e02fe2..81a8c313e9d 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -27,9 +27,10 @@ use syntax_pos::{BytePos, Symbol};
 
 use crate::utils::paths;
 use crate::utils::{
-    get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_const, is_refutable, last_path_segment,
-    match_trait_method, match_type, match_var, multispan_sugg, snippet, snippet_opt, snippet_with_applicability,
-    span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, SpanlessEq,
+    get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
+    is_integer_const, is_refutable, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg,
+    snippet, snippet_opt, snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg,
+    span_lint_and_then, SpanlessEq,
 };
 
 declare_clippy_lint! {
@@ -1470,17 +1471,7 @@ fn check_for_loop_explicit_counter<'a, 'tcx>(
                             "for ({}, {}) in {}.enumerate()",
                             name,
                             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()
-                                )
-                            }
+                            make_iterator_snippet(cx, arg, &mut applicability),
                         ),
                         applicability,
                     );
@@ -1490,6 +1481,39 @@ fn check_for_loop_explicit_counter<'a, 'tcx>(
     }
 }
 
+/// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the
+/// actual `Iterator` that the loop uses.
+fn make_iterator_snippet(cx: &LateContext<'_, '_>, arg: &Expr, applic_ref: &mut Applicability) -> String {
+    let impls_iterator = get_trait_def_id(cx, &paths::ITERATOR)
+        .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(arg), id, &[]));
+    if impls_iterator {
+        format!(
+            "{}",
+            sugg::Sugg::hir_with_applicability(cx, arg, "_", applic_ref).maybe_par()
+        )
+    } else {
+        // (&x).into_iter() ==> x.iter()
+        // (&mut x).into_iter() ==> x.iter_mut()
+        match &arg.kind {
+            ExprKind::AddrOf(mutability, arg_inner) if has_iter_method(cx, cx.tables.expr_ty(&arg_inner)).is_some() => {
+                let meth_name = match mutability {
+                    MutMutable => "iter_mut",
+                    MutImmutable => "iter",
+                };
+                format!(
+                    "{}.{}()",
+                    sugg::Sugg::hir_with_applicability(cx, &arg_inner, "_", applic_ref).maybe_par(),
+                    meth_name,
+                )
+            },
+            _ => format!(
+                "{}.into_iter()",
+                sugg::Sugg::hir_with_applicability(cx, arg, "_", applic_ref).maybe_par()
+            ),
+        }
+    }
+}
+
 /// Checks for the `FOR_KV_MAP` lint.
 fn check_for_loop_over_map_kv<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
diff --git a/tests/ui/explicit_counter_loop.rs b/tests/ui/explicit_counter_loop.rs
index 71ef1e8674a..e6fbf83a287 100644
--- a/tests/ui/explicit_counter_loop.rs
+++ b/tests/ui/explicit_counter_loop.rs
@@ -12,6 +12,16 @@ fn main() {
     for _v in &vec {
         _index += 1
     }
+
+    let mut _index = 0;
+    for _v in &mut vec {
+        _index += 1;
+    }
+
+    let mut _index = 0;
+    for _v in vec {
+        _index += 1;
+    }
 }
 
 mod issue_1219 {
diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr
index 5efd51abf18..c5643a857b1 100644
--- a/tests/ui/explicit_counter_loop.stderr
+++ b/tests/ui/explicit_counter_loop.stderr
@@ -2,7 +2,7 @@ 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()`
+   |               ^^^^ help: consider using: `for (_index, _v) in vec.iter().enumerate()`
    |
    = note: `-D clippy::explicit-counter-loop` implied by `-D warnings`
 
@@ -10,25 +10,37 @@ 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()`
+   |               ^^^^ help: consider using: `for (_index, _v) in vec.iter().enumerate()`
+
+error: the variable `_index` is used as a loop counter.
+  --> $DIR/explicit_counter_loop.rs:17:15
+   |
+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.
+  --> $DIR/explicit_counter_loop.rs:22:15
+   |
+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.
-  --> $DIR/explicit_counter_loop.rs:51:19
+  --> $DIR/explicit_counter_loop.rs:61: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.
-  --> $DIR/explicit_counter_loop.rs:62:19
+  --> $DIR/explicit_counter_loop.rs:72: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.
-  --> $DIR/explicit_counter_loop.rs:120:19
+  --> $DIR/explicit_counter_loop.rs:130:19
    |
 LL |         for _i in 3..10 {
-   |                   ^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()`
+   |                   ^^^^^ help: consider using: `for (count, _i) in 3..10.enumerate()`
 
-error: aborting due to 5 previous errors
+error: aborting due to 7 previous errors