about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2019-04-25 18:13:47 +0000
committerbors <bors@rust-lang.org>2019-04-25 18:13:47 +0000
commit910d538ef1f643334974aa0c2f45ff44a74d92b1 (patch)
treed58d92b9b070eb7bc1eea9e6150293f6618515c2
parent6a0105e59f6b07236f894e4f4bdf05a9592d93c1 (diff)
parent4f801a278dc4ed68ef209f07fece4d8b8e99fbeb (diff)
downloadrust-910d538ef1f643334974aa0c2f45ff44a74d92b1.tar.gz
rust-910d538ef1f643334974aa0c2f45ff44a74d92b1.zip
Auto merge of #4008 - g-bartoszek:boxed-fnmut, r=phansch
Do not trigger redundant_closure for non-function types

fixes #3898

Added a check for the entity being called in the closure body to be a FnDef. This way lint does not trigger for ADTs (Box) but I'm not sure if it's correct and not too restrictive.

<!--
Thank you for making Clippy better!

We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.

If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `util/dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo fmt`

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

Delete this line and everything above before opening your PR -->

changelog: Fix false positive in `redundant_closure` pertaining to non-function types
-rw-r--r--clippy_lints/src/eta_reduction.rs4
-rw-r--r--tests/ui/eta.fixed16
-rw-r--r--tests/ui/eta.rs16
-rw-r--r--tests/ui/eta.stderr14
4 files changed, 49 insertions, 1 deletions
diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs
index 63abe47b442..60e2ab8b9b9 100644
--- a/clippy_lints/src/eta_reduction.rs
+++ b/clippy_lints/src/eta_reduction.rs
@@ -1,4 +1,5 @@
 use if_chain::if_chain;
+use matches::matches;
 use rustc::hir::*;
 use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
 use rustc::ty::{self, Ty};
@@ -65,6 +66,9 @@ fn check_closure(cx: &LateContext<'_, '_>, expr: &Expr) {
             if !(is_adjusted(cx, ex) || args.iter().any(|arg| is_adjusted(cx, arg)));
 
             let fn_ty = cx.tables.expr_ty(caller);
+
+            if matches!(fn_ty.sty, ty::FnDef(_, _) | ty::FnPtr(_) | ty::Closure(_, _));
+
             if !type_is_unsafe_function(cx, fn_ty);
 
             if compare_inputs(&mut iter_input_pats(decl, body), &mut args.into_iter());
diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed
index 7e487fe2167..bae9b09c697 100644
--- a/tests/ui/eta.fixed
+++ b/tests/ui/eta.fixed
@@ -133,3 +133,19 @@ fn divergent(_: u8) -> ! {
 fn generic<T>(_: T) -> u8 {
     0
 }
+
+fn passes_fn_mut(mut x: Box<dyn FnMut()>) {
+    requires_fn_once(|| x());
+}
+fn requires_fn_once<T: FnOnce()>(_: T) {}
+
+fn test_redundant_closure_with_function_pointer() {
+    type FnPtrType = fn(u8);
+    let foo_ptr: FnPtrType = foo;
+    let a = Some(1u8).map(foo_ptr);
+}
+
+fn test_redundant_closure_with_another_closure() {
+    let closure = |a| println!("{}", a);
+    let a = Some(1u8).map(closure);
+}
diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs
index a0d83c43825..a23da73bde7 100644
--- a/tests/ui/eta.rs
+++ b/tests/ui/eta.rs
@@ -133,3 +133,19 @@ fn divergent(_: u8) -> ! {
 fn generic<T>(_: T) -> u8 {
     0
 }
+
+fn passes_fn_mut(mut x: Box<dyn FnMut()>) {
+    requires_fn_once(|| x());
+}
+fn requires_fn_once<T: FnOnce()>(_: T) {}
+
+fn test_redundant_closure_with_function_pointer() {
+    type FnPtrType = fn(u8);
+    let foo_ptr: FnPtrType = foo;
+    let a = Some(1u8).map(|a| foo_ptr(a));
+}
+
+fn test_redundant_closure_with_another_closure() {
+    let closure = |a| println!("{}", a);
+    let a = Some(1u8).map(|a| closure(a));
+}
diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr
index 77423694f81..eb55a251bcc 100644
--- a/tests/ui/eta.stderr
+++ b/tests/ui/eta.stderr
@@ -68,5 +68,17 @@ error: redundant closure found
 LL |     let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect();
    |                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_ascii_uppercase`
 
-error: aborting due to 11 previous errors
+error: redundant closure found
+  --> $DIR/eta.rs:145:27
+   |
+LL |     let a = Some(1u8).map(|a| foo_ptr(a));
+   |                           ^^^^^^^^^^^^^^ help: remove closure as shown: `foo_ptr`
+
+error: redundant closure found
+  --> $DIR/eta.rs:150:27
+   |
+LL |     let a = Some(1u8).map(|a| closure(a));
+   |                           ^^^^^^^^^^^^^^ help: remove closure as shown: `closure`
+
+error: aborting due to 13 previous errors