-
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
Fix FP on if_then_some_else_none
when there is early return
#7980
Conversation
r? @xFrednet (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.
The implementation looks good to me.
For future reference: The example in the issue could in theory still use bool::then()
and then call flatten on the result like this(playground)
fn f2(b: bool, v: Option<()>) -> Option<()> {
b.then(|| {
v?;
Some(())
})
.flatten()
}
However, this only works if the early return is also inside the return expression of function. I like this solution implemented in this PR. Just wanted to document it for future reference 🙃
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.
Looks good, only one small formatting NIT 😅
span_lint_and_help( | ||
cx, | ||
IF_THEN_SOME_ELSE_NONE, | ||
expr.span, | ||
"this could be simplified with `bool::then`", | ||
None, | ||
&help, | ||
); | ||
); |
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.
rustfmt sadly doesn't work inside macros. Could you fix this indention and the one in line 103? Then it's all good 👍
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.
Hm, I thought that this change was because of rustfmt 🤔
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.
These indentations also often happen in my editor, when I move code around. It's good that rustfmt is really solid otherwise, and this might be fixed when Rust supports if let
chains in the future 🙃
Looks good to me, thank you for the bug fix! 🙃 @bors r+ |
📌 Commit ec3d1c8 has been approved by |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
closes #7870
changelog: [
if_then_some_else_none
] now does not fire when there is early return