rust-clippy
rust-clippy copied to clipboard
rust-lang
Suggest removing `filter_map` for `Iterator::filter_map(|x| Some(x))`
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
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
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.
@rustbot claim
Assigning this to me as I am working on the sibling issue.
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 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
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
@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
