Suggest flatten over filter(Option::is_some)

[flip1995]

What it does

Checks for usage of filter(Option::is_some) or filter(Result::is_ok) on an Iterator.

This is more succinctly written with just flatten().

Careful: The suggestion must be MaybeIncorrect, as this changes the type of the iterator from Item = Option<T> to Item = T.

good-first-issue instructions: This lint should be quite easy to add as another clippy_lints/src/methods/ lint. All of the parts required to check for this should already be there and can be reused.

Advantage

• Shorter code
• Potentially more readable code in following methods in the iterator chain:

  .filter(Option::is_some).map(|x| /*some code*/ x.unwrap() /*some code*/)
  // becomes
  .flatten().map(|x| /*some code*/ x /*some code*/)

Drawbacks

Changes the type of the iterator chain. This will lead to more refactorings required down the chain. But I think in most cases this will benefit the code complexity overall.

Example

v.into_iter().filter(Option::is_some);
v.into_iter().filter(Result::is_ok);

Could be written as:

v.into_iter().flatten();

Playground

[PartiallyUntyped]

@rustbot claim

[y21]

🤔 This almost exists as option_filter_map, but that only lints if there's a .map(Option::unwrap) to avoid the change of types

[PartiallyUntyped]

Should I extend and/or possibly rename?

[y21]

IMO it could make sense to have this as a new lint, given that this changes types of iterator chains and may not be as applicable as option_filter_map (which should always work in any case without refactoring). But if it becomes a new lint, then it should check that the parent expr of .filter() isn't a call to .map(), so as to avoid linting twice on the same expression but with different fixes. Renaming a lint also requires deprecating the old name.
Maybe others have a different opinion on this though

[PartiallyUntyped]

@y21 maybe I am missing something but there doesn't seem to be a result_filter_map to capture the mirror case.

[PartiallyUntyped]

How do you think this should be tackled in light of the missing mirror?

Maybe I can add both the mirror and this?

[flip1995]

IMO it does make sense to add a mirror for the option_filter_map. Those 2 things should be split into 2 PRs though.