Improve NEEDLESS_RANGE_LOOP error reporting

This commit is contained in:
mcarton 2016-07-01 18:44:59 +02:00
parent e613c8b492
commit 9bd7fa05e0
No known key found for this signature in database
GPG key ID: 5E427C794CBA45E8
2 changed files with 73 additions and 30 deletions

View file

@ -13,7 +13,7 @@ use std::borrow::Cow;
use std::collections::HashMap; use std::collections::HashMap;
use syntax::ast; 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, span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher,
walk_ptrs_ty}; walk_ptrs_ty};
use utils::paths; use utils::paths;
@ -377,17 +377,16 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
}; };
if visitor.nonindex { if visitor.nonindex {
span_lint(cx, span_lint_and_then(cx,
NEEDLESS_RANGE_LOOP, NEEDLESS_RANGE_LOOP,
expr.span, expr.span,
&format!("the loop variable `{}` is used to index `{}`. Consider using `for ({}, \ &format!("the loop variable `{}` is used to index `{}`", ident.node, indexed),
item) in {}.iter().enumerate(){}{}` or similar iterators", |db| {
ident.node, multispan_sugg(db, "consider using an iterator".to_string(), &[
indexed, (pat.span, &format!("({}, <item>)", ident.node)),
ident.node, (arg.span, &format!("{}.iter().enumerate(){}{}", indexed, take, skip)),
indexed, ]);
take, });
skip));
} else { } else {
let repl = if starts_at_zero && take.is_empty() { let repl = if starts_at_zero && take.is_empty() {
format!("&{}", indexed) 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) format!("{}.iter(){}{}", indexed, take, skip)
}; };
span_lint(cx, span_lint_and_then(cx,
NEEDLESS_RANGE_LOOP, NEEDLESS_RANGE_LOOP,
expr.span, expr.span,
&format!("the loop variable `{}` is only used to index `{}`. \ &format!("the loop variable `{}` is only used to index `{}`.", ident.node, indexed),
Consider using `for item in {}` or similar iterators", |db| {
ident.node, multispan_sugg(db, "consider using an iterator".to_string(), &[
indexed, (pat.span, "<item>"),
repl)); (arg.span, &repl),
]);
});
} }
} }
} }

View file

@ -96,23 +96,41 @@ fn main() {
let mut vec = vec![1, 2, 3, 4]; let mut vec = vec![1, 2, 3, 4];
let vec2 = vec![1, 2, 3, 4]; let vec2 = vec![1, 2, 3, 4];
for i in 0..vec.len() { 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]); 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 // ICE #746
for j in 0..4 { for j in 0..4 {
//~^ ERROR `j` is only used to index `STATIC` //~^ ERROR `j` is only used to index `STATIC`
//~| HELP consider
//~| HELP consider
//~| SUGGESTION for <item> in STATIC.iter().take(4) {
println!("{:?}", STATIC[j]); println!("{:?}", STATIC[j]);
} }
for j in 0..4 { for j in 0..4 {
//~^ ERROR `j` is only used to index `CONST` //~^ ERROR `j` is only used to index `CONST`
//~| HELP consider
//~| HELP consider
//~| SUGGESTION for <item> in CONST.iter().take(4) {
println!("{:?}", CONST[j]); println!("{:?}", CONST[j]);
} }
for i in 0..vec.len() { 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); println!("{} {}", vec[i], i);
} }
for i in 0..vec.len() { // not an error, indexing more than one variable 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() { 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]); println!("{}", vec2[i]);
} }
for i in 5..vec.len() { 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]); println!("{}", vec[i]);
} }
for i in 0..MAX_LEN { 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]); println!("{}", vec[i]);
} }
for i in 0...MAX_LEN { 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]); println!("{}", vec[i]);
} }
for i in 5..10 { 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]); println!("{}", vec[i]);
} }
for i in 5...10 { 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]); println!("{}", vec[i]);
} }
for i in 5..vec.len() { 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); println!("{} {}", vec[i], i);
} }
for i in 5..10 { 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); println!("{} {}", vec[i], i);
} }