lint flat_map into filter_map

[KamilaBorowska]

filter_map is a more specific version of flat_map. It would be neat if Clippy recommended using filter_map instead of flat_map when the function in flat_map returns Option.

This is an example from documentation with filter_map replaced by flat_map.

fn main() {
    let a = ["1", "2", "lol"];

    let mut iter = a.iter().flat_map(|s| s.parse().ok());

    assert_eq!(iter.next(), Some(1));
    assert_eq!(iter.next(), Some(2));
    assert_eq!(iter.next(), None);
}

it would be more readable with filter_map instead of flat_map.

[killercup]

Eh, quick question: Why not just use a.iter().flat_map(|s| s.parse());? Result::iter only yield the Ok value, just like Option::iter only yields a value when it's Some.

(Yes, I like to abuse flat_map and the Iterator impls on these types.)

(And here comes the usual E-hard suggestion: Write a lint that detects unnecessary method calls, like the .ok() here.)

[clarfonthey]

IMO now that Try is stable, upstream should replace filter_map with a version that uses Try instead. That would allow use of Result as well as option.

[Manishearth]

You can't change the signatures of trait methods upstream

…

On Dec 2, 2017 8:16 AM, "Clar Roʒe" ***@***.***> wrote: IMO now that Try is stable, upstream should replace filter_map with a version that uses Try instead. That would allow use of Result as well as option. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2241 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSMYiE44RuNJHm0KXBTX95KNIyWbEks5s8XfUgaJpZM4Qln-C> .

[clarfonthey]

...right, that's correct. :/

[oli-obk]

You can't change the signatures of trait methods upstream

you can if it's generic and strictly allows more code

[Manishearth]

I'm talking about trait methods. You can do that with inherent methods. This is a trait you can implement yourself. Most don't override flat_map, but they *can*, so we can't change this. On Dec 2, 2017 12:46 PM, "Oliver Schneider" <[email protected]> wrote: You can't change the signatures of trait methods upstream you can if it's generic and strictly allows more code — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2241 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSHplrdiuK78_AMLvFh04hzmkVgLuks5s8bcOgaJpZM4Qln-C> .

[clarfonthey]

I mean, they could always do a crater run to check. Although at the moment, isn't FilterMap literally coded as a façade over FlatMap, or am I wrong?

[Manishearth]

No, that's not one of the "allowed" kinds of breakages. Crater is not a carte blanche to make breaking changes. There's a set of allowed breaking changes. We still do crater runs for those. Additionally for fixing bugs and soundless issues we are allowed to make otherwise-disallowed breakages with a crater run, but this is not a biggie.

…

On Dec 2, 2017 1:01 PM, "Clar Roʒe" ***@***.***> wrote: I mean, they *could* always do a crater run to check. Although at the moment, isn't FilterMap literally coded as a façade over FlatMap, or am I wrong? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2241 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSG7ciDup5LRK6DdPEEysO_MpI_2aks5s8bqzgaJpZM4Qln-C> .

[KamilaBorowska]

FilterMap has a better implementation of size_hint than FlatMap as it knows that next can only provide 0 or 1 element and doesn't store currently processed iterator in it as it doesn't need to do that with Option - after getting a single element, Option iterator always keeps returning None.

[llogiq]

One thing we could lint is manually constructing one-element-vec![..]s in flat_map, where using filter_map will do. Example: rust-lang/rust#55476