-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Lint simple expressions in manual_filter_map
, manual_find_map
#8958
Conversation
r? @giraffate (rust-highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reviewing.
Overall looks good, thanks! I made one comment.
@@ -58,15 +59,15 @@ fn lint_filter_some_map_unwrap( | |||
map_arg: &hir::Expr<'_>, | |||
target_span: Span, | |||
methods_span: Span, | |||
) { | |||
) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it would be better to separate function into one that check and return bool
and one that just emit the lint. I was confused that the function name lint_filter_some_map_unwrap
return bool
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, changed it to be an is_
function
9e362ba
to
307b8cd
Compare
@bors r+ Thanks! |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: Lint simple expressions in [
manual_filter_map
], [manual_find_map
]The current comparison rules out
.find(|a| a.is_some()).map(|b| b.unwrap())
becausea
being a reference can effect more complicated expressions, this adds a simple check for that case and adds the necessary derefsThere's some overlap with
option_filter_map
solint_filter_some_map_unwrap
now returns abool
to indicate it linted