Only handle ranges starting with 0 for needless_range_loop (fixes #279)

This commit is contained in:
Manish Goregaokar 2015-09-02 16:11:51 +05:30
parent eb3b9b35da
commit 73c34e12b3
2 changed files with 30 additions and 20 deletions

View file

@ -37,26 +37,32 @@ impl LintPass for LoopsPass {
if let Some((pat, arg, body)) = recover_for_loop(expr) { if let Some((pat, arg, body)) = recover_for_loop(expr) {
// check for looping over a range and then indexing a sequence with it // check for looping over a range and then indexing a sequence with it
// -> the iteratee must be a range literal // -> the iteratee must be a range literal
if let ExprRange(_, _) = arg.node { if let ExprRange(Some(ref l), _) = arg.node {
// the var must be a single name // Range should start with `0`
if let PatIdent(_, ref ident, _) = pat.node { if let ExprLit(ref lit) = l.node {
let mut visitor = VarVisitor { cx: cx, var: ident.node.name, if let LitInt(0, _) = lit.node {
indexed: HashSet::new(), nonindex: false };
walk_expr(&mut visitor, body); // the var must be a single name
// linting condition: we only indexed one variable if let PatIdent(_, ref ident, _) = pat.node {
if visitor.indexed.len() == 1 { let mut visitor = VarVisitor { cx: cx, var: ident.node.name,
let indexed = visitor.indexed.into_iter().next().expect( indexed: HashSet::new(), nonindex: false };
"Len was nonzero, but no contents found"); walk_expr(&mut visitor, body);
if visitor.nonindex { // linting condition: we only indexed one variable
span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!( if visitor.indexed.len() == 1 {
"the loop variable `{}` is used to index `{}`. Consider using \ let indexed = visitor.indexed.into_iter().next().expect(
`for ({}, item) in {}.iter().enumerate()` or similar iterators", "Len was nonzero, but no contents found");
ident.node.name, indexed, ident.node.name, indexed)); if visitor.nonindex {
} else { span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!(
span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!( "the loop variable `{}` is used to index `{}`. Consider using \
"the loop variable `{}` is only used to index `{}`. \ `for ({}, item) in {}.iter().enumerate()` or similar iterators",
Consider using `for item in &{}` or similar iterators", ident.node.name, indexed, ident.node.name, indexed));
ident.node.name, indexed, indexed)); } else {
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.name, indexed, indexed));
}
}
} }
} }
} }

View file

@ -30,6 +30,10 @@ fn main() {
println!("{} {}", vec[i], vec2[i]); println!("{} {}", vec[i], vec2[i]);
} }
for i in 5..vec.len() { // not an error, not starting with 0
println!("{}", vec[i]);
}
for _v in vec.iter() { } //~ERROR it is more idiomatic to loop over `&vec` 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_mut() { } //~ERROR it is more idiomatic to loop over `&mut vec`