Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

redundant closure lint fails for FnMut where FnOnce is expected #3898

Closed
gnzlbg opened this issue Mar 20, 2019 · 2 comments · Fixed by #4008
Closed

redundant closure lint fails for FnMut where FnOnce is expected #3898

gnzlbg opened this issue Mar 20, 2019 · 2 comments · Fixed by #4008
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Mar 20, 2019

This compiles (playground):

pub fn bar(mut x: Box<dyn FnMut()>) {
    foo(|| x())
}

pub fn foo<T: FnOnce()>(_: T) {}

but clippy suggests:

warning: redundant closure found
 --> src/lib.rs:2:9
  |
2 |     foo(|| x())
  |         ^^^^^^ help: remove closure as shown: `x`
  |
  = note: #[warn(clippy::redundant_closure)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

which fails to compile with (playground):

error[E0277]: expected a `std::ops::FnOnce<()>` closure, found `std::boxed::Box<dyn std::ops::FnMut()>`
 --> src/lib.rs:2:5
  |
2 |     foo(x)
  |     ^^^ expected an `FnOnce<()>` closure, found `std::boxed::Box<dyn std::ops::FnMut()>`
  |
  = help: the trait `std::ops::FnOnce<()>` is not implemented for `std::boxed::Box<dyn std::ops::FnMut()>`
  = note: wrap the `std::boxed::Box<dyn std::ops::FnMut()>` in a closure with no arguments: `|| { /* code */ }
note: required by `foo`
 --> src/lib.rs:5:1
  |
5 | pub fn foo<T: FnOnce()>(_: T) {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error
@mati865
Copy link
Contributor

mati865 commented Mar 20, 2019

cc #1439

@phansch phansch added C-bug Category: Clippy is not doing the correct thing L-suggestion Lint: Improving, adding or fixing lint suggestions I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied labels Mar 20, 2019
@g-bartoszek
Copy link

I'll try to fix it.

bors added a commit that referenced this issue Apr 25, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants