clippy::redundant_closure suggestion unnecessarily complicated

[therealprof]

Clippy rightfully suggests to simplify a map with a as_str () in the closure to us as_str directly. However the given suggestion is a bit too complicated and verbose given that the String module is always imported:

warning: redundant closure found
   --> src/main.rs:317:30
    |
317 |         version.as_ref().map(|v| v.as_str()),
    |                              ^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::String::as_str`
    |
    = 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

It would be gread if the lint would say:

^^^^^^^^^^^^^^ help:  remove closure as shown: `String::as_str`

instead.

The version is:

clippy 0.0.212 (1fac3808 2019-02-20)

[cecton]

I got this similar issue today:

error: redundant closure found
   --> src/cli/commands.rs:698:54
    |
698 |                     break attributes.into_iter().map(|x| x.unwrap());
    |                                                      ^^^^^^^^^^^^^^ help: remove closure as shown: `std::result::Result::unwrap`
    |
    = 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

Even though I do think redundant closures are bad, doing the change proposed by Clippy makes the code less readable imho. I don't really want to disable the rule since the use case in the following example is a valid one so I wish to keep it:

xs.map(|x| foo(x))

Note: I cannot import std::result::Result in this module because my module has its own Result already.

Source code on: https://github.com/cecton/gptman/blob/d64b8c7a3403ddf2ee7bb5f864be88168b2f9d41/src/cli/commands.rs#L698

[mitsuhiko]

I'm not sure I prefer having String::as_str over |x| x.as_str(). Is this a new lint I just never noticed before? I get this constantly now.

[Manishearth]

The lint is old, but it did get smarter recently. It doesn't seem too good at picking strings for suggestions though.

Might be worth splitting out the code dealing with methods as a separate lint

[therealprof]

@mitsuhiko I'm kind of in the same boat, I actually prefer |x| x.as_str(). I'm not too keen on disabling this lint entirely though...

Maybe it'd be possible to whitelist certain cases?