new lint: using for i in 0..x { .. vec[i] .. }
instead of iterator (fixes #3)
This commit is contained in:
parent
4400aaed43
commit
f6090909d3
@ -32,6 +32,7 @@ pub mod unicode;
|
||||
pub mod strings;
|
||||
pub mod methods;
|
||||
pub mod returns;
|
||||
pub mod loops;
|
||||
|
||||
#[plugin_registrar]
|
||||
pub fn plugin_registrar(reg: &mut Registry) {
|
||||
@ -59,6 +60,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
|
||||
reg.register_lint_pass(box returns::ReturnPass as LintPassObject);
|
||||
reg.register_lint_pass(box methods::MethodsPass as LintPassObject);
|
||||
reg.register_lint_pass(box types::LetPass as LintPassObject);
|
||||
reg.register_lint_pass(box loops::LoopsPass as LintPassObject);
|
||||
|
||||
reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
|
||||
misc::SINGLE_MATCH,
|
||||
@ -87,5 +89,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
|
||||
methods::STR_TO_STRING,
|
||||
methods::STRING_TO_STRING,
|
||||
types::LET_UNIT_VALUE,
|
||||
loops::NEEDLESS_RANGE_LOOP,
|
||||
]);
|
||||
}
|
||||
|
105
src/loops.rs
Normal file
105
src/loops.rs
Normal file
@ -0,0 +1,105 @@
|
||||
use rustc::lint::*;
|
||||
use syntax::ast::*;
|
||||
use syntax::visit::{Visitor, walk_expr};
|
||||
use std::collections::HashSet;
|
||||
|
||||
use utils::{span_lint, get_parent_expr};
|
||||
|
||||
declare_lint!{ pub NEEDLESS_RANGE_LOOP, Warn,
|
||||
"Warn about looping over a range of indices if a normal iterator would do" }
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
pub struct LoopsPass;
|
||||
|
||||
impl LintPass for LoopsPass {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array!(NEEDLESS_RANGE_LOOP)
|
||||
}
|
||||
|
||||
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
|
||||
if let Some((pat, arg, body)) = recover_for_loop(expr) {
|
||||
// the var must be a single name
|
||||
if let PatIdent(_, ref ident, _) = pat.node {
|
||||
// the iteratee must be a range literal
|
||||
if let ExprRange(_, _) = arg.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().unwrap();
|
||||
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.as_str(), indexed.as_str(),
|
||||
ident.node.name.as_str(), indexed.as_str()));
|
||||
} 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.as_str(), indexed.as_str(), indexed.as_str()));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Recover the essential nodes of a desugared for loop:
|
||||
/// `for pat in arg { body }` becomes `(pat, arg, body)`.
|
||||
fn recover_for_loop<'a>(expr: &'a Expr) -> Option<(&'a Pat, &'a Expr, &'a Expr)> {
|
||||
if_let_chain! {
|
||||
[
|
||||
let ExprMatch(ref iterexpr, ref arms, _) = expr.node,
|
||||
let ExprCall(_, ref iterargs) = iterexpr.node,
|
||||
iterargs.len() == 1,
|
||||
arms.len() == 1 && arms[0].guard.is_none(),
|
||||
let ExprLoop(ref block, _) = arms[0].body.node,
|
||||
block.stmts.is_empty(),
|
||||
let Some(ref loopexpr) = block.expr,
|
||||
let ExprMatch(_, ref innerarms, MatchSource::ForLoopDesugar) = loopexpr.node,
|
||||
innerarms.len() == 2 && innerarms[0].pats.len() == 1,
|
||||
let PatEnum(_, Some(ref somepats)) = innerarms[0].pats[0].node,
|
||||
somepats.len() == 1
|
||||
], {
|
||||
return Some((&*somepats[0],
|
||||
&*iterargs[0],
|
||||
&*innerarms[0].body));
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
struct VarVisitor<'v, 't: 'v> {
|
||||
cx: &'v Context<'v, 't>, // context reference
|
||||
var: Name, // var name to look for as index
|
||||
indexed: HashSet<Name>, // indexed variables
|
||||
nonindex: bool, // has the var been used otherwise?
|
||||
}
|
||||
|
||||
impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> {
|
||||
fn visit_expr(&mut self, expr: &'v Expr) {
|
||||
if let ExprPath(None, ref path) = expr.node {
|
||||
if path.segments.len() == 1 && path.segments[0].identifier.name == self.var {
|
||||
// we are referencing our variable! now check if it's as an index
|
||||
if_let_chain! {
|
||||
[
|
||||
let Some(parexpr) = get_parent_expr(self.cx, expr),
|
||||
let ExprIndex(ref seqexpr, _) = parexpr.node,
|
||||
let ExprPath(None, ref seqvar) = seqexpr.node,
|
||||
seqvar.segments.len() == 1
|
||||
], {
|
||||
self.indexed.insert(seqvar.segments[0].identifier.name);
|
||||
return; // no need to walk further
|
||||
}
|
||||
}
|
||||
// we are not indexing anything, record that
|
||||
self.nonindex = true;
|
||||
return;
|
||||
}
|
||||
}
|
||||
walk_expr(self, expr);
|
||||
}
|
||||
}
|
17
tests/compile-fail/for_loop.rs
Executable file
17
tests/compile-fail/for_loop.rs
Executable file
@ -0,0 +1,17 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
|
||||
#[deny(needless_range_loop)]
|
||||
fn main() {
|
||||
let vec = vec![1, 2, 3, 4];
|
||||
let vec2 = vec![1, 2, 3, 4];
|
||||
for i in 0..vec.len() { //~ERROR the loop variable `i` is only used to index `vec`.
|
||||
println!("{}", vec[i]);
|
||||
}
|
||||
for i in 0..vec.len() { //~ERROR the loop variable `i` is used to index `vec`.
|
||||
println!("{} {}", vec[i], i);
|
||||
}
|
||||
for i in 0..vec.len() { // not an error, indexing more than one variable
|
||||
println!("{} {}", vec[i], vec2[i]);
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user