From 49f6eb88d32d70057cc50e259f9bd59a81aebd5f Mon Sep 17 00:00:00 2001
From: llogiq <bogusandre@gmail.com>
Date: Fri, 14 Aug 2015 14:21:05 +0200
Subject: [PATCH] fixed false positives (at the cost of some false negatives)

---
 src/eta_reduction.rs      | 73 ++++++++++++++++++++++++---------------
 tests/compile-fail/eta.rs | 25 ++++++++++----
 2 files changed, 64 insertions(+), 34 deletions(-)

diff --git a/src/eta_reduction.rs b/src/eta_reduction.rs
index 484d46ddc21..e0d4182081f 100644
--- a/src/eta_reduction.rs
+++ b/src/eta_reduction.rs
@@ -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))[..])
             }
         }
     }
diff --git a/tests/compile-fail/eta.rs b/tests/compile-fail/eta.rs
index 9e48ec1c3a5..bf6ecd79617 100755
--- a/tests/compile-fail/eta.rs
+++ b/tests/compile-fail/eta.rs
@@ -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 }