Merge pull request #280 from Manishearth/fx-279

Only handle ranges starting with 0 for needless_range_loop (fixes #279)
This commit is contained in:
llogiq 2015-09-02 12:44:21 +02:00
commit a9869e6fe4
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) {
// check for looping over a range and then indexing a sequence with it
// -> the iteratee must be a range literal
if let ExprRange(_, _) = arg.node {
// the var must be a single name
if let PatIdent(_, ref ident, _) = pat.node {
let mut visitor = VarVisitor { cx: cx, var: ident.node.name,
indexed: HashSet::new(), nonindex: false };
walk_expr(&mut visitor, body);
// linting condition: we only indexed one variable
if visitor.indexed.len() == 1 {
let indexed = visitor.indexed.into_iter().next().expect(
"Len was nonzero, but no contents found");
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.name, indexed, ident.node.name, 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));
if let ExprRange(Some(ref l), _) = arg.node {
// Range should start with `0`
if let ExprLit(ref lit) = l.node {
if let LitInt(0, _) = lit.node {
// the var must be a single name
if let PatIdent(_, ref ident, _) = pat.node {
let mut visitor = VarVisitor { cx: cx, var: ident.node.name,
indexed: HashSet::new(), nonindex: false };
walk_expr(&mut visitor, body);
// linting condition: we only indexed one variable
if visitor.indexed.len() == 1 {
let indexed = visitor.indexed.into_iter().next().expect(
"Len was nonzero, but no contents found");
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.name, indexed, ident.node.name, 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]);
}
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_mut() { } //~ERROR it is more idiomatic to loop over `&mut vec`