Skip to content

Commit

Permalink
Auto merge of #4008 - g-bartoszek:boxed-fnmut, r=phansch
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Apr 25, 2019
2 parents 6a0105e + 4f801a2 commit 910d538
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 1 deletion.
4 changes: 4 additions & 0 deletions clippy_lints/src/eta_reduction.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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());
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/eta.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
16 changes: 16 additions & 0 deletions tests/ui/eta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
14 changes: 13 additions & 1 deletion tests/ui/eta.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 910d538

Please sign in to comment.