Rust-clippy: New lint: map to Option followed by flatten

Created on 3 Sep 2019  路  10Comments  路  Source: rust-lang/rust-clippy


Calling map to wrap the value in an Option and flatten right after is equivalent to filter_map.

A-complexity A-suggestion L-lint good-first-issue

All 10 comments

Playground

fn main() {
    let v = [10, 20, 30];

    let filtered1 = v
        .iter()
        .map(|x| if *x > 10 { Some(x) } else { None })
        .flatten()
        .collect::<Vec<_>>();

    let filtered2 = v
        .iter()
        .filter_map(|x| if *x > 10 { Some(x) } else { None })
        .collect::<Vec<_>>();

    assert_eq!(filtered1, filtered2);
}

This can be implemented in clippy_lints/src/methods/mod.rs, similar to multiple other lints in this module.

I would like to work on this as a first issue.

Awesome, don't hesitate to ask questions, if you have any!

I am not sure whether this lint would be EarlyLintPass or LateLintPass. I guess EarlyLintPass as it doesn't need to check any types, is that right?

You don't really have to care about that. You can just add another case here
https://github.com/rust-lang/rust-clippy/blob/abbb7ee12f9eca4ea21d484c052d7f1cbc73b7c2/clippy_lints/src/methods/mod.rs#L1069-L1072

and implement the lint from there.

You should create a new file in the methods module and implement it there instead of mod.rs directly. See:
https://github.com/rust-lang/rust-clippy/blob/abbb7ee12f9eca4ea21d484c052d7f1cbc73b7c2/clippy_lints/src/methods/mod.rs#L1108-L1112

manual_saturating_arithmetic


But to answer your question: This should be a LateLintPass, since you have to make sure, that the type map is called on is an Iterator

Ok, thank you for the explanation.
I have added the following to the match statement and also created the lint using declare_clippy_lint! and appended it to declare_lint_pass! inside the mod.rs file, however it doesn't seem to match over my test. I have used the example above as the test.

            ["map", "flatten"] => {
                println!("LINT CALLED");
                map_flatten_filtermap::lint(cx, expr, arg_lists[0])
            }

(I am relatively new to Rust and this is the first time I am writing a lint, sorry for asking so much)

From https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md#lint-declaration:

Next we need to run util/dev update_lints to register the lint in various places, mainly in clippy_lints/src/lib.rs.

Have you done this? If not, run util/dev update_lints and try running your test again. If you have, I don't really know what could cause this from your description, so please open a WIP PR, so I can have a look at your code.

I ran it but still no luck, I have opened a WIP PR here: #4521
Thank you for all your help :)

Do we need a new lint for this? map_flatten already lints on this code. I think this would have to be modification to that lint. Just check if the map closure produces Options and recommend filter_map instead of flat_map.

I have noticed that the map_flatten lint already lints my example. I'll try to modify the other lint instead.

Was this page helpful?
0 / 5 - 0 ratings