fixed false positives (at the cost of some false negatives)

This commit is contained in:
llogiq 2015-08-14 14:21:05 +02:00
parent 1118f97691
commit 5d99ebec72
2 changed files with 64 additions and 34 deletions

View File

@ -19,41 +19,58 @@ impl LintPass for EtaPass {
}
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
if let ExprClosure(_, ref decl, ref blk) = expr.node {
if !blk.stmts.is_empty() {
// || {foo(); bar()}; can't be reduced here
return;
}
if let Some(ref ex) = blk.expr {
if let ExprCall(ref caller, ref args) = ex.node {
if args.len() != decl.inputs.len() {
// Not the same number of arguments, there
// is no way the closure is the same as the function
return;
}
for (ref a1, ref a2) in decl.inputs.iter().zip(args) {
if let PatIdent(_, ident, _) = a1.pat.node {
// XXXManishearth Should I be checking the binding mode here?
if let ExprPath(None, ref p) = a2.node {
if p.segments.len() != 1 {
// If it's a proper path, it can't be a local variable
return;
}
if p.segments[0].identifier != ident.node {
// The two idents should be the same
return
}
} else {
match &expr.node {
&ExprCall(_, ref args) |
&ExprMethodCall(_, _, ref args) => {
for arg in args {
check_closure(cx, &*arg)
}
},
_ => (),
}
}
}
fn is_adjusted(cx: &Context, e: &Expr) -> bool {
cx.tcx.tables.borrow().adjustments.get(&e.id).is_some()
}
fn check_closure(cx: &Context, expr: &Expr) {
if let ExprClosure(_, ref decl, ref blk) = expr.node {
if !blk.stmts.is_empty() {
// || {foo(); bar()}; can't be reduced here
return;
}
if let Some(ref ex) = blk.expr {
if let ExprCall(ref caller, ref args) = ex.node {
if args.len() != decl.inputs.len() {
// Not the same number of arguments, there
// is no way the closure is the same as the function
return;
}
if args.iter().any(|arg| is_adjusted(cx, arg)) { return; }
for (ref a1, ref a2) in decl.inputs.iter().zip(args) {
if let PatIdent(_, ident, _) = a1.pat.node {
// XXXManishearth Should I be checking the binding mode here?
if let ExprPath(None, ref p) = a2.node {
if p.segments.len() != 1 {
// If it's a proper path, it can't be a local variable
return;
}
if p.segments[0].identifier != ident.node {
// The two idents should be the same
return
}
} else {
return
}
} else {
return
}
span_lint(cx, REDUNDANT_CLOSURE, expr.span,
&format!("redundant closure found. Consider using `{}` in its place",
expr_to_string(caller))[..])
}
span_lint(cx, REDUNDANT_CLOSURE, expr.span,
&format!("redundant closure found. Consider using `{}` in its place",
expr_to_string(caller))[..])
}
}
}

View File

@ -4,18 +4,31 @@
#![deny(redundant_closure)]
fn main() {
let a = |a, b| foo(a, b);
let a = Some(1u8).map(|a| foo(a));
//~^ ERROR redundant closure found. Consider using `foo` in its place
let c = |a, b| {1+2; foo}(a, b);
meta(|a| foo(a));
//~^ ERROR redundant closure found. Consider using `foo` in its place
let c = Some(1u8).map(|a| {1+2; foo}(a));
//~^ ERROR redundant closure found. Consider using `{ 1 + 2; foo }` in its place
let d = |a, b| foo((|c, d| foo2(c,d))(a,b), b);
//~^ ERROR redundant closure found. Consider using `foo2` in its place
let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted?
all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted
}
fn foo(_: u8, _: u8) {
fn meta<F>(f: F) where F: Fn(u8) {
f(1u8)
}
fn foo(_: u8) {
}
fn foo2(_: u8, _: u8) -> u8 {
fn foo2(_: u8) -> u8 {
1u8
}
fn all<X, F>(x: &[X], y: &X, f: F) -> bool
where F: Fn(&X, &X) -> bool {
x.iter().all(|e| f(e, y))
}
fn below(x: &u8, y: &u8) -> bool { x < y }