Add a suggestion to replace map(f).unwrap_or(None) with and_then(f).
#2112
Conversation
Add a suggestion to replace `map(f).unwrap_or(None)` with `and_then(f)`.
clippy_lints/src/methods.rs
Outdated
| // get snippets for args to map() and unwrap_or() | ||
| let map_snippet = snippet(cx, map_args[1].span, ".."); | ||
| let unwrap_snippet = snippet(cx, unwrap_args[1].span, ".."); | ||
| // lint message | ||
| let arg = if unwrap_snippet == "None" { "None" } else { "a" }; |
There was a problem hiding this comment.
My first thought was "oh no no, don't compare snippets to raw text". But in this one case almost nothing can go wrong, since we already checked the type. Well, the only thing that can go wrong is someone creating const None: Option<Foo> = Some(Foo);. I think there's a special place in hell for anyone doing that. 😈
I'm so torn between "do it right" and "doing it right is less readable"!
There was a problem hiding this comment.
Hey sorry for horrifying you - I had no hesitant on comparing snippets to raw text 😛
There was a problem hiding this comment.
While adding more tests, I found that comparing snippets to raw text does not work in some trivial cases. I copied them from tests for OPTION_MAP_UNWRAP_OR. e.g.
let _ = opt.map(|x| Some(x + 1))
.unwrap_or({
None
});Is this acceptable, or should we "do it right"?
|
Added a comment and tests. |
|
Yay! So... now that this one is through, I noticed the lint (the original one already had this issue) does not have a structured suggestion ( |
Closes #2109.