[`redundant_pattern_matching`]: catch `if let true`

This commit is contained in:
y21 2023-12-16 16:37:58 +01:00
parent a859e5cc1c
commit 850d77ed55
10 changed files with 50 additions and 12 deletions

View File

@ -57,7 +57,7 @@ fn is_format(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
Some(higher::IfLetOrMatch::Match(_, arms, MatchSource::Normal)) => { Some(higher::IfLetOrMatch::Match(_, arms, MatchSource::Normal)) => {
arms.iter().any(|arm| is_format(cx, arm.body)) arms.iter().any(|arm| is_format(cx, arm.body))
}, },
Some(higher::IfLetOrMatch::IfLet(_, _, then, r#else)) => { Some(higher::IfLetOrMatch::IfLet(_, _, then, r#else, _)) => {
is_format(cx, then) || r#else.is_some_and(|e| is_format(cx, e)) is_format(cx, then) || r#else.is_some_and(|e| is_format(cx, e))
}, },
_ => false, _ => false,

View File

@ -20,7 +20,7 @@ pub(super) fn check<'tcx>(
span: Span, span: Span,
) { ) {
let inner_expr = peel_blocks_with_stmt(body); let inner_expr = peel_blocks_with_stmt(body);
if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None }) if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None, .. })
= higher::IfLet::hir(cx, inner_expr) = higher::IfLet::hir(cx, inner_expr)
// Ensure match_expr in `if let` statement is the same as the pat from the for-loop // Ensure match_expr in `if let` statement is the same as the pat from the for-loop
&& let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind && let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind

View File

@ -61,7 +61,7 @@ impl<'tcx> QuestionMark {
&& let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) && let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init)
{ {
match if_let_or_match { match if_let_or_match {
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => { IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else, ..) => {
if let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then) if let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then)
&& let Some(if_else) = if_else && let Some(if_else) = if_else
&& is_never_expr(cx, if_else).is_some() && is_never_expr(cx, if_else).is_some()

View File

@ -41,7 +41,7 @@ fn check_arm<'tcx>(
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)
&& let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner { && let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner {
IfLetOrMatch::IfLet(scrutinee, pat, _, els) => Some((scrutinee, pat, els)), IfLetOrMatch::IfLet(scrutinee, pat, _, els, _) => Some((scrutinee, pat, els)),
IfLetOrMatch::Match(scrutinee, arms, ..) => if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none()) IfLetOrMatch::Match(scrutinee, arms, ..) => if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none())
// if there are more than two arms, collapsing would be non-trivial // if there are more than two arms, collapsing would be non-trivial
// one of the arms must be "wild-like" // one of the arms must be "wild-like"
@ -75,7 +75,7 @@ fn check_arm<'tcx>(
) )
// ...or anywhere in the inner expression // ...or anywhere in the inner expression
&& match inner { && match inner {
IfLetOrMatch::IfLet(_, _, body, els) => { IfLetOrMatch::IfLet(_, _, body, els, _) => {
!is_local_used(cx, body, binding_id) && els.map_or(true, |e| !is_local_used(cx, e, binding_id)) !is_local_used(cx, body, binding_id) && els.map_or(true, |e| !is_local_used(cx, e, binding_id))
}, },
IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| is_local_used(cx, arm, binding_id)), IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| is_local_used(cx, arm, binding_id)),

View File

@ -1104,6 +1104,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
if_let.let_pat, if_let.let_pat,
if_let.let_expr, if_let.let_expr,
if_let.if_else.is_some(), if_let.if_else.is_some(),
if_let.let_span,
); );
needless_match::check_if_let(cx, expr, &if_let); needless_match::check_if_let(cx, expr, &if_let);
} }

View File

