From 9bd7fa05e0fd3049e74a900e39c17cd9d6ebfbbc Mon Sep 17 00:00:00 2001 From: mcarton Date: Fri, 1 Jul 2016 18:44:59 +0200 Subject: [PATCH] Improve `NEEDLESS_RANGE_LOOP` error reporting --- clippy_lints/src/loops.rs | 41 +++++++++++----------- tests/compile-fail/for_loop.rs | 62 ++++++++++++++++++++++++++++------ 2 files changed, 73 insertions(+), 30 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 1a6ac75eb37..0f9d030c2f2 100644 --- 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!("({}, )", 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, ""), + (arg.span, &repl), + ]); + }); } } } diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index 411a4b11c17..6028c5c0f3a 100644 --- 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 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 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 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 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, ) 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 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 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 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 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 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 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, ) 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, ) in vec.iter().enumerate().take(10).skip(5) { println!("{} {}", vec[i], i); } -- 2.44.0