-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New lint: map to Option followed by flatten #4496
Comments
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 |
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 |
You don't really have to care about that. You can just add another case here rust-clippy/clippy_lints/src/methods/mod.rs Lines 1069 to 1072 in abbb7ee
and implement the lint from there. You should create a new file in the methods module and implement it there instead of rust-clippy/clippy_lints/src/methods/mod.rs Lines 1108 to 1112 in abbb7ee
But to answer your question: This should be a |
Ok, thank you for the explanation. ["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) |
Have you done this? If not, run |
I ran it but still no luck, I have opened a WIP PR here: #4521 |
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 |
I have noticed that the map_flatten lint already lints my example. I'll try to modify the other lint instead. |
…flip1995 Handle mapping to Option in `map_flatten` lint Fixes rust-lang#4496 The existing [`map_flatten`](https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten) lint suggests changing `expr.map(...).flatten()` to `expr.flat_map(...)` when `expr` is `Iterator`. This PR changes suggestion to `filter_map` instead of `flat_map` when mapping to `Option`, because it is more natural Also here are some questions: * If expression has type which implements `Iterator` trait (`match_trait_method(cx, expr, &paths::ITERATOR) == true`), how can I get type of iterator elements? Currently I use return type of closure inside `map`, but probably it is not good way * I would like to change suggestion range to cover only `.map(...).flatten()`, that is from: ``` let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `vec![5_i8; 6].into_iter().flat_map ``` to ``` let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)` ``` Is it ok? * Is `map_flatten` lint intentionally in `pedantic` category, or could it be moved to `complexity`? changelog: Handle mapping to Option in [`map_flatten`](https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten) lint
Calling map to wrap the value in an
Option
and flatten right after is equivalent to filter_map.The text was updated successfully, but these errors were encountered: