-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
detect redundant nested match #13798
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,6 +108,28 @@ fn check_arm<'tcx>( | |
"the outer pattern can be modified to include the inner pattern", | ||
); | ||
}); | ||
} else if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr) | ||
&& let IfLetOrMatch::Match(inner_scrutinee, inner_arms, ..) = inner | ||
&& outer_pat.span.eq_ctxt(inner_scrutinee.span) | ||
&& let Some(inner_binding_id) = path_to_local(peel_ref_operators(cx, inner_scrutinee)) | ||
&& let Some(outer_binding_id) = path_to_local(peel_ref_operators(cx, outer_then_body)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this pattern here unreachable, considering that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be better if this lint rewrote the nested |
||
// check if the inner match can be collapsed into the outer match | ||
&& inner_binding_id == outer_binding_id | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. checks if the outer |
||
&& outer_guard.is_none_or(|e| !is_local_used(cx, e, inner_binding_id)) | ||
&& !inner_arms | ||
.iter() | ||
.any(|arm| is_local_used(cx, arm.body, inner_binding_id)) | ||
{ | ||
let msg = "this match can be collapsed into the outer match"; | ||
span_lint_and_then(cx, COLLAPSIBLE_MATCH, inner_expr.span, msg, |diag| { | ||
let mut help_span = MultiSpan::from_spans(vec![outer_pat.span, inner_scrutinee.span]); | ||
help_span.push_span_label(outer_pat.span, "outer pattern"); | ||
help_span.push_span_label(inner_scrutinee.span, "inner pattern"); | ||
diag.span_help( | ||
help_span, | ||
"the outer pattern can be modified to include the inner pattern", | ||
); | ||
}); | ||
} | ||
} | ||
|
||
|
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 conditional branch added this time could have been written by nesting
if
s together with the conditional branch above, but I have not done so for the sake of readability (the former condition is long).