Fix map_unwrap_or fail to cover Result::unwrap_or
#15718
Conversation
rustbot
added
the
S-waiting-on-review
|
Lintcheck changes for 1e16651
This comment will be updated if you push new changes |
|
Unless you have a good reason not to, please use |
profetia
force-pushed
the
issue15714
branch
from
d89877a to
b08fb14
Compare
|
Moved. Thank you! |
profetia
force-pushed
the
issue15714
branch
from
b08fb14 to
6375703
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Also fixes #15713 |
profetia
force-pushed
the
issue15714
branch
from
0b7431e to
48b8dd9
Compare
|
Also closes #15752 |
There was a problem hiding this comment.
If possible, you should squash together the two commits "Apply map_unwrap to Clippy itself" and make it the first commit). This way, after every commit, Clippy tests still pass (including dogfood), and it makes it easy to bisect should something go wrong. Then each "fix" commit can be put on top.
profetia
force-pushed
the
issue15714
branch
from
48b8dd9 to
761ecf0
Compare
|
Updated. Thank you |
| // is_some_and is stabilised && `unwrap_or` argument is false; suggest `is_some_and` instead | ||
| let suggest_is_some_and = matches!(&unwrap_arg.kind, ExprKind::Lit(lit) | ||
| if matches!(lit.node, rustc_ast::LitKind::Bool(false))) | ||
| && msrv.meets(cx, msrvs::OPTION_RESULT_IS_VARIANT_AND); |
There was a problem hiding this comment.
Shouldn't this be left to manual_is_variant_and? For example, this suggests is_some_and but not is_none_or
There was a problem hiding this comment.
I don't think manual_is_variant_and checks for unwrap_or(false)
There was a problem hiding this comment.
No, I mean let this lint turn map().unwrap_or_else(bool, ..) into map_or_else(bool, ..) first, and then manual_is_variant_and would transform that further
There was a problem hiding this comment.
I'm ok with this behavior. Do you want it to be handled in one transform?
There was a problem hiding this comment.
Sorry for not explaining myself clearly^^ Yes, I also prefer this to be handled in two transforms, so what I'm proposing is to remove the suggest_is_some_and logic from this lint
Closes #15713
Closes #15714
Closes #15752
changelog: [
map_unwrap_or] add cover forResult::unwrap_or