Auto merge of rust-lang#12745 - y21:collapsible_match_or_pat, r=llogiq

Suggest collapsing nested or patterns if the MSRV allows it

Nested `or` patterns have been stable since 1.53, so we should be able to suggest `Some(1 | 2)` if the MSRV isn't set below that.

This change adds an msrv check and also moves it to `matches/mod.rs`, because it's also needed by `redundant_guards`.

changelog: [`collapsible_match`]: suggest collapsing nested or patterns if the MSRV allows it

Author: bors

clippy_lints/src/matches/collapsible_match.rs

@@ -1,3 +1,4 @@
+use clippy_config::msrvs::Msrv;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::higher::IfLetOrMatch;
 use clippy_utils::source::snippet;
@@ -11,12 +12,12 @@ use rustc_hir::{Arm, Expr, HirId, Pat, PatKind};
 use rustc_lint::LateContext;
 use rustc_span::Span;
 
-use super::COLLAPSIBLE_MATCH;
+use super::{pat_contains_disallowed_or, COLLAPSIBLE_MATCH};
 
-pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
+pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], msrv: &Msrv) {
     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, Some(els_arm.body));
+            check_arm(cx, true, arm.pat, arm.body, arm.guard, Some(els_arm.body), msrv);
         }
     }
 }
@@ -26,8 +27,9 @@ pub(super) fn check_if_let<'tcx>(
     pat: &'tcx Pat<'_>,
     body: &'tcx Expr<'_>,
     else_expr: Option<&'tcx Expr<'_>>,
+    msrv: &Msrv,
 ) {
-    check_arm(cx, false, pat, body, None, else_expr);
+    check_arm(cx, false, pat, body, None, else_expr, msrv);
 }
 
 fn check_arm<'tcx>(
@@ -37,6 +39,7 @@ fn check_arm<'tcx>(
     outer_then_body: &'tcx Expr<'tcx>,
     outer_guard: Option<&'tcx Expr<'tcx>>,
     outer_else_body: Option<&'tcx Expr<'tcx>>,
+    msrv: &Msrv,
 ) {
     let inner_expr = peel_blocks_with_stmt(outer_then_body);
     if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr)
@@ -57,7 +60,7 @@ fn check_arm<'tcx>(
         // match expression must be a local binding
         // match <local> { .. }
         && let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_scrutinee))
-        && !pat_contains_or(inner_then_pat)
+        && !pat_contains_disallowed_or(inner_then_pat, msrv)
         // the binding must come from the pattern of the containing match arm
         // ..<local>.. => match <local> { .. }
         && let (Some(binding_span), is_innermost_parent_pat_struct)
@@ -142,13 +145,3 @@ fn find_pat_binding_and_is_innermost_parent_pat_struct(pat: &Pat<'_>, hir_id: Hi
     });
     (span, is_innermost_parent_pat_struct)
 }
-
-fn pat_contains_or(pat: &Pat<'_>) -> bool {
-    let mut result = false;
-    pat.walk(|p| {
-        let is_or = matches!(p.kind, PatKind::Or(_));
-        result |= is_or;
-        !is_or
-    });
-    result
-}

clippy_lints/src/matches/mod.rs

@@ -27,7 +27,7 @@ mod wild_in_or_pats;
 use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::source::walk_span_to_context;
 use clippy_utils::{higher, in_constant, is_direct_expn_of, is_span_match, span_contains_cfg};
-use rustc_hir::{Arm, Expr, ExprKind, LetStmt, MatchSource, Pat};
+use rustc_hir::{Arm, Expr, ExprKind, LetStmt, MatchSource, Pat, PatKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_session::impl_lint_pass;
@@ -1040,7 +1040,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                 significant_drop_in_scrutinee::check(cx, expr, ex, arms, source);
             }
 
