Fix false positives of needless_match #9092
Conversation
|
r? @llogiq (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| fn check_all_arms(cx: &LateContext<'_>, match_expr: &Expr<'_>, arms: &[Arm<'_>]) -> bool { | ||
| for arm in arms { | ||
| let arm_expr = peel_blocks_with_stmt(arm.body); | ||
| if let PatKind::Wild = arm.pat.kind { |
There was a problem hiding this comment.
This should also return false for arm.guard.is_some()
There was a problem hiding this comment.
@Jarcho
I think my code already works for arms with if guards.
If not, could you raise some cases my code dosen't catch?
pat => expr_same_as_patpat if some_expr => expr_same_as_pat_ => scrutinee_ if some_expr => scrutinee
For now, other cases than these four returns false.
There was a problem hiding this comment.
Match guards can have side-effects in them. Horrible example:
match 0 {
0 if { println!("guard"); false } => 0,
_ => 0,
}Expr::can_have_side_effects would also solve the problem.
There was a problem hiding this comment.
Ohhh...I forgot side effects.
OKay. I will use Expr::can_have_side_effects.
|
thank you for your review @Jarcho . |
llogiq
added
S-waiting-on-author
|
@rustbot label -S-waiting-on-author +S-waiting-on-review |
rustbot
added
S-waiting-on-review
|
hmm..test failed though it passes in the local environment. |
|
Try rebasing. Possibly a conflict with #8356. |
tamaroning
force-pushed
the
fix-needless-match
branch
from
3b520bc to
45084ee
Compare
|
I think you just need to bless the (And sorry for being away for so long, thanks to @Jarcho for picking up the slack) |
|
Thank you. My commit somehow does not contained .stderr but fixed :) |
|
Thank you! @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
6 participants
closes: #9084
made needless_match take into account arm in the form of
_ if => ...changelog: none