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?