Suggest removing filter_map for Iterator::filter_map(|x| Some(x))

[PartiallyUntyped]

Summary

When we encounter a filter_map on what is effectively an identity function followed by Some, we shouldn't recommend map(identity), but we should instead recommend completely removing filter_map.

Many thanks to @Centri3 for finding this and #12501 .

Reproducer

I tried this code:

fn main() {
    let _= vec![Some(10), None].into_iter().filter_map(|x| Some(x));
}

What I saw:

    Checking playground v0.0.1 (/playground)
warning: this `.filter_map` can be written more simply using `.map`
 --> src/main.rs:4:12
  |
4 |     let _= vec![Some(10), None].into_iter().filter_map(|x| Some(x));
  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
  = note: `#[warn(clippy::unnecessary_filter_map)]` on by default

warning: redundant closure
 --> src/main.rs:4:56
  |
4 |     let _= vec![Some(10), None].into_iter().filter_map(|x| Some(x));
  |                                                        ^^^^^^^^^^^ help: replace the closure with the function itself: `Some`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
  = note: `#[warn(clippy::redundant_closure)]` on by default

warning: `playground` (bin "playground") generated 2 warnings (run `cargo clippy --fix --bin "playground"` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.26s

What I expected to see:

    Checking playground v0.0.1 (/playground)
warning: this `.filter_map` for `|x| Some(x)` can be more succinctly written as
 --> src/main.rs:4:12
  |
4 |     let _= vec![Some(10), None].into_iter();
  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
  = note: `#[warn(clippy::unnecessary_filter_map)]` on by default

This is a follow up from #12501's discussion.

Version

Latest Nightly on playground

Additional Labels

No response

[Centri3]

PS - The original idea was for removing the Some instead of changig filter_map -> map if it's Option<Option<T>> (including references), but this is a good addition too :3

[PartiallyUntyped]

Is that machine applicable? I was thinking that removing Some for Option<Option<_>> means that it can't be followed by a map with closure Option<_> -> T, so we can't replace it.

I think it's the same rationale as to why iter_filter_is_some is not machine applicable.

[PartiallyUntyped]

I don't think this is a clippy bug, but more like an enhancement?

[Centri3]

It isn't machine applicable, yes

[PartiallyUntyped]

@rustbot claim

Assigning this to me as I am working on the sibling issue.

[PartiallyUntyped]

This should be under unnecessary_filter_map, and should not be treated as a special case of filter_map_identity.

This should be straightforward; Introduce a check in unnecessary_filter_map to see whether the closure is |x| Some(x) or simply a path to Some.

[omer-shtivi]

@rustbot claim

[belyakov-am]

@omer-shtivi are you planning on opening PR for this issue? I'm willing to take over this issue if you don't have time to do this/changed you mind

[omer-shtivi]

Hi @belyakov-am I'm still planning to do it, but it will just take me some time as I'm learning how to contribute to clippy

[wowinter13]

@m-rph @Centri3 hello there, could you assign this issue to me?
I will rework the PR that was started by @omer-shtivi
Looks like it ain't much left
~ETA: a few days

[PartiallyUntyped]

You may use @rustbot claim

[wowinter13]

@rustbot claim