Skip to content

Handle non-adjacent boolean arms in match_like_matches_macro #17624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fzlzjerry wants to merge 2 commits into from
Closed

Handle non-adjacent boolean arms in match_like_matches_macro #17624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
034e59c
Extract pattern overlap analysis into shared utility
fzlzjerry Aug 26, 2026
dc93a16
Handle non-adjacent boolean match arms
fzlzjerry Aug 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
223 changes: 108 additions & 115 deletions clippy_lints/src/matches/match_like_matches.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,133 +69,126 @@ pub(super) fn check_match<'tcx>(
scrutinee: &'tcx Expr<'_>,
arms: &'tcx [Arm<'tcx>],
) -> bool {
if let Some((last_arm, arms_without_last)) = arms.split_last()
&& let Some((first_arm, middle_arms)) = arms_without_last.split_first()
&& !span_contains_comment(cx, e.span)
&& cx.typeck_results().expr_ty(e).is_bool()
&& let Some(b0) = find_bool_lit(first_arm.body)
&& let Some(b1) = find_bool_lit(last_arm.body)
&& b0 != b1
// We handle two cases:
&& (
// - There are no middle arms, i.e., 2 arms in total
//
// In that case, the first arm may or may not have a guard, because this:
// ```rs
// match e {
// Either::Left $(if $guard)|+ => true, // or `false`, but then we'll need `!matches!(..)`
// _ => false,
// }
// ```
// can always become this:
// ```rs
// matches!(e, Either::Left $(if $guard)|+)
// ```
//
// But if the guard _is_ present, it may not be an `if-let` guard, as `matches!` doesn't
// support these (currently?)
(middle_arms.is_empty() && first_arm.guard.is_none_or(|g| !has_let_expr(g)))

// - (added in #6216) There are middle arms
//
// In that case, neither they nor the first arm may have guards
// -- otherwise, they couldn't be combined into an or-pattern in `matches!`
//
// This:
// ```rs
// match e {
// Either3::First => true,
// Either3::Second => true,
// _ /* matches `Either3::Third` */ => false,
// }
// ```
// can become this:
// ```rs
// matches!(e, Either3::First | Either3::Second)
// ```
//
// But this:
// ```rs
// match e {
// Either3::First if X => true,
// Either3::Second => true,
// _ => false,
// }
// ```
// cannot be transformed.
//
// We set an additional constraint of all of them needing to return the same bool,
// so we don't lint things like:
// ```rs
// match e {
// Either3::First => true,
// Either3::Second => false,
// _ => false,
// }
// ```
// This is not *strictly* necessary, but it simplifies the logic a bit
|| arms_without_last.iter().all(|arm| {
cx.tcx.hir_attrs(arm.hir_id).is_empty() && arm.guard.is_none() && find_bool_lit(arm.body) == Some(b0)
})
)
{
if !is_wild(last_arm.pat) {
if arms.len() < 2 || span_contains_comment(cx, e.span) || !cx.typeck_results().expr_ty(e).is_bool() {
return false;
}

let Some(arm_values) = arms
.iter()
.map(|arm| find_bool_lit(arm.body))
.collect::<Option<Vec<_>>>()
else {
return false;
};

let (selected_value, selected_arms, guard) = if let [first_arm, last_arm] = arms {
Comment thread
fzlzjerry marked this conversation as resolved.
// A two-arm match may preserve a non-let guard on its first arm.
if arm_values[0] == arm_values[1] || !is_wild(last_arm.pat) || first_arm.guard.is_some_and(has_let_expr) {
return false;
}

(arm_values[0], vec![first_arm], first_arm.guard)
} else {
// Longer matches combine guard-free arms with the selected result into an or-pattern.
// Guards can't be combined into an or-pattern. Attributes may also remove an arm before
// linting, which could change which result should be represented by the pattern.
if arms
.iter()
.any(|arm| !cx.tcx.hir_attrs(arm.hir_id).is_empty() || arm.guard.is_some())
{
return false;
}

for arm in arms_without_last {
let pat = arm.pat;
if !is_lint_allowed(cx, REDUNDANT_PATTERN_MATCHING, pat.hir_id) && is_some_wild(pat.kind) {
// A wildcard can't be part of the suggested pattern, so represent the opposite result. If
// there isn't one, represent the `true` arms directly.
let selected_value = arms
.iter()
.position(|arm| is_wild(arm.pat))
.is_none_or(|index| !arm_values[index]);

if !arm_values.contains(&selected_value) || !arm_values.contains(&!selected_value) {
return false;
}

let selected_arms: Vec<_> = arms
.iter()
.zip(&arm_values)
.filter(|&(_, &value)| value == selected_value)
.map(|(arm, _)| arm)
.collect();

// An earlier arm returning the opposite value takes precedence over a later selected arm.
// Moving that selected pattern into `matches!` is only sound when the two cannot overlap.
for (index, (arm, &value)) in arms.iter().zip(&arm_values).enumerate() {
if value == selected_value
&& arms[..index]
.iter()
.zip(&arm_values[..index])
.any(|(earlier_arm, &earlier_value)| {
earlier_value != selected_value
&& super::pat_overlap::patterns_overlap(cx, earlier_arm.pat, arm.pat)
})
{
return false;
}
}

// The suggestion may be incorrect, because some arms can have `cfg` attributes
// evaluated into `false` and so such arms will be stripped before.
let mut applicability = Applicability::MaybeIncorrect;
let pat = {
use itertools::Itertools as _;
arms_without_last
.iter()
.map(|arm| snippet_with_applicability(cx, arm.pat.span, "..", &mut applicability))
.join(" | ")
};
let pat_and_guard = if let Some(g) = first_arm.guard {
format!(
"{pat} if {}",
snippet_with_applicability(cx, g.span, "..", &mut applicability)
)
} else {
pat
};
(selected_value, selected_arms, None)
};

// strip potential borrows (#6503), but only if the type is a reference
let mut ex_new = scrutinee;
if let ExprKind::AddrOf(BorrowKind::Ref, .., ex_inner) = scrutinee.kind
&& let ty::Ref(..) = cx.typeck_results().expr_ty(ex_inner).kind()
{
ex_new = ex_inner;
for arm in &selected_arms {
let pat = arm.pat;
if !is_lint_allowed(cx, REDUNDANT_PATTERN_MATCHING, pat.hir_id) && is_some_wild(pat.kind) {
return false;
}
}
Comment thread
fzlzjerry marked this conversation as resolved.

let (snippet, _) = snippet_with_context(cx, ex_new.span, e.span.ctxt(), "..", &mut applicability);
span_lint_and_then(
cx,
MATCH_LIKE_MATCHES_MACRO,
e.span,
"match expression looks like `matches!` macro",
|diag| {
diag.span_suggestion_verbose(
e.span,
"use `matches!` directly",
format!("{}matches!({snippet}, {pat_and_guard})", if b0 { "" } else { "!" }),
applicability,
);
},
);
true
// The suggestion may be incorrect, because some arms can have `cfg` attributes evaluated into
// `false` and so such arms will be stripped before.
let mut applicability = Applicability::MaybeIncorrect;
let pat = {
use itertools::Itertools as _;
selected_arms
.iter()
.map(|arm| snippet_with_applicability(cx, arm.pat.span, "..", &mut applicability))
.join(" | ")
};
let pat_and_guard = if let Some(g) = guard {
format!(
"{pat} if {}",
snippet_with_applicability(cx, g.span, "..", &mut applicability)
)
} else {
false
pat
};

// strip potential borrows (#6503), but only if the type is a reference
let mut ex_new = scrutinee;
if let ExprKind::AddrOf(BorrowKind::Ref, .., ex_inner) = scrutinee.kind
&& let ty::Ref(..) = cx.typeck_results().expr_ty(ex_inner).kind()
{
ex_new = ex_inner;
}

let (snippet, _) = snippet_with_context(cx, ex_new.span, e.span.ctxt(), "..", &mut applicability);
span_lint_and_then(
cx,
MATCH_LIKE_MATCHES_MACRO,
e.span,
"match expression looks like `matches!` macro",
|diag| {
diag.span_suggestion_verbose(
e.span,
"use `matches!` directly",
format!(
"{}matches!({snippet}, {pat_and_guard})",
if selected_value { "" } else { "!" }
),
applicability,
);
},
);
true
}

/// Extract a `bool` or `{ bool }`
Expand Down
Loading