allow or patterns for a high enough MSRV in collapsible_match

This commit is contained in:
y21 2024-05-02 04:45:45 +02:00
parent 852a64f875
commit 272413f458
5 changed files with 88 additions and 49 deletions

View File

@ -1,3 +1,4 @@
use clippy_config::msrvs::Msrv;
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::IfLetOrMatch; use clippy_utils::higher::IfLetOrMatch;
use clippy_utils::source::snippet; use clippy_utils::source::snippet;
@ -11,12 +12,12 @@
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_span::Span; 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)) { if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
for arm in arms { 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<'_>, pat: &'tcx Pat<'_>,
body: &'tcx Expr<'_>, body: &'tcx Expr<'_>,
else_expr: Option<&'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>( fn check_arm<'tcx>(
@ -37,6 +39,7 @@ fn check_arm<'tcx>(
outer_then_body: &'tcx Expr<'tcx>, outer_then_body: &'tcx Expr<'tcx>,
outer_guard: Option<&'tcx Expr<'tcx>>, outer_guard: Option<&'tcx Expr<'tcx>>,
outer_else_body: Option<&'tcx Expr<'tcx>>, outer_else_body: Option<&'tcx Expr<'tcx>>,
msrv: &Msrv,
) { ) {
let inner_expr = peel_blocks_with_stmt(outer_then_body); let inner_expr = peel_blocks_with_stmt(outer_then_body);
if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr) 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 expression must be a local binding
// match <local> { .. } // match <local> { .. }
&& let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_scrutinee)) && 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 // the binding must come from the pattern of the containing match arm
// ..<local>.. => match <local> { .. } // ..<local>.. => match <local> { .. }
&& let (Some(binding_span), is_innermost_parent_pat_struct) && 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) (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
}

View File

