Skip to content

refactor(match_like_matches_macro): disentangle the if-let and match cases #15854

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

refactor(match_like_matches_macro): disentangle the if-let and match cases #15854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6d0fafd
clean-up
ada4a Oct 9, 2025
62e1225
inline `find_matches_sugg` into `check_if_let`
ada4a Oct 9, 2025
27d5f5c
inline `is_if_let` into all callers
ada4a Oct 9, 2025
8519d49
inline `arms_without_last` into all callers
ada4a Oct 9, 2025
d1d5f0c
inline `arms` into all callers
ada4a Oct 9, 2025
83e2b3d
inline `find_matches_sugg` into `check_match`
ada4a Oct 9, 2025
bf5170a
match arm pats aren't `Option`al anymore
ada4a Oct 9, 2025
0efe3cf
split `arms` into first, last, and middle pats as slice
ada4a Oct 9, 2025
dd7f605
inline `{first,last}_{attrs,expr,pat,guard}` into all callers
ada4a Oct 9, 2025
0535908
final refactor and docs
ada4a Oct 9, 2025
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
196 changes: 122 additions & 74 deletions clippy_lints/src/matches/match_like_matches.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
//! Lint a `match` or `if let .. { .. } else { .. }` expr that could be replaced by `matches!`

use super::REDUNDANT_PATTERN_MATCHING;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{is_lint_allowed, is_wild, span_contains_comment};
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Arm, Attribute, BorrowKind, Expr, ExprKind, Pat, PatKind, QPath};
use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Pat, PatKind, QPath};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty;
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_if_let<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
Expand All @@ -20,16 +21,42 @@ pub(crate) fn check_if_let<'tcx>(
then_expr: &'tcx Expr<'_>,
else_expr: &'tcx Expr<'_>,
) {
find_matches_sugg(
cx,
let_expr,
IntoIterator::into_iter([
(&[][..], Some(let_pat), then_expr, None),
(&[][..], None, else_expr, None),
]),
expr,
true,
);
if !span_contains_comment(cx.sess().source_map(), expr.span)
&& cx.typeck_results().expr_ty(expr).is_bool()
&& let Some(b0) = find_bool_lit(then_expr)
&& let Some(b1) = find_bool_lit(else_expr)
&& b0 != b1
{
if !is_lint_allowed(cx, REDUNDANT_PATTERN_MATCHING, let_pat.hir_id) && is_some_wild(let_pat.kind) {
return;
}

// 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 = snippet_with_applicability(cx, let_pat.span, "..", &mut applicability);

// strip potential borrows (#6503), but only if the type is a reference
let mut ex_new = let_expr;
if let ExprKind::AddrOf(BorrowKind::Ref, .., ex_inner) = let_expr.kind
&& let ty::Ref(..) = cx.typeck_results().expr_ty(ex_inner).kind()
{
ex_new = ex_inner;
}
span_lint_and_sugg(
cx,
MATCH_LIKE_MATCHES_MACRO,
expr.span,
"if let .. else expression looks like `matches!` macro",
"try",
format!(
"{}matches!({}, {pat})",
if b0 { "" } else { "!" },
snippet_with_applicability(cx, ex_new.span, "..", &mut applicability),
),
applicability,
);
}
}

pub(super) fn check_match<'tcx>(
Expand All @@ -38,55 +65,80 @@ pub(super) fn check_match<'tcx>(
scrutinee: &'tcx Expr<'_>,
arms: &'tcx [Arm<'tcx>],
) -> bool {
find_matches_sugg(
cx,
scrutinee,
arms.iter()
.map(|arm| (cx.tcx.hir_attrs(arm.hir_id), Some(arm.pat), arm.body, arm.guard)),
e,
false,
)
}