-            collapsible_match::check_match(cx, arms);
+            collapsible_match::check_match(cx, arms, &self.msrv);
             if !from_expansion {
                 // These don't depend on a relationship between multiple arms
                 match_wild_err_arm::check(cx, ex, arms);
@@ -1066,7 +1066,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                     needless_match::check_match(cx, ex, arms, expr);
                     match_on_vec_items::check(cx, ex);
                     match_str_case_mismatch::check(cx, ex, arms);
-                    redundant_guards::check(cx, arms);
+                    redundant_guards::check(cx, arms, &self.msrv);
 
                     if !in_constant(cx, expr.hir_id) {
                         manual_unwrap_or::check(cx, expr, ex, arms);
@@ -1083,7 +1083,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                 match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), 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);
+            collapsible_match::check_if_let(cx, if_let.let_pat, if_let.if_then, if_let.if_else, &self.msrv);
             if !from_expansion {
                 if let Some(else_expr) = if_let.if_else {
                     if self.msrv.meets(msrvs::MATCHES_MACRO) {
@@ -1195,3 +1195,18 @@ fn contains_cfg_arm(cx: &LateContext<'_>, e: &Expr<'_>, scrutinee: &Expr<'_>, ar
         Err(()) => true,
     }
 }
+
+/// Checks if `pat` contains OR patterns that cannot be nested due to a too low MSRV.
+fn pat_contains_disallowed_or(pat: &Pat<'_>, msrv: &Msrv) -> bool {
+    if msrv.meets(msrvs::OR_PATTERNS) {
+        return false;
+    }
+
+    let mut result = false;
+    pat.walk(|p| {
+        let is_or = matches!(p.kind, PatKind::Or(_));
+        result |= is_or;
+        !is_or
+    });
+    result
+}

clippy_lints/src/matches/redundant_guards.rs

@@ -1,40 +1,32 @@
+use clippy_config::msrvs::Msrv;
 use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::macros::matching_root_macro_call;
 use clippy_utils::source::snippet;
 use clippy_utils::visitors::{for_each_expr, is_local_used};
 use clippy_utils::{in_constant, path_to_local};
 use rustc_ast::{BorrowKind, LitKind};
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, MatchSource, Node, Pat, PatKind, UnOp};
+use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, MatchSource, Node, PatKind, UnOp};
 use rustc_lint::LateContext;
 use rustc_span::symbol::Ident;
-use rustc_span::{Span, Symbol};
+use rustc_span::{sym, Span, Symbol};
 use std::borrow::Cow;
 use std::ops::ControlFlow;
 
-use super::REDUNDANT_GUARDS;
+use super::{pat_contains_disallowed_or, REDUNDANT_GUARDS};
 
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>], msrv: &Msrv) {
     for outer_arm in arms {
         let Some(guard) = outer_arm.guard else {
             continue;
         };
 
         // `Some(x) if matches!(x, y)`
-        if let ExprKind::Match(
-            scrutinee,
-            [
-                arm,
-                Arm {
-                    pat: Pat {
-                        kind: PatKind::Wild, ..
-                    },
-                    ..
-                },
-            ],
-            MatchSource::Normal,
-        ) = guard.kind
+        if let ExprKind::Match(scrutinee, [arm, _], MatchSource::Normal) = guard.kind
+            && matching_root_macro_call(cx, guard.span, sym::matches_macro).is_some()
             && let Some(binding) = get_pat_binding(cx, scrutinee, outer_arm)
+            && !pat_contains_disallowed_or(arm.pat, msrv)
         {
             let pat_span = match (arm.pat.kind, binding.byref_ident) {
                 (PatKind::Ref(pat, _), Some(_)) => pat.span,
@@ -53,6 +45,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
         // `Some(x) if let Some(2) = x`
         else if let ExprKind::Let(let_expr) = guard.kind
             && let Some(binding) = get_pat_binding(cx, let_expr.init, outer_arm)
+            && !pat_contains_disallowed_or(let_expr.pat, msrv)
         {
             let pat_span = match (let_expr.pat.kind, binding.byref_ident) {
                 (PatKind::Ref(pat, _), Some(_)) => pat.span,

tests/ui/collapsible_match.rs

@@ -4,7 +4,8 @@
     clippy::needless_return,
     clippy::no_effect,
     clippy::single_match,
-    clippy::uninlined_format_args
+    clippy::uninlined_format_args,
+    clippy::let_unit_value
 )]
 
 fn lint_cases(opt_opt: Option<Option<u32>>, res_opt: Result<Option<u32>, String>) {
@@ -238,13 +239,22 @@ fn negative_cases(res_opt: Result<Option<u32>, String>, res_res: Result<Result<u
         },
         _ => return,
     }
-    match make::<Option<E<u32>>>() {
+    #[clippy::msrv = "1.52.0"]
+    let _ = match make::<Option<E<u32>>>() {
         Some(val) => match val {
             E::A(val) | E::B(val) => foo(val),
             _ => return,
         },
         _ => return,
-    }
+    };
+    #[clippy::msrv = "1.53.0"]
+    let _ = match make::<Option<E<u32>>>() {
+        Some(val) => match val {
+            E::A(val) | E::B(val) => foo(val),
+            _ => return,
+        },
+        _ => return,
+    };
     if let Ok(val) = res_opt {
         if let Some(n) = val {
             let _ = || {