]> git.lizzy.rs Git - rust.git/blobdiff - src/tools/clippy/clippy_lints/src/collapsible_match.rs
Auto merge of #81156 - DrMeepster:read_buf, r=joshtriplett
[rust.git] / src / tools / clippy / clippy_lints / src / collapsible_match.rs
index a403a9846babd9d1966fe681da7b1484db789924..626f9971f01e7d678cbc69c22a00d09841109cf7 100644 (file)
@@ -1,6 +1,7 @@
 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};
@@ -40,6 +41,7 @@
     ///     };
     /// }
     /// ```
+    #[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");
                 },
             );
@@ -121,9 +152,8 @@ fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir>
     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;