/// Lint a `match` or `if let` for replacement by `matches!`
fn find_matches_sugg<'a, 'b, I>(
cx: &LateContext<'_>,
ex: &Expr<'_>,
mut iter: I,
expr: &Expr<'_>,
is_if_let: bool,
) -> bool
where
'b: 'a,
I: Clone
+ DoubleEndedIterator
+ ExactSizeIterator
+ Iterator<Item = (&'a [Attribute], Option<&'a Pat<'b>>, &'a Expr<'b>, Option<&'a Expr<'b>>)>,
{
if !span_contains_comment(cx.sess().source_map(), expr.span)
&& iter.len() >= 2
&& cx.typeck_results().expr_ty(expr).is_bool()
&& let Some((_, last_pat_opt, last_expr, _)) = iter.next_back()
&& let iter_without_last = iter.clone()
&& let Some((first_attrs, _, first_expr, first_guard)) = iter.next()
&& let Some(b0) = find_bool_lit(&first_expr.kind)
&& let Some(b1) = find_bool_lit(&last_expr.kind)
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.sess().source_map(), 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
&& (first_guard.is_none() || iter.len() == 0)
&& first_attrs.is_empty()
&& iter.all(|arm| find_bool_lit(&arm.2.kind).is_some_and(|b| b == b0) && arm.3.is_none() && arm.0.is_empty())
// 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)|+)
// ```
middle_arms.is_empty()

// - (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 let Some(last_pat) = last_pat_opt
&& !is_wild(last_pat)
{
if !is_wild(last_arm.pat) {
return false;
}

for arm in iter_without_last.clone() {
if let Some(pat) = arm.1
&& !is_lint_allowed(cx, REDUNDANT_PATTERN_MATCHING, pat.hir_id)
&& is_some(pat.kind)
{
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) {
return false;
}
}
Expand All @@ -96,14 +148,12 @@ where
let mut applicability = Applicability::MaybeIncorrect;
let pat = {
use itertools::Itertools as _;
iter_without_last
.filter_map(|arm| {
let pat_span = arm.1?.span;
Some(snippet_with_applicability(cx, pat_span, "..", &mut applicability))
})
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_guard {
let pat_and_guard = if let Some(g) = first_arm.guard {
format!(
"{pat} if {}",
snippet_with_applicability(cx, g.span, "..", &mut applicability)
Expand All @@ -113,20 +163,17 @@ where
};

// strip potential borrows (#6503), but only if the type is a reference
let mut ex_new = ex;
if let ExprKind::AddrOf(BorrowKind::Ref, .., ex_inner) = ex.kind
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;
}
span_lint_and_sugg(
cx,
MATCH_LIKE_MATCHES_MACRO,
expr.span,
format!(
"{} expression looks like `matches!` macro",
if is_if_let { "if let .. else" } else { "match" }
),
e.span,
"match expression looks like `matches!` macro",
"try",
format!(
"{}matches!({}, {pat_and_guard})",
Expand All @@ -142,11 +189,11 @@ where
}

/// Extract a `bool` or `{ bool }`
fn find_bool_lit(ex: &ExprKind<'_>) -> Option<bool> {
match ex {
fn find_bool_lit(ex: &Expr<'_>) -> Option<bool> {
match ex.kind {
ExprKind::Lit(Spanned {
node: LitKind::Bool(b), ..
}) => Some(*b),
}) => Some(b),
ExprKind::Block(
rustc_hir::Block {
stmts: [],
Expand All @@ -168,8 +215,9 @@ fn find_bool_lit(ex: &ExprKind<'_>) -> Option<bool> {
}
}

fn is_some(path_kind: PatKind<'_>) -> bool {
match path_kind {
/// Checks whether a pattern is `Some(_)`
fn is_some_wild(pat_kind: PatKind<'_>) -> bool {
match pat_kind {
PatKind::TupleStruct(QPath::Resolved(_, path), [first, ..], _) if is_wild(first) => {
let name = path.segments[0].ident;
name.name == rustc_span::sym::Some
Expand Down
5 changes: 2 additions & 3 deletions tests/ui/match_expr_like_matches_macro.fixed → tests/ui/match_like_matches_macro.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#![warn(clippy::match_like_matches_macro)]
#![allow(
unreachable_patterns,
dead_code,
clippy::equatable_if_let,
clippy::needless_borrowed_reference,
clippy::redundant_guards
Expand All @@ -14,11 +13,11 @@ fn main() {
let _y = matches!(x, Some(0));
//~^^^^ match_like_matches_macro

// Lint
// No lint: covered by `redundant_pattern_matching`
let _w = x.is_some();
//~^^^^ redundant_pattern_matching

// Turn into is_none
// No lint: covered by `redundant_pattern_matching`
let _z = x.is_none();
//~^^^^ redundant_pattern_matching

Expand Down
5 changes: 2 additions & 3 deletions tests/ui/match_expr_like_matches_macro.rs → tests/ui/match_like_matches_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#![warn(clippy::match_like_matches_macro)]
#![allow(
unreachable_patterns,
dead_code,
clippy::equatable_if_let,
clippy::needless_borrowed_reference,
clippy::redundant_guards
Expand All @@ -17,14 +16,14 @@ fn main() {
};
//~^^^^ match_like_matches_macro

// Lint
// No lint: covered by `redundant_pattern_matching`
let _w = match x {
Some(_) => true,
_ => false,
};
//~^^^^ redundant_pattern_matching

// Turn into is_none
// No lint: covered by `redundant_pattern_matching`
let _z = match x {
Some(_) => false,
None => true,
Expand Down
Loading