Improve `NEEDLESS_RANGE_LOOP` error reporting

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 1a6ac75eb377ef36e0675c98b223f1542df3c1da..0f9d030c2f285b373d8256e823aa40370e2ffb7c 100644 (file)
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -13,7 +13,7 @@
 use std::collections::HashMap;
 use syntax::ast;
 
-use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro,
+use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, multispan_sugg, in_external_macro,
             span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher,
             walk_ptrs_ty};
 use utils::paths;
@@ -377,17 +377,16 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
                 };
 
                 if visitor.nonindex {
-                    span_lint(cx,
-                              NEEDLESS_RANGE_LOOP,
-                              expr.span,
-                              &format!("the loop variable `{}` is used to index `{}`. Consider using `for ({}, \
-                                        item) in {}.iter().enumerate(){}{}` or similar iterators",
-                                       ident.node,
-                                       indexed,
-                                       ident.node,
-                                       indexed,
-                                       take,
-                                       skip));
+                    span_lint_and_then(cx,
+                                       NEEDLESS_RANGE_LOOP,
+                                       expr.span,
+                                       &format!("the loop variable `{}` is used to index `{}`", ident.node, indexed),
+                                       |db| {
+                        multispan_sugg(db, "consider using an iterator".to_string(), &[
+                            (pat.span, &format!("({}, <item>)", ident.node)),
+                            (arg.span, &format!("{}.iter().enumerate(){}{}", indexed, take, skip)),
+                        ]);
+                    });
                 } else {
                     let repl = if starts_at_zero && take.is_empty() {
                         format!("&{}", indexed)
@@ -395,14 +394,16 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
                         format!("{}.iter(){}{}", indexed, take, skip)
                     };
 
-                    span_lint(cx,
-                              NEEDLESS_RANGE_LOOP,
-                              expr.span,
-                              &format!("the loop variable `{}` is only used to index `{}`. \
-                                        Consider using `for item in {}` or similar iterators",
-                                       ident.node,
-                                       indexed,
-                                       repl));
+                    span_lint_and_then(cx,
+                                       NEEDLESS_RANGE_LOOP,
+                                       expr.span,
+                                       &format!("the loop variable `{}` is only used to index `{}`.", ident.node, indexed),
+                                       |db| {
+                        multispan_sugg(db, "consider using an iterator".to_string(), &[
+                            (pat.span, "<item>"),
+                            (arg.span, &repl),
+                        ]);
+                    });
                 }
             }
         }

diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs
index 411a4b11c17a064a442461c96b694d81fd1f2987..6028c5c0f3a84ddde5c98c3f84dfdd1773aca3bc 100644 (file)
--- a/tests/compile-fail/for_loop.rs
+++ b/tests/compile-fail/for_loop.rs
@@ -96,23 +96,41 @@ fn main() {
     let mut vec = vec![1, 2, 3, 4];
     let vec2 = vec![1, 2, 3, 4];
     for i in 0..vec.len() {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in &vec`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in &vec {
         println!("{}", vec[i]);
     }
 
+    for i in 0..vec.len() { let _ = vec[i]; }
+    //~^ ERROR `i` is only used to index `vec`
+    //~| HELP consider
+    //~| HELP consider
+    //~| SUGGESTION for <item> in &vec { let _ = vec[i]; }
+
     // ICE #746
     for j in 0..4 {
         //~^ ERROR `j` is only used to index `STATIC`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in STATIC.iter().take(4) {
         println!("{:?}", STATIC[j]);
     }
 
     for j in 0..4 {
         //~^ ERROR `j` is only used to index `CONST`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in CONST.iter().take(4) {
         println!("{:?}", CONST[j]);
     }
 
     for i in 0..vec.len() {
-        //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate()`
+        //~^ ERROR `i` is used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for (i, <item>) in vec.iter().enumerate() {
         println!("{} {}", vec[i], i);
     }
     for i in 0..vec.len() {      // not an error, indexing more than one variable
@@ -120,42 +138,66 @@ fn main() {
     }
 
     for i in 0..vec.len() {
-        //~^ ERROR `i` is only used to index `vec2`. Consider using `for item in vec2.iter().take(vec.len())`
+        //~^ ERROR `i` is only used to index `vec2`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec2.iter().take(vec.len()) {
         println!("{}", vec2[i]);
     }
 
     for i in 5..vec.len() {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().skip(5)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().skip(5) {
         println!("{}", vec[i]);
     }
 
     for i in 0..MAX_LEN {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(MAX_LEN)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().take(MAX_LEN) {
         println!("{}", vec[i]);
     }
 
     for i in 0...MAX_LEN {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(MAX_LEN)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().take(MAX_LEN) {
         println!("{}", vec[i]);
     }
 
     for i in 5..10 {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().take(10).skip(5) {
         println!("{}", vec[i]);
     }
 
     for i in 5...10 {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().take(10).skip(5) {
         println!("{}", vec[i]);
     }
 
     for i in 5..vec.len() {
-        //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().skip(5)`
+        //~^ ERROR `i` is used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for (i, <item>) in vec.iter().enumerate().skip(5) {
         println!("{} {}", vec[i], i);
     }
 
     for i in 5..10 {
-        //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().take(10).skip(5)`
+        //~^ ERROR `i` is used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for (i, <item>) in vec.iter().enumerate().take(10).skip(5) {
         println!("{} {}", vec[i], i);
     }