@ -12,13 +12,13 @@ use rustc_hir::LangItem::{self, OptionNone, OptionSome, PollPending, PollReady,
use rustc_hir::{Arm, Expr, ExprKind, Guard, Node, Pat, PatKind, QPath, UnOp}; use rustc_hir::{Arm, Expr, ExprKind, Guard, Node, Pat, PatKind, QPath, UnOp};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ty::{self, GenericArgKind, Ty}; use rustc_middle::ty::{self, GenericArgKind, Ty};
use rustc_span::{sym, Symbol}; use rustc_span::{sym, Span, Symbol};
use std::fmt::Write; use std::fmt::Write;
use std::ops::ControlFlow; use std::ops::ControlFlow;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) { if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) {
find_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false); find_method_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false);
} }
} }
@ -28,8 +28,34 @@ pub(super) fn check_if_let<'tcx>(
pat: &'tcx Pat<'_>, pat: &'tcx Pat<'_>,
scrutinee: &'tcx Expr<'_>, scrutinee: &'tcx Expr<'_>,
has_else: bool, has_else: bool,
let_span: Span,
) { ) {
find_sugg_for_if_let(cx, expr, pat, scrutinee, "if", has_else); find_if_let_true(cx, pat, scrutinee, let_span);
find_method_sugg_for_if_let(cx, expr, pat, scrutinee, "if", has_else);
}
fn find_if_let_true<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, scrutinee: &'tcx Expr<'_>, let_span: Span) {
if let PatKind::Lit(lit) = pat.kind
&& let ExprKind::Lit(lit) = lit.kind
&& let LitKind::Bool(is_true) = lit.node
{
let mut snip = snippet(cx, scrutinee.span, "..").into_owned();
if !is_true {
// Invert condition for `if let false = ...`
snip.insert(0, '!');
}
span_lint_and_sugg(
cx,
REDUNDANT_PATTERN_MATCHING,
let_span,
"using `if let` to pattern match a boolean",
"consider using a regular `if` expression",
snip,
Applicability::MachineApplicable,
);
}
} }
// Extract the generic arguments out of a type // Extract the generic arguments out of a type
@ -100,7 +126,7 @@ fn find_method_and_type<'tcx>(
} }
} }
fn find_sugg_for_if_let<'tcx>( fn find_method_sugg_for_if_let<'tcx>(
cx: &LateContext<'tcx>, cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>,
let_pat: &Pat<'_>, let_pat: &Pat<'_>,

View File

@ -186,7 +186,7 @@ impl<'tcx> OffendingFilterExpr<'tcx> {
match higher::IfLetOrMatch::parse(cx, map_body.value) { match higher::IfLetOrMatch::parse(cx, map_body.value) {
// For `if let` we want to check that the variant matching arm references the local created by // For `if let` we want to check that the variant matching arm references the local created by
// its pattern // its pattern
Some(higher::IfLetOrMatch::IfLet(sc, pat, then, Some(else_))) Some(higher::IfLetOrMatch::IfLet(sc, pat, then, Some(else_), ..))
if let Some((ident, span)) = expr_uses_local(pat, then) => if let Some((ident, span)) = expr_uses_local(pat, then) =>
{ {
(sc, else_, ident, span) (sc, else_, ident, span)

View File

@ -238,6 +238,7 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) ->
let_expr, let_expr,
if_then, if_then,
if_else: Some(if_else), if_else: Some(if_else),
..
}) = higher::IfLet::hir(cx, expr) }) = higher::IfLet::hir(cx, expr)
&& !cx.typeck_results().expr_ty(expr).is_unit() && !cx.typeck_results().expr_ty(expr).is_unit()
&& !is_else_clause(cx.tcx, expr) && !is_else_clause(cx.tcx, expr)

View File

@ -256,6 +256,7 @@ impl QuestionMark {
let_expr, let_expr,
if_then, if_then,
if_else, if_else,
..
}) = higher::IfLet::hir(cx, expr) }) = higher::IfLet::hir(cx, expr)
&& !is_else_clause(cx.tcx, expr) && !is_else_clause(cx.tcx, expr)
&& let PatKind::TupleStruct(ref path1, [field], ddpos) = let_pat.kind && let PatKind::TupleStruct(ref path1, [field], ddpos) = let_pat.kind

View File

@ -91,6 +91,9 @@ pub struct IfLet<'hir> {
pub if_then: &'hir Expr<'hir>, pub if_then: &'hir Expr<'hir>,
/// `if let` else expression /// `if let` else expression
pub if_else: Option<&'hir Expr<'hir>>, pub if_else: Option<&'hir Expr<'hir>>,
/// `if let PAT = EXPR`
/// ^^^^^^^^^^^^^^
pub let_span: Span,
} }
impl<'hir> IfLet<'hir> { impl<'hir> IfLet<'hir> {
@ -99,9 +102,10 @@ impl<'hir> IfLet<'hir> {
if let ExprKind::If( if let ExprKind::If(
Expr { Expr {
kind: kind:
ExprKind::Let(hir::Let { ExprKind::Let(&hir::Let {
pat: let_pat, pat: let_pat,
init: let_expr, init: let_expr,
span: let_span,
.. ..
}), }),
.. ..
@ -129,6 +133,7 @@ impl<'hir> IfLet<'hir> {
let_expr, let_expr,
if_then, if_then,
if_else, if_else,
let_span,
}); });
} }
None None
@ -146,6 +151,9 @@ pub enum IfLetOrMatch<'hir> {
&'hir Pat<'hir>, &'hir Pat<'hir>,
&'hir Expr<'hir>, &'hir Expr<'hir>,
Option<&'hir Expr<'hir>>, Option<&'hir Expr<'hir>>,
/// `if let PAT = EXPR`
/// ^^^^^^^^^^^^^^
Span,
), ),
} }
@ -160,7 +168,8 @@ impl<'hir> IfLetOrMatch<'hir> {
let_pat, let_pat,
if_then, if_then,
if_else, if_else,
}| { Self::IfLet(let_expr, let_pat, if_then, if_else) }, let_span,
}| { Self::IfLet(let_expr, let_pat, if_then, if_else, let_span) },
), ),
} }
} }