Wrapping type which implements IntoIterator with std::iter::once

What it does

It warns about wrapping an IntoIterator (concrete example: Option) with std::iter::once. I'd say this is an antipattern because for example the Option already implements IntoIterator. Example of code removed as part of another 1.52 lint which was triggered in the same code (filter_map_identity): https://github.com/interledger-rs/interledger-rs/commit/7b352a6dca744bc7e529aef55616f5b80d86593f.

In the playground minified example it would recommend:

warning: extra iterator wrapping

    std::iter::once(default_route).flatten()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    help: try this: `default_route.into_iter()`

The original code case was more complicated, because there was further Iterator::map to transform the iterator into type-compatible Iterator<Item = (String, T)> in order to chain the resulting std::iter::Map<std::iter::Flatten<std::iter::once<Option<T>>>, _> with the other iterators. However, if the lint would trigger on std::iter::once being given an IntoIterator value, and recommend calling the value.into_iter() instead, it would make it obvious to the user regardless of the further use that they are now creating unnecessary wrapping.

Could be that I'm not seeing some cases corner cases for this. Perhaps this only should for std::iter::once(Option<T>).flatten() instead.

If this lint ended up being the wisest to make only for std::iter::once(Option<T>).flatten() then the case of filter_map_identity would not need to be concerned with, because that already triggers a warning.

Categories (optional)

• Kind: clippy::complexity (sounds like it at least)

What is the advantage of the recommended code over the original code

It simplifies the code and teaches the user about the type (for example: Option) implementing IntoIterator.

Not having this lint will allow patterns which only hurt readability and fail to teach the user this is extra work.

Drawbacks

I couldn't decide if this should be on both any T: IntoIterator or just when std::iter::once(Option<T>).flatten() is used. Latter might be more straightforward.

Example

(This is the same as the earlier playground link).

fn main() {
    println!("{}", example(Some(1)).count());
}

fn example<T>(default_route: Option<T>) -> impl Iterator<Item = T> {
    std::iter::once(default_route).flatten()
}

Could be written as:

fn main() {
    println!("{}", example(Some(1)).count());
}

fn example<T>(default_route: Option<T>) -> impl Iterator<Item = T> {
    default_route.into_iter()
}

Start with the linked Rust Playground example and compare the two proposed scopes: all IntoIterator values or only std::iter::once(Option).flatten(). Review existing clippy::complexity lint conventions before deciding the scope. Done means the selected pattern is diagnosed with the intended suggestion and the relevant behavior is covered by tests.