@ -27,7 +27,7 @@
use clippy_config::msrvs::{self, Msrv}; use clippy_config::msrvs::{self, Msrv};
use clippy_utils::source::walk_span_to_context; 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 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_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
use rustc_session::impl_lint_pass; use rustc_session::impl_lint_pass;
@ -1040,7 +1040,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
significant_drop_in_scrutinee::check(cx, expr, ex, arms, source); 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 { if !from_expansion {
// These don't depend on a relationship between multiple arms // These don't depend on a relationship between multiple arms
match_wild_err_arm::check(cx, ex, arms); match_wild_err_arm::check(cx, ex, arms);
@ -1066,7 +1066,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
needless_match::check_match(cx, ex, arms, expr); needless_match::check_match(cx, ex, arms, expr);
match_on_vec_items::check(cx, ex); match_on_vec_items::check(cx, ex);
match_str_case_mismatch::check(cx, ex, arms); 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) { if !in_constant(cx, expr.hir_id) {
manual_unwrap_or::check(cx, expr, ex, arms); manual_unwrap_or::check(cx, expr, ex, arms);
@ -1083,7 +1083,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr); match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
} }
} else if let Some(if_let) = higher::IfLet::hir(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); collapsible_match::check_if_let(cx, if_let.let_pat, if_let.if_then, if_let.if_else, &self.msrv);
if !from_expansion { if !from_expansion {
if let Some(else_expr) = if_let.if_else { if let Some(else_expr) = if_let.if_else {
if self.msrv.meets(msrvs::MATCHES_MACRO) { if self.msrv.meets(msrvs::MATCHES_MACRO) {
@ -1195,3 +1195,18 @@ fn contains_cfg_arm(cx: &LateContext<'_>, e: &Expr<'_>, scrutinee: &Expr<'_>, ar
Err(()) => true, 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
}

View File

@ -1,3 +1,4 @@
use clippy_config::msrvs::Msrv;
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet; use clippy_utils::source::snippet;
use clippy_utils::visitors::{for_each_expr, is_local_used}; use clippy_utils::visitors::{for_each_expr, is_local_used};
@ -12,9 +13,9 @@
use std::borrow::Cow; use std::borrow::Cow;
use std::ops::ControlFlow; 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 { for outer_arm in arms {
let Some(guard) = outer_arm.guard else { let Some(guard) = outer_arm.guard else {
continue; continue;
@ -35,6 +36,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
MatchSource::Normal, MatchSource::Normal,
) = guard.kind ) = guard.kind
&& let Some(binding) = get_pat_binding(cx, scrutinee, outer_arm) && 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) { let pat_span = match (arm.pat.kind, binding.byref_ident) {
(PatKind::Ref(pat, _), Some(_)) => pat.span, (PatKind::Ref(pat, _), Some(_)) => pat.span,
@ -53,6 +55,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
// `Some(x) if let Some(2) = x` // `Some(x) if let Some(2) = x`
else if let ExprKind::Let(let_expr) = guard.kind else if let ExprKind::Let(let_expr) = guard.kind
&& let Some(binding) = get_pat_binding(cx, let_expr.init, outer_arm) && 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) { let pat_span = match (let_expr.pat.kind, binding.byref_ident) {
(PatKind::Ref(pat, _), Some(_)) => pat.span, (PatKind::Ref(pat, _), Some(_)) => pat.span,

View File

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

View File

@ -1,5 +1,5 @@
error: this `match` can be collapsed into the outer `match` error: this `match` can be collapsed into the outer `match`
--> tests/ui/collapsible_match.rs:13:20 --> tests/ui/collapsible_match.rs:14:20
| |
LL | Ok(val) => match val { LL | Ok(val) => match val {
| ____________________^ | ____________________^
@ -10,7 +10,7 @@ LL | | },
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:13:12 --> tests/ui/collapsible_match.rs:14:12
| |
LL | Ok(val) => match val { LL | Ok(val) => match val {
| ^^^ replace this binding | ^^^ replace this binding
@ -21,7 +21,7 @@ LL | Some(n) => foo(n),
= help: to override `-D warnings` add `#[allow(clippy::collapsible_match)]` = help: to override `-D warnings` add `#[allow(clippy::collapsible_match)]`
error: this `match` can be collapsed into the outer `match` error: this `match` can be collapsed into the outer `match`
--> tests/ui/collapsible_match.rs:23:20 --> tests/ui/collapsible_match.rs:24:20
| |
LL | Ok(val) => match val { LL | Ok(val) => match val {
| ____________________^ | ____________________^
@ -32,7 +32,7 @@ LL | | },
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:23:12 --> tests/ui/collapsible_match.rs:24:12
| |
LL | Ok(val) => match val { LL | Ok(val) => match val {
| ^^^ replace this binding | ^^^ replace this binding
@ -41,7 +41,7 @@ LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `if let` can be collapsed into the outer `if let` error: this `if let` can be collapsed into the outer `if let`
--> tests/ui/collapsible_match.rs:33:9 --> tests/ui/collapsible_match.rs:34:9
| |
LL | / if let Some(n) = val { LL | / if let Some(n) = val {
LL | | LL | |
@ -50,7 +50,7 @@ LL | | }
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:32:15 --> tests/ui/collapsible_match.rs:33:15
| |
LL | if let Ok(val) = res_opt { LL | if let Ok(val) = res_opt {
| ^^^ replace this binding | ^^^ replace this binding
@ -58,7 +58,7 @@ LL | if let Some(n) = val {
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `if let` can be collapsed into the outer `if let` error: this `if let` can be collapsed into the outer `if let`
--> tests/ui/collapsible_match.rs:41:9 --> tests/ui/collapsible_match.rs:42:9
| |
LL | / if let Some(n) = val { LL | / if let Some(n) = val {
LL | | LL | |
@ -69,7 +69,7 @@ LL | | }
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:40:15 --> tests/ui/collapsible_match.rs:41:15
| |
LL | if let Ok(val) = res_opt { LL | if let Ok(val) = res_opt {
| ^^^ replace this binding | ^^^ replace this binding
@ -77,7 +77,7 @@ LL | if let Some(n) = val {
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `match` can be collapsed into the outer `if let` error: this `match` can be collapsed into the outer `if let`
--> tests/ui/collapsible_match.rs:53:9 --> tests/ui/collapsible_match.rs:54:9
| |
LL | / match val { LL | / match val {
LL | | LL | |
@ -87,7 +87,7 @@ LL | | }
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:52:15 --> tests/ui/collapsible_match.rs:53:15
| |
LL | if let Ok(val) = res_opt { LL | if let Ok(val) = res_opt {
| ^^^ replace this binding | ^^^ replace this binding
@ -96,7 +96,7 @@ LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `if let` can be collapsed into the outer `match` error: this `if let` can be collapsed into the outer `match`
--> tests/ui/collapsible_match.rs:63:13 --> tests/ui/collapsible_match.rs:64:13
| |
LL | / if let Some(n) = val { LL | / if let Some(n) = val {
LL | | LL | |
@ -105,7 +105,7 @@ LL | | }
| |_____________^ | |_____________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:62:12 --> tests/ui/collapsible_match.rs:63:12
| |
LL | Ok(val) => { LL | Ok(val) => {
| ^^^ replace this binding | ^^^ replace this binding
@ -113,7 +113,7 @@ LL | if let Some(n) = val {
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `match` can be collapsed into the outer `if let` error: this `match` can be collapsed into the outer `if let`
--> tests/ui/collapsible_match.rs:73:9 --> tests/ui/collapsible_match.rs:74:9
| |
LL | / match val { LL | / match val {
LL | | LL | |
@ -123,7 +123,7 @@ LL | | }
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:72:15 --> tests/ui/collapsible_match.rs:73:15
| |
LL | if let Ok(val) = res_opt { LL | if let Ok(val) = res_opt {
| ^^^ replace this binding | ^^^ replace this binding
@ -132,7 +132,7 @@ LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `if let` can be collapsed into the outer `match` error: this `if let` can be collapsed into the outer `match`
--> tests/ui/collapsible_match.rs:85:13 --> tests/ui/collapsible_match.rs:86:13
| |
LL | / if let Some(n) = val { LL | / if let Some(n) = val {
LL | | LL | |
@ -143,7 +143,7 @@ LL | | }
| |_____________^ | |_____________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:84:12 --> tests/ui/collapsible_match.rs:85:12
| |
LL | Ok(val) => { LL | Ok(val) => {
| ^^^ replace this binding | ^^^ replace this binding
@ -151,7 +151,7 @@ LL | if let Some(n) = val {
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `match` can be collapsed into the outer `match` error: this `match` can be collapsed into the outer `match`
--> tests/ui/collapsible_match.rs:97:20 --> tests/ui/collapsible_match.rs:98:20
| |
LL | Ok(val) => match val { LL | Ok(val) => match val {
| ____________________^ | ____________________^
@ -162,7 +162,7 @@ LL | | },
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:97:12 --> tests/ui/collapsible_match.rs:98:12
| |
LL | Ok(val) => match val { LL | Ok(val) => match val {
| ^^^ replace this binding | ^^^ replace this binding
@ -171,7 +171,7 @@ LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `match` can be collapsed into the outer `match` error: this `match` can be collapsed into the outer `match`
--> tests/ui/collapsible_match.rs:107:22 --> tests/ui/collapsible_match.rs:108:22
| |
LL | Some(val) => match val { LL | Some(val) => match val {
| ______________________^ | ______________________^
@ -182,7 +182,7 @@ LL | | },
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:107:14 --> tests/ui/collapsible_match.rs:108:14
| |
LL | Some(val) => match val { LL | Some(val) => match val {
| ^^^ replace this binding | ^^^ replace this binding
@ -190,8 +190,26 @@ LL |
LL | Some(n) => foo(n), LL | Some(n) => foo(n),
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: this `match` can be collapsed into the outer `match`
--> tests/ui/collapsible_match.rs:252:22
|
LL | Some(val) => match val {
| ______________________^
LL | | E::A(val) | E::B(val) => foo(val),
LL | | _ => return,
LL | | },
| |_________^
|
help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:252:14
|
LL | Some(val) => match val {
| ^^^ replace this binding
LL | E::A(val) | E::B(val) => foo(val),
| ^^^^^^^^^^^^^^^^^^^^^ with this pattern
error: this `if let` can be collapsed into the outer `if let` error: this `if let` can be collapsed into the outer `if let`
--> tests/ui/collapsible_match.rs:273:9 --> tests/ui/collapsible_match.rs:283:9
| |
LL | / if let Some(u) = a { LL | / if let Some(u) = a {
LL | | LL | |
@ -200,7 +218,7 @@ LL | | }
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:272:27 --> tests/ui/collapsible_match.rs:282:27
| |
LL | if let Issue9647::A { a, .. } = x { LL | if let Issue9647::A { a, .. } = x {
| ^ replace this binding | ^ replace this binding
@ -208,7 +226,7 @@ LL | if let Some(u) = a {
| ^^^^^^^ with this pattern, prefixed by a: | ^^^^^^^ with this pattern, prefixed by a:
error: this `if let` can be collapsed into the outer `if let` error: this `if let` can be collapsed into the outer `if let`
--> tests/ui/collapsible_match.rs:282:9 --> tests/ui/collapsible_match.rs:292:9
| |
LL | / if let Some(u) = a { LL | / if let Some(u) = a {
LL | | LL | |
@ -217,12 +235,12 @@ LL | | }
| |_________^ | |_________^
| |
help: the outer pattern can be modified to include the inner pattern help: the outer pattern can be modified to include the inner pattern
--> tests/ui/collapsible_match.rs:281:35 --> tests/ui/collapsible_match.rs:291:35
| |
LL | if let Issue9647::A { a: Some(a), .. } = x { LL | if let Issue9647::A { a: Some(a), .. } = x {
| ^ replace this binding | ^ replace this binding
LL | if let Some(u) = a { LL | if let Some(u) = a {
| ^^^^^^^ with this pattern | ^^^^^^^ with this pattern
error: aborting due to 12 previous errors error: aborting due to 13 previous errors