Merge CollapsibleMatch into Matches lint pass

Jarcho · Jarcho · commit 81e44502ac2e · 2022-06-03T12:14:24.000-04:00
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
@@ -38,7 +38,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(casts::UNNECESSARY_CAST),
     LintId::of(collapsible_if::COLLAPSIBLE_ELSE_IF),
     LintId::of(collapsible_if::COLLAPSIBLE_IF),
-    LintId::of(collapsible_match::COLLAPSIBLE_MATCH),
     LintId::of(comparison_chain::COMPARISON_CHAIN),
     LintId::of(copies::IFS_SAME_COND),
     LintId::of(copies::IF_SAME_THEN_ELSE),
@@ -143,6 +142,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN),
     LintId::of(match_result_ok::MATCH_RESULT_OK),
     LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH),
+    LintId::of(matches::COLLAPSIBLE_MATCH),
     LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
     LintId::of(matches::MATCH_AS_REF),
     LintId::of(matches::MATCH_LIKE_MATCHES_MACRO),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
@@ -93,7 +93,6 @@ store.register_lints(&[
     cognitive_complexity::COGNITIVE_COMPLEXITY,
     collapsible_if::COLLAPSIBLE_ELSE_IF,
     collapsible_if::COLLAPSIBLE_IF,
-    collapsible_match::COLLAPSIBLE_MATCH,
     comparison_chain::COMPARISON_CHAIN,
     copies::BRANCHES_SHARING_CODE,
     copies::IFS_SAME_COND,
@@ -263,6 +262,7 @@ store.register_lints(&[
     match_on_vec_items::MATCH_ON_VEC_ITEMS,
     match_result_ok::MATCH_RESULT_OK,
     match_str_case_mismatch::MATCH_STR_CASE_MISMATCH,
+    matches::COLLAPSIBLE_MATCH,
     matches::INFALLIBLE_DESTRUCTURING_MATCH,
     matches::MATCH_AS_REF,
     matches::MATCH_BOOL,
diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs
@@ -12,7 +12,6 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
     LintId::of(collapsible_if::COLLAPSIBLE_ELSE_IF),
     LintId::of(collapsible_if::COLLAPSIBLE_IF),
-    LintId::of(collapsible_match::COLLAPSIBLE_MATCH),
     LintId::of(comparison_chain::COMPARISON_CHAIN),
     LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
     LintId::of(dereference::NEEDLESS_BORROW),
@@ -50,6 +49,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
     LintId::of(map_clone::MAP_CLONE),
     LintId::of(match_result_ok::MATCH_RESULT_OK),
+    LintId::of(matches::COLLAPSIBLE_MATCH),
     LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
     LintId::of(matches::MATCH_LIKE_MATCHES_MACRO),
     LintId::of(matches::MATCH_OVERLAPPING_ARM),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -192,7 +192,6 @@ mod casts;
 mod checked_conversions;
 mod cognitive_complexity;
 mod collapsible_if;
-mod collapsible_match;
 mod comparison_chain;
 mod copies;
 mod copy_iterator;
@@ -568,7 +567,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(len_zero::LenZero));
     store.register_late_pass(|| Box::new(attrs::Attributes));
     store.register_late_pass(|| Box::new(blocks_in_if_conditions::BlocksInIfConditions));
-    store.register_late_pass(|| Box::new(collapsible_match::CollapsibleMatch));
     store.register_late_pass(|| Box::new(unicode::Unicode));
     store.register_late_pass(|| Box::new(uninit_vec::UninitVec));
     store.register_late_pass(|| Box::new(unit_hash::UnitHash));
diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs
@@ -6,68 +6,28 @@ use if_chain::if_chain;
 use rustc_errors::MultiSpan;
 use rustc_hir::LangItem::OptionNone;
 use rustc_hir::{Arm, Expr, Guard, HirId, Let, Pat, PatKind};
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_lint::LateContext;
 use rustc_span::Span;
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// Finds nested `match` or `if let` expressions where the patterns may be "collapsed" together
-    /// without adding any branches.
-    ///
-    /// Note that this lint is not intended to find _all_ cases where nested match patterns can be merged, but only
-    /// cases where merging would most likely make the code more readable.
-    ///
-    /// ### Why is this bad?
-    /// It is unnecessarily verbose and complex.
-    ///
-    /// ### Example
-    /// ```rust
-    /// fn func(opt: Option<Result<u64, String>>) {
-    ///     let n = match opt {
-    ///         Some(n) => match n {
-    ///             Ok(n) => n,
-    ///             _ => return,
-    ///         }
-    ///         None => return,
-    ///     };
-    /// }
-    /// ```
-    /// Use instead:
-    /// ```rust
-    /// fn func(opt: Option<Result<u64, String>>) {
-    ///     let n = match opt {
-    ///         Some(Ok(n)) => n,
-    ///         _ => return,
-    ///     };
-    /// }
-    /// ```
-    #[clippy::version = "1.50.0"]
-    pub COLLAPSIBLE_MATCH,
-    style,
-    "Nested `match` or `if let` expressions where the patterns may be \"collapsed\" together."
-}
-
-declare_lint_pass!(CollapsibleMatch => [COLLAPSIBLE_MATCH]);
+use super::COLLAPSIBLE_MATCH;
 
-impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
-        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 => {},
+pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
+    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));
         }
     }
 }
 
+pub(super) fn check_if_let<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &'tcx Pat<'_>,
+    body: &'tcx Expr<'_>,
+    else_expr: Option<&'tcx Expr<'_>>,
+) {
+    check_arm(cx, false, pat, body, None, else_expr);
+}
+
 fn check_arm<'tcx>(
     cx: &LateContext<'tcx>,
     outer_is_match: bool,
diff --git a/clippy_lints/src/matches/match_like_matches.rs b/clippy_lints/src/matches/match_like_matches.rs
@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_wild;
 use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::{higher, is_wild};
 use rustc_ast::{Attribute, LitKind};
 use rustc_errors::Applicability;
 use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat};
@@ -11,22 +11,24 @@ use rustc_span::source_map::Spanned;
 use super::MATCH_LIKE_MATCHES_MACRO;
 
 /// Lint a `match` or `if let .. { .. } else { .. }` expr that could be replaced by `matches!`
-pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-    if let Some(higher::IfLet {
-        let_pat,
+pub(crate) fn check_if_let<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    let_pat: &'tcx Pat<'_>,
+    let_expr: &'tcx Expr<'_>,
+    then_expr: &'tcx Expr<'_>,
+    else_expr: &'tcx Expr<'_>,
+) {
+    find_matches_sugg(
+        cx,
         let_expr,
-        if_then,
-        if_else: Some(if_else),
-    }) = higher::IfLet::hir(cx, expr)
-    {
-        find_matches_sugg(
-            cx,
-            let_expr,
-            IntoIterator::into_iter([(&[][..], Some(let_pat), if_then, None), (&[][..], None, if_else, None)]),
-            expr,
-            true,
-        );
-    }
+        IntoIterator::into_iter([
+            (&[][..], Some(let_pat), then_expr, None),
+            (&[][..], None, else_expr, None),
+        ]),
+        expr,
+        true,
+    );
 }
 
 pub(super) fn check_match<'tcx>(
diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs
@@ -1,12 +1,14 @@
 use clippy_utils::source::{snippet_opt, span_starts_with, walk_span_to_context};
-use clippy_utils::{meets_msrv, msrvs};
+use clippy_utils::{higher, meets_msrv, msrvs};
 use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat};
 use rustc_lexer::{tokenize, TokenKind};
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
 use rustc_semver::RustcVersion;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{Span, SpanData, SyntaxContext};
 
+mod collapsible_match;
 mod infallible_destructuring_match;
 mod match_as_ref;
 mod match_bool;
@@ -610,6 +612,44 @@ declare_clippy_lint! {
     "`match` or match-like `if let` that are unnecessary"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Finds nested `match` or `if let` expressions where the patterns may be "collapsed" together
+    /// without adding any branches.
+    ///
+    /// Note that this lint is not intended to find _all_ cases where nested match patterns can be merged, but only
+    /// cases where merging would most likely make the code more readable.
+    ///
+    /// ### Why is this bad?
+    /// It is unnecessarily verbose and complex.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn func(opt: Option<Result<u64, String>>) {
+    ///     let n = match opt {
+    ///         Some(n) => match n {
+    ///             Ok(n) => n,
+    ///             _ => return,
+    ///         }
+    ///         None => return,
+    ///     };
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn func(opt: Option<Result<u64, String>>) {
+    ///     let n = match opt {
+    ///         Some(Ok(n)) => n,
+    ///         _ => return,
+    ///     };
+    /// }
+    /// ```
+    #[clippy::version = "1.50.0"]
+    pub COLLAPSIBLE_MATCH,
+    style,
+    "Nested `match` or `if let` expressions where the patterns may be \"collapsed\" together."
+}
+
 #[derive(Default)]
 pub struct Matches {
     msrv: Option<RustcVersion>,
@@ -644,19 +684,29 @@ impl_lint_pass!(Matches => [
     MATCH_LIKE_MATCHES_MACRO,
     MATCH_SAME_ARMS,
     NEEDLESS_MATCH,
+    COLLAPSIBLE_MATCH,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Matches {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if expr.span.from_expansion() {
+        if in_external_macro(cx.sess(), expr.span) {
             return;
         }
+        let from_expansion = expr.span.from_expansion();
 
         if let ExprKind::Match(ex, arms, source) = expr.kind {
             if !span_starts_with(cx, expr.span, "match") {
                 return;
             }
-            if !contains_cfg_arm(cx, expr, ex, arms) {
+
+            collapsible_match::check_match(cx, arms);
+            if !from_expansion {
+                // These don't depend on a relationship between multiple arms
+                match_wild_err_arm::check(cx, ex, arms);
+                wild_in_or_pats::check(cx, arms);
+            }
+
+            if !from_expansion && !contains_cfg_arm(cx, expr, ex, arms) {
                 if source == MatchSource::Normal {
                     if !(meets_msrv(self.msrv, msrvs::MATCHES_MACRO)
                         && match_like_matches::check_match(cx, expr, ex, arms))
@@ -680,16 +730,32 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                 }
                 match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
             }
-
-            // These don't depend on a relationship between multiple arms
-            match_wild_err_arm::check(cx, ex, arms);
-            wild_in_or_pats::check(cx, arms);
-        } else {
-            if meets_msrv(self.msrv, msrvs::MATCHES_MACRO) {
-                match_like_matches::check(cx, expr);
+        } else if let Some(if_let) = higher::IfLet::hir(cx, expr) {
+            collapsible_match::check_if_let(cx, if_let.let_pat, if_let.if_then, if_let.if_else);
+            if !from_expansion {
+                if let Some(else_expr) = if_let.if_else {
+                    if meets_msrv(self.msrv, msrvs::MATCHES_MACRO) {
+                        match_like_matches::check_if_let(
+                            cx,
+                            expr,
+                            if_let.let_pat,
+                            if_let.let_expr,
+                            if_let.if_then,
+                            else_expr,
+                        );
+                    }
+                }
+                redundant_pattern_match::check_if_let(
+                    cx,
+                    expr,
+                    if_let.let_pat,
+                    if_let.let_expr,
+                    if_let.if_else.is_some(),
+                );
+                needless_match::check_if_let(cx, expr, &if_let);
             }
+        } else if !from_expansion {
             redundant_pattern_match::check(cx, expr);
-            needless_match::check(cx, expr);
         }
     }
 
diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs
@@ -47,20 +47,18 @@ pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>],
 ///     some_enum
 /// }
 /// ```
-pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
-    if let Some(ref if_let) = higher::IfLet::hir(cx, ex) {
-        if !is_else_clause(cx.tcx, ex) && expr_ty_matches_p_ty(cx, if_let.let_expr, ex) && check_if_let(cx, if_let) {
-            let mut applicability = Applicability::MachineApplicable;
-            span_lint_and_sugg(
-                cx,
-                NEEDLESS_MATCH,
-                ex.span,
-                "this if-let expression is unnecessary",
-                "replace it with",
-                snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(),
-                applicability,
-            );
-        }
+pub(crate) fn check_if_let<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'_>, if_let: &higher::IfLet<'tcx>) {
+    if !is_else_clause(cx.tcx, ex) && expr_ty_matches_p_ty(cx, if_let.let_expr, ex) && check_if_let_inner(cx, if_let) {
+        let mut applicability = Applicability::MachineApplicable;
+        span_lint_and_sugg(
+            cx,
+            NEEDLESS_MATCH,
+            ex.span,
+            "this if-let expression is unnecessary",
+            "replace it with",
+            snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(),
+            applicability,
+        );
     }
 }
 
@@ -77,15 +75,15 @@ fn check_all_arms(cx: &LateContext<'_>, match_expr: &Expr<'_>, arms: &[Arm<'_>])
     true
 }
 
-fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
+fn check_if_let_inner(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
     if let Some(if_else) = if_let.if_else {
         if !pat_same_as_expr(if_let.let_pat, peel_blocks_with_stmt(if_let.if_then)) {
             return false;
         }
 
         // Recursively check for each `else if let` phrase,
         if let Some(ref nested_if_let) = higher::IfLet::hir(cx, if_else) {
-            return check_if_let(cx, nested_if_let);
+            return check_if_let_inner(cx, nested_if_let);
         }
 
         if matches!(if_else.kind, ExprKind::Block(..)) {
diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs
