From e41a6fc042cdd40f415eba4288d5c5e1161266b1 Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
Date: Sun, 6 Feb 2022 13:42:17 -0500
Subject: [PATCH] Split out `match_like_matches_macro`
---
.../src/matches/match_like_matches.rs | 166 +++++++++++++++++
clippy_lints/src/matches/mod.rs | 172 +-----------------
2 files changed, 175 insertions(+), 163 deletions(-)
create mode 100644 clippy_lints/src/matches/match_like_matches.rs
diff --git a/clippy_lints/src/matches/match_like_matches.rs b/clippy_lints/src/matches/match_like_matches.rs
new file mode 100644
index 00000000000..d605b6d73c0
--- /dev/null
+++ b/clippy_lints/src/matches/match_like_matches.rs
@@ -0,0 +1,166 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+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::{BorrowKind, Expr, ExprKind, Guard, MatchSource, Pat};
+use rustc_lint::LateContext;
+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<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
+ if let Some(higher::IfLet {
+ let_pat,
+ let_expr,
+ if_then,
+ if_else: Some(if_else),
+ }) = higher::IfLet::hir(cx, expr)
+ {
+ return find_matches_sugg(
+ cx,
+ let_expr,
+ IntoIterator::into_iter([(&[][..], Some(let_pat), if_then, None), (&[][..], None, if_else, None)]),
+ expr,
+ true,
+ );
+ }
+
+ if let ExprKind::Match(scrut, arms, MatchSource::Normal) = expr.kind {
+ return find_matches_sugg(
+ cx,
+ scrut,
+ arms.iter().map(|arm| {
+ (
+ cx.tcx.hir().attrs(arm.hir_id),
+ Some(arm.pat),
+ arm.body,
+ arm.guard.as_ref(),
+ )
+ }),
+ expr,
+ false,
+ );
+ }
+
+ 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 Guard<'b>>,
+ ),
+ >,
+{
+ if_chain! {
+ if iter.len() >= 2;
+ if cx.typeck_results().expr_ty(expr).is_bool();
+ if let Some((_, last_pat_opt, last_expr, _)) = iter.next_back();
+ let iter_without_last = iter.clone();
+ if let Some((first_attrs, _, first_expr, first_guard)) = iter.next();
+ if let Some(b0) = find_bool_lit(&first_expr.kind, is_if_let);
+ if let Some(b1) = find_bool_lit(&last_expr.kind, is_if_let);
+ if b0 != b1;
+ if first_guard.is_none() || iter.len() == 0;
+ if first_attrs.is_empty();
+ if iter
+ .all(|arm| {
+ find_bool_lit(&arm.2.kind, is_if_let).map_or(false, |b| b == b0) && arm.3.is_none() && arm.0.is_empty()
+ });
+ then {
+ if let Some(last_pat) = last_pat_opt {
+ if !is_wild(last_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 _;
+ iter_without_last
+ .filter_map(|arm| {
+ let pat_span = arm.1?.span;
+ Some(snippet_with_applicability(cx, pat_span, "..", &mut applicability))
+ })
+ .join(" | ")
+ };
+ let pat_and_guard = if let Some(Guard::If(g)) = first_guard {
+ format!("{} if {}", pat, snippet_with_applicability(cx, g.span, "..", &mut applicability))
+ } else {
+ pat
+ };
+
+ // 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 {
+ if 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" }),
+ "try this",
+ format!(
+ "{}matches!({}, {})",
+ if b0 { "" } else { "!" },
+ snippet_with_applicability(cx, ex_new.span, "..", &mut applicability),
+ pat_and_guard,
+ ),
+ applicability,
+ );
+ true
+ } else {
+ false
+ }
+ }
+}
+
+/// Extract a `bool` or `{ bool }`
+fn find_bool_lit(ex: &ExprKind<'_>, is_if_let: bool) -> Option<bool> {
+ match ex {
+ ExprKind::Lit(Spanned {
+ node: LitKind::Bool(b), ..
+ }) => Some(*b),
+ ExprKind::Block(
+ rustc_hir::Block {
+ stmts: &[],
+ expr: Some(exp),
+ ..
+ },
+ _,
+ ) if is_if_let => {
+ if let ExprKind::Lit(Spanned {
+ node: LitKind::Bool(b), ..
+ }) = exp.kind
+ {
+ Some(b)
+ } else {
+ None
+ }
+ },
+ _ => None,
+ }
+}
diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs
index 67fb918d20c..ff07fc6d4d4 100644
--- a/clippy_lints/src/matches/mod.rs
+++ b/clippy_lints/src/matches/mod.rs
@@ -3,6 +3,7 @@ use clippy_utils::diagnostics::{
multispan_sugg, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
};
use clippy_utils::macros::{is_panic, root_macro_call};
+use clippy_utils::peel_blocks_with_stmt;
use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs};
@@ -12,28 +13,28 @@ use clippy_utils::{
path_to_local, path_to_local_id, peel_blocks, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns,
strip_pat_refs,
};
-use clippy_utils::{higher, peel_blocks_with_stmt};
use clippy_utils::{paths, search_same, SpanlessEq, SpanlessHash};
-use core::iter::{once, ExactSizeIterator};
+use core::iter::once;
use if_chain::if_chain;
-use rustc_ast::ast::{Attribute, LitKind};
+use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_hir::{
- self as hir, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Guard, HirId, Local, MatchSource,
- Mutability, Node, Pat, PatKind, PathSegment, QPath, RangeEnd, TyKind,
+ self as hir, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, HirId, Local, MatchSource, Mutability,
+ Node, Pat, PatKind, PathSegment, QPath, RangeEnd, TyKind,
};
use rustc_hir::{HirIdMap, HirIdSet};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, Ty, TyS, VariantDef};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::source_map::{Span, Spanned};
-use rustc_span::{sym, symbol::kw};
+use rustc_span::{sym, symbol::kw, Span};
use std::cmp::{max, Ordering};
use std::collections::hash_map::Entry;
+mod match_like_matches;
+
declare_clippy_lint! {
/// ### What it does
/// Checks for matches with a single arm where an `if let`
@@ -622,7 +623,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
redundant_pattern_match::check(cx, expr);
if meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO) {
- if !check_match_like_matches(cx, expr) {
+ if !match_like_matches::check(cx, expr) {
lint_match_arms(cx, expr);
}
} else {
@@ -1382,161 +1383,6 @@ fn check_wild_in_or_pats(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
}
}
-/// Lint a `match` or `if let .. { .. } else { .. }` expr that could be replaced by `matches!`
-fn check_match_like_matches<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
- if let Some(higher::IfLet {
- let_pat,
- let_expr,
- if_then,
- if_else: Some(if_else),
- }) = higher::IfLet::hir(cx, expr)
- {
- return find_matches_sugg(
- cx,
- let_expr,
- IntoIterator::into_iter([(&[][..], Some(let_pat), if_then, None), (&[][..], None, if_else, None)]),
- expr,
- true,
- );
- }
-
- if let ExprKind::Match(scrut, arms, MatchSource::Normal) = expr.kind {
- return find_matches_sugg(
- cx,
- scrut,
- arms.iter().map(|arm| {
- (
- cx.tcx.hir().attrs(arm.hir_id),
- Some(arm.pat),
- arm.body,
- arm.guard.as_ref(),
- )
- }),
- expr,
- false,
- );
- }
-
- 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 Guard<'b>>,
- ),
- >,
-{
- if_chain! {
- if iter.len() >= 2;
- if cx.typeck_results().expr_ty(expr).is_bool();
- if let Some((_, last_pat_opt, last_expr, _)) = iter.next_back();
- let iter_without_last = iter.clone();
- if let Some((first_attrs, _, first_expr, first_guard)) = iter.next();
- if let Some(b0) = find_bool_lit(&first_expr.kind, is_if_let);
- if let Some(b1) = find_bool_lit(&last_expr.kind, is_if_let);
- if b0 != b1;
- if first_guard.is_none() || iter.len() == 0;
- if first_attrs.is_empty();
- if iter
- .all(|arm| {
- find_bool_lit(&arm.2.kind, is_if_let).map_or(false, |b| b == b0) && arm.3.is_none() && arm.0.is_empty()
- });
- then {
- if let Some(last_pat) = last_pat_opt {
- if !is_wild(last_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 _;
- iter_without_last
- .filter_map(|arm| {
- let pat_span = arm.1?.span;
- Some(snippet_with_applicability(cx, pat_span, "..", &mut applicability))
- })
- .join(" | ")
- };
- let pat_and_guard = if let Some(Guard::If(g)) = first_guard {
- format!("{} if {}", pat, snippet_with_applicability(cx, g.span, "..", &mut applicability))
- } else {
- pat
- };
-
- // 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 {
- if 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" }),
- "try this",
- format!(
- "{}matches!({}, {})",
- if b0 { "" } else { "!" },
- snippet_with_applicability(cx, ex_new.span, "..", &mut applicability),
- pat_and_guard,
- ),
- applicability,
- );
- true
- } else {
- false
- }
- }
-}
-
-/// Extract a `bool` or `{ bool }`
-fn find_bool_lit(ex: &ExprKind<'_>, is_if_let: bool) -> Option<bool> {
- match ex {
- ExprKind::Lit(Spanned {
- node: LitKind::Bool(b), ..
- }) => Some(*b),
- ExprKind::Block(
- rustc_hir::Block {
- stmts: &[],
- expr: Some(exp),
- ..
- },
- _,
- ) if is_if_let => {
- if let ExprKind::Lit(Spanned {
- node: LitKind::Bool(b), ..
- }) = exp.kind
- {
- Some(b)
- } else {
- None
- }
- },
- _ => None,
- }
-}
-
#[allow(clippy::too_many_lines)]
fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
if expr.span.from_expansion() || arms.len() != 1 || is_refutable(cx, arms[0].pat) {