use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::visitors::LocalUsedVisitor;
-use clippy_utils::{is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq};
+use clippy_utils::higher::IfLetOrMatch;
+use clippy_utils::visitors::is_local_used;
+use clippy_utils::{is_lang_ctor, is_unit_expr, path_to_local, peel_ref_operators, SpanlessEq};
use if_chain::if_chain;
use rustc_hir::LangItem::OptionNone;
use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind};
/// };
/// }
/// ```
+ #[clippy::version = "1.50.0"]
pub COLLAPSIBLE_MATCH,
style,
"Nested `match` or `if let` expressions where the patterns may be \"collapsed\" together."
impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
- if let ExprKind::Match(_expr, arms, _source) = expr.kind {
- if let Some(wild_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
- for arm in arms {
- check_arm(arm, wild_arm, cx);
+ match IfLetOrMatch::parse(cx, expr) {
+ Some(IfLetOrMatch::Match(_, arms, _)) => {
+ if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
+ for arm in arms {
+ check_arm(cx, true, arm.pat, arm.body, arm.guard.as_ref(), Some(els_arm.body));
+ }
}
- }
+ },
+ Some(IfLetOrMatch::IfLet(_, pat, body, els)) => {
+ check_arm(cx, false, pat, body, None, els);
+ },
+ None => {},
}
}
}
-fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext<'tcx>) {
- let expr = strip_singleton_blocks(arm.body);
+fn check_arm<'tcx>(
+ cx: &LateContext<'tcx>,
+ outer_is_match: bool,
+ outer_pat: &'tcx Pat<'tcx>,
+ outer_then_body: &'tcx Expr<'tcx>,
+ outer_guard: Option<&'tcx Guard<'tcx>>,
+ outer_else_body: Option<&'tcx Expr<'tcx>>,
+) {
+ let inner_expr = strip_singleton_blocks(outer_then_body);
if_chain! {
- if let ExprKind::Match(expr_in, arms_inner, _) = expr.kind;
- // the outer arm pattern and the inner match
- if expr_in.span.ctxt() == arm.pat.span.ctxt();
- // there must be no more than two arms in the inner match for this lint
- if arms_inner.len() == 2;
- // no if guards on the inner match
- if arms_inner.iter().all(|arm| arm.guard.is_none());
+ if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr);
+ if let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner {
+ IfLetOrMatch::IfLet(scrutinee, pat, _, els) => Some((scrutinee, pat, els)),
+ IfLetOrMatch::Match(scrutinee, arms, ..) => if_chain! {
+ // if there are more than two arms, collapsing would be non-trivial
+ if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none());
+ // one of the arms must be "wild-like"
+ if let Some(wild_idx) = arms.iter().rposition(|a| arm_is_wild_like(cx, a));
+ then {
+ let (then, els) = (&arms[1 - wild_idx], &arms[wild_idx]);
+ Some((scrutinee, then.pat, Some(els.body)))
+ } else {
+ None
+ }
+ },
+ };
+ if outer_pat.span.ctxt() == inner_scrutinee.span.ctxt();
// match expression must be a local binding
// match <local> { .. }
- if let Some(binding_id) = path_to_local(peel_ref_operators(cx, expr_in));
- // one of the branches must be "wild-like"
- if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| arm_is_wild_like(cx, arm_inner));
- let (wild_inner_arm, non_wild_inner_arm) =
- (&arms_inner[wild_inner_arm_idx], &arms_inner[1 - wild_inner_arm_idx]);
- if !pat_contains_or(non_wild_inner_arm.pat);
+ if let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_scrutinee));
+ if !pat_contains_or(inner_then_pat);
// the binding must come from the pattern of the containing match arm
// ..<local>.. => match <local> { .. }
- if let Some(binding_span) = find_pat_binding(arm.pat, binding_id);
- // the "wild-like" branches must be equal
- if SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, wild_outer_arm.body);
+ if let Some(binding_span) = find_pat_binding(outer_pat, binding_id);
+ // the "else" branches must be equal
+ if match (outer_else_body, inner_else_body) {
+ (None, None) => true,
+ (None, Some(e)) | (Some(e), None) => is_unit_expr(e),
+ (Some(a), Some(b)) => SpanlessEq::new(cx).eq_expr(a, b),
+ };
// the binding must not be used in the if guard
- let mut used_visitor = LocalUsedVisitor::new(cx, binding_id);
- if match arm.guard {
- None => true,
- Some(Guard::If(expr) | Guard::IfLet(_, expr)) => !used_visitor.check_expr(expr),
+ if outer_guard.map_or(true, |(Guard::If(e) | Guard::IfLet(_, e))| !is_local_used(cx, *e, binding_id));
+ // ...or anywhere in the inner expression
+ if match inner {
+ IfLetOrMatch::IfLet(_, _, body, els) => {
+ !is_local_used(cx, body, binding_id) && els.map_or(true, |e| !is_local_used(cx, e, binding_id))
+ },
+ IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| is_local_used(cx, arm, binding_id)),
};
- // ...or anywhere in the inner match
- if !arms_inner.iter().any(|arm| used_visitor.check_arm(arm));
then {
+ let msg = format!(
+ "this `{}` can be collapsed into the outer `{}`",
+ if matches!(inner, IfLetOrMatch::Match(..)) { "match" } else { "if let" },
+ if outer_is_match { "match" } else { "if let" },
+ );
span_lint_and_then(
cx,
COLLAPSIBLE_MATCH,
- expr.span,
- "unnecessary nested match",
+ inner_expr.span,
+ &msg,
|diag| {
- let mut help_span = MultiSpan::from_spans(vec![binding_span, non_wild_inner_arm.pat.span]);
+ let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_then_pat.span]);
help_span.push_span_label(binding_span, "replace this binding".into());
- help_span.push_span_label(non_wild_inner_arm.pat.span, "with this pattern".into());
+ help_span.push_span_label(inner_then_pat.span, "with this pattern".into());
diag.span_help(help_span, "the outer pattern can be modified to include the inner pattern");
},
);
expr
}
-/// A "wild-like" pattern is wild ("_") or `None`.
-/// For this lint to apply, both the outer and inner match expressions
-/// must have "wild-like" branches that can be combined.
+/// A "wild-like" arm has a wild (`_`) or `None` pattern and no guard. Such arms can be "collapsed"
+/// into a single wild arm without any significant loss in semantics or readability.
fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
if arm.guard.is_some() {
return false;