Add suggestions to `EXPLICIT_[INTO_]ITER_LOOP`

Also reduces the highlighted span to the expr containing the
`.[into_]iter()` call (so the suggestion is probably applicable by
rustfix.)

Fixes #1484

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 73926fed0f159b2f581e63d3159df2e36adc3dda..e3492fd00f75cfd7c4b451dfc05ceb229f257e24 100644 (file)
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -588,28 +588,38 @@ fn check_for_loop_arg(cx: &LateContext, pat: &Pat, arg: &Expr, expr: &Expr) {
             if &*method_name.as_str() == "iter" || &*method_name.as_str() == "iter_mut" {
                 if is_ref_iterable_type(cx, &args[0]) {
                     let object = snippet(cx, args[0].span, "_");
-                    span_lint(cx,
-                              EXPLICIT_ITER_LOOP,
-                              expr.span,
-                              &format!("it is more idiomatic to loop over `&{}{}` instead of `{}.{}()`",
-                                       if &*method_name.as_str() == "iter_mut" {
-                                           "mut "
-                                       } else {
-                                           ""
-                                       },
-                                       object,
-                                       object,
-                                       method_name));
+                    let suggestion = format!("&{}{}",
+                                             if &*method_name.as_str() == "iter_mut" {
+                                                 "mut "
+                                             } else {
+                                                 ""
+                                             },
+                                             object);
+                    span_lint_and_then(cx,
+                                       EXPLICIT_ITER_LOOP,
+                                       arg.span,
+                                       &format!("it is more idiomatic to loop over `{}` instead of `{}.{}()`",
+                                                suggestion,
+                                                object,
+                                                method_name),
+                                       |db| {
+                        db.span_suggestion(arg.span, "to write this more concisely, try looping over", suggestion);
+                    });
                 }
             } else if &*method_name.as_str() == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) {
                 let object = snippet(cx, args[0].span, "_");
-                span_lint(cx,
-                          EXPLICIT_INTO_ITER_LOOP,
-                          expr.span,
-                          &format!("it is more idiomatic to loop over `{}` instead of `{}.{}()`",
-                                   object,
-                                   object,
-                                   method_name));
+                span_lint_and_then(cx,
+                                   EXPLICIT_INTO_ITER_LOOP,
+                                   arg.span,
+                                   &format!("it is more idiomatic to loop over `{}` instead of `{}.{}()`",
+                                            object,
+                                            object,
+                                            method_name),
+                                   |db| {
+                    db.span_suggestion(arg.span,
+                                       "to write this more concisely, try looping over",
+                                       object.to_string());
+                });
 
             } else if &*method_name.as_str() == "next" && match_trait_method(cx, arg, &paths::ITERATOR) {
                 span_lint(cx,

diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs
index 659935e6098ed49065c8a0987907cfc7289d5287..5e13b7783291fa3182ad83ee4350bf5ea4c9965f 100644 (file)
--- a/tests/compile-fail/for_loop.rs
+++ b/tests/compile-fail/for_loop.rs
@@ -286,39 +286,86 @@ fn main() {
         id_col[i] = 1f64;
     }
 
-    /*
+    // This is fine.
     for i in (10..0).map(|x| x * 2) {
         println!("{}", i);
-    }*/
+    }
 
-    for _v in vec.iter() { } //~ERROR it is more idiomatic to loop over `&vec`
-    for _v in vec.iter_mut() { } //~ERROR it is more idiomatic to loop over `&mut vec`
+    for _v in vec.iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&vec`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &vec
 
+    for _v in vec.iter_mut() { }
+    //~^ ERROR it is more idiomatic to loop over `&mut vec`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION vec
 
     let out_vec = vec![1,2,3];
-    for _v in out_vec.into_iter() { } //~ERROR it is more idiomatic to loop over `out_vec` instead of `out_vec.into_iter()`
+    for _v in out_vec.into_iter() { }
+    //~^ ERROR it is more idiomatic to loop over `out_vec` instead of `out_vec.into_iter()`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION out_vec
 
     for _v in &vec { } // these are fine
     for _v in &mut vec { } // these are fine
 
-    for _v in [1, 2, 3].iter() { } //~ERROR it is more idiomatic to loop over `&[
+    for _v in [1, 2, 3].iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&[
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &[1, 2, 3]
+
     for _v in (&mut [1, 2, 3]).iter() { } // no error
-    for _v in [0; 32].iter() {} //~ERROR it is more idiomatic to loop over `&[
+
+    for _v in [0; 32].iter() {}
+    //~^ ERROR it is more idiomatic to loop over `&[
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &[0; 32]
+
     for _v in [0; 33].iter() {} // no error
+
     let ll: LinkedList<()> = LinkedList::new();
-    for _v in ll.iter() { } //~ERROR it is more idiomatic to loop over `&ll`
+    for _v in ll.iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&ll`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &ll
+
     let vd: VecDeque<()> = VecDeque::new();
-    for _v in vd.iter() { } //~ERROR it is more idiomatic to loop over `&vd`
+    for _v in vd.iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&vd`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &vd
+
     let bh: BinaryHeap<()> = BinaryHeap::new();
-    for _v in bh.iter() { } //~ERROR it is more idiomatic to loop over `&bh`
+    for _v in bh.iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&bh`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &bh
+
     let hm: HashMap<(), ()> = HashMap::new();
-    for _v in hm.iter() { } //~ERROR it is more idiomatic to loop over `&hm`
+    for _v in hm.iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&hm`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &hm
+
     let bt: BTreeMap<(), ()> = BTreeMap::new();
-    for _v in bt.iter() { } //~ERROR it is more idiomatic to loop over `&bt`
+    for _v in bt.iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&bt`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &bt
+
     let hs: HashSet<()> = HashSet::new();
-    for _v in hs.iter() { } //~ERROR it is more idiomatic to loop over `&hs`
+    for _v in hs.iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&hs`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &hs
+
     let bs: BTreeSet<()> = BTreeSet::new();
-    for _v in bs.iter() { } //~ERROR it is more idiomatic to loop over `&bs`
+    for _v in bs.iter() { }
+    //~^ ERROR it is more idiomatic to loop over `&bs`
+    //~| HELP to write this more concisely, try looping over
+    //~| SUGGESTION &bs
+
 
     for _v in vec.iter().next() { } //~ERROR you are iterating over `Iterator::next()`