Redundant closure lint recommends invalid code

[Michael-F-Bryan]

I've found an issue in the redundant_closure lint where it'll suggest a closure which won't actually compile.

I think what's happening is clippy sees a closure that just calls a function, and because the input/output types match up it thinks the closure is redundant. However, when you get rid of the closure it fails because of lifetimes/HKT.

use std::path::Path;

fn redundant<P: AsRef<Path>>(path: P) {
    let path = path.as_ref();
    println!("{}", path.display());
}

fn use_it<F>(thunk: F)
where
    F: FnOnce(&str),
{
    thunk("/bin/bash");
}

fn main() {
    use_it(|s| redundant(s));
    // use_it(redundant); <- recommended code that doesn't compile
}

And rustc gives the following compilation error:

error[E0631]: type mismatch in function arguments
  --> src/main.rs:17:5
   |
3  | fn redundant<P: AsRef<Path>>(path: P) {
   | ------------------------------------- found signature of `fn(_) -> _`
...
17 |     use_it(redundant); //<- recommended code that doesn't compile
   |     ^^^^^^ expected signature of `for<'r> fn(&'r str) -> _`
   |
note: required by `use_it`

...

error[E0271]: type mismatch resolving `for<'r> <fn(_) {redundant::<_>} as std::ops::FnOnce<(&'r str,)>>::Output == ()`
  --> src/main.rs:17:5
   |
17 |     use_it(redundant); //<- recommended code that doesn't compile
   |     ^^^^^^ expected bound lifetime parameter, found concrete lifetime
   |
note: required by `use_it`

(playground)

This may also be a simpler example for #1439.

[snoyberg]

I may have a simpler example of the same issue:

fn main() {
    let msg = (|| return "Hello, world!")();
    println!("{}", msg);
}

[kiljacken]

I've just run into the issue with the following code sample: Playground

[jonhoo]

Sadly this also matches the function signature of Option::map, which means that clippy will recommend replacing

foo.map(|x| f(x))

with

foo.map(f)

even when f is, say, Box<dyn Fn>, which would not compile.

[lopopolo]

I have run into another error with this lint: Clippy is suggesting for me to replace a call with a function that is not exported into my scope.

error: redundant closure found
   --> punchtop-playlist/src/fs/mod.rs:151:21
    |
151 |             .filter(|img| img.is_some())?;
    |                     ^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `neguse_types::image::Image::is_some`

img is a neguse_types::image::Image and is set using a function called neguse_taglib::get_front_cover(&self.path). neguse_taglib depends on neguse_types but does not export any of the structs.

Clippy's suggestion is not possible to implement without depending on an additional crate.

[zrneely]

Here's another example:

type Constructor = unsafe extern "C" fn() -> *mut Foo;
// magically create an instance of "Constructor" (in my case I'm dynamically loading it)
let constructor: libloading::Symbol<Constructor> = ...;
let result: *mut Foo = std::panic::catch_unwind(|| constructor()).map_err(...)?;

Clippy recommends passing "constructor" directly into std::panic::catch_unwind, which results in a compilation failure. Note that Symbol<T> has a deref impl for T, but manually doing the deref also results in a compilation failure:

// this does not compile
let result = std::panic::catch_unwind(*constructor).map_err(...)?;

(I have since realized that it's useless to catch_unwind across an FFI boundary 😄)

[baumanj]

I may have a simpler example of the same issue:

fn main() {
    let msg = (|| return "Hello, world!")();
    println!("{}", msg);
}

I'm not sure that's the same issue, @snoyberg. Or, at least I'm not able to reproduce it with that code. When I run clippy on that in the playground, I get a clippy::needless_return, not redundant_closure. If I remove the return, I get

warning: Try not to call a closure in the expression where it is declared.
 --> src/main.rs:2:15
  |
2 |     let msg = (|| "Hello, world!")();
  |               ^^^^^^^^^^^^^^^^^^^^^^ help: Try doing something like: : `"Hello, world!"`
  |
  = note: #[warn(clippy::redundant_closure_call)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call

But implementing that suggestion works just fine. Am I missing something?

[hbooth]

I created a toy example of an issue I encountered today:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=7f4def58eeb43e602d521ad6c974b09c

[flip1995]

Thanks for the example, but writing result.map_err(MyError::AKind) compiles fine. Am I missing something?

[hbooth]

Apologies, it was apparently I that was missing something. I must have used improper syntax (likely appending a () to the end: result.map_err(MyError::AKind())).

[silmeth]

Just encountered the exact same issue, just with a bit different types. I have a function that takes a check on Cow<str>, similar to this:

fn check_string_helper<F>(validate: F) where F: Fn(Cow<str>) -> bool {
    let value: Cow<str> = Cow::Owned("foo".into());
    if validate(value) {
        println!("OK");
    } else {
        println!("Wrong");
    }
}

And I try pass it a validation function that takes generic AsRef<str>, eg.:

pub fn validate_regex<S: AsRef<str>>(s: S) -> bool {
    Regex::new(s.as_ref()).is_ok()
}

Passing closure |s| validate_regex(s) works but triggers redundant_closure lint, passing validate_regex as suggested doesn’t compile with expected signature of `for<'r> fn(std::borrow::Cow<'r, str>) -> _` and found signature of `fn(_) -> _` . Link to Playground.

[EstebanBorai]

My case looks like the following:

I'm using Yew and I'm having the same warning when writing:

let set_current_tab_callback = self
    .link
    .callback(|next_tab: usize| Msg::SetActive(next_tab));

The warning thats being printed is:

 error: redundant closure found
##[error]  --> src/components/github_radar/mod.rs:57:23
   |
57 |             .callback(|next_tab: usize| Msg::SetActive(next_tab));
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `Msg::SetActive`
   |
   = note: `-D clippy::redundant-closure` implied by `-D warnings`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

And If I follow the warning, then I get:

expected a `std::ops::Fn<(_,)>` closure, found `components::github_radar::Msg`
the trait `std::ops::Fn<(_,)>` is not implemented for `components::github_radar::Msg`rustc(E0277)