False positive with redundant_closure and Deref

[sfackler]

The new Clippy in 1.34 makes a suggestion that doesn't work:

use std::ops::Deref;

struct Foo;

impl Deref for Foo {
    type Target = str;

    fn deref(&self) -> &str {
        "hi"
    }
}

fn main() {
    let _ = [Foo].iter().map(|s| s.to_string()).collect::<Vec<_>>();;
}

warning: redundant closure found
  --> src/main.rs:14:30
   |
14 |     let _ = [Foo].iter().map(|s| s.to_string()).collect::<Vec<_>>();
   |                              ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::ToString::to_string`
   |
   = 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

You can't remove the closure since ToString::to_string won't deref the Foo to a &str.

[yaahc]

Just to add to this, I saw a similar problem yesterday and posted about it in the clippy discord channel. Gonna copy paste the convo

ag_dubsToday at 1:02 PM
curious if i'm alone in really being frustrated by the new #[warn(clippy::redundant_closure)] lint
it seems to consistently recommend i do things that will not compile
it's totally possible i'm missing something
cc @Manishearth ?
YaahToday at 1:15 PM
^ its been recommending private namespaces
so far all the ones i ran into have worked so long as I figured out what publicly exported path for the function actually was manually but the suggested fixes are often not helpful
example

src/scfailfarmer/main.rs|77 col 14 error| redundant closure found
||    |
|| 77 |         .map(|p| p.cmd())
||    |              ^^^^^^^^^^^ help: remove closure as shown: `sysinfo::traits::ProcessExt::cmd`
||    |
||    = note: `-D clippy::redundant-closure` implied by `-D warnings`

original source

    let screens = system.get_process_by_name("SCREEN");
    screens
        .iter()
        .map(|p| p.cmd())
    ...

with suggestion

src/scfailfarmer/main.rs|77 col 23 error 603| module `traits` is private
||    |
|| 77 |         .map(sysinfo::traits::ProcessExt::cmd)
||    |                       ^^^^^^

using the properly exported path

use sysinfo::{ProcessExt, SystemExt};
...
        .map(ProcessExt::cmd)

which still doesn't work because the signature doesn't expect the level of reference this iterator uses
correct code

    screens
        .into_iter()
        .map(ProcessExt::cmd)

[weiznich]

We are seeing the same on this diesel PR.

error: redundant closure found
   --> diesel/src/pg/types/numeric.rs:113:33
    |
113 |                     .take_while(|i| i.is_zero())
    |                                 ^^^^^^^^^^^^^^^ help: remove closure as shown: `pg::types::numeric::bigdecimal::num_traits::Zero::is_zero`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

Applying the suggestion does not work because the iterator yields a &&i16 but Zero is only implemented for i16 (and is_zero takes values by reference).

[mitsuhiko]

Even when it works I'm not sure that the suggestion are actually a good idea. I don't think it makes the code more readable by using the associated methods.

[Manishearth]

(I think if you don't like it you should disable it, disabling it for everyone requires a stronger argument)

[Manishearth]

As mentioned in the other issue, one path forward may be to split out the one that deals with methods into a pedantic lint. That seems more controversial than the main lint.

[ebroto]

Closing as this was fixed by #4191
(playground)