diff options
| -rw-r--r-- | clippy_lints/src/format_push_string.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/loops/manual_flatten.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/manual_let_else.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/matches/collapsible_match.rs | 4 | ||||
| -rw-r--r-- | clippy_lints/src/matches/mod.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/matches/redundant_pattern_match.rs | 34 | ||||
| -rw-r--r-- | clippy_lints/src/methods/filter_map.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/option_if_let_else.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/question_mark.rs | 1 | ||||
| -rw-r--r-- | clippy_utils/src/higher.rs | 13 |
10 files changed, 50 insertions, 12 deletions
diff --git a/clippy_lints/src/format_push_string.rs b/clippy_lints/src/format_push_string.rs index 3901dd984f9..2b08437d827 100644 --- a/clippy_lints/src/format_push_string.rs +++ b/clippy_lints/src/format_push_string.rs @@ -57,7 +57,7 @@ fn is_format(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { Some(higher::IfLetOrMatch::Match(_, arms, MatchSource::Normal)) => { 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)) }, _ => false, diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs index a726b116956..36de9021f49 100644 --- a/clippy_lints/src/loops/manual_flatten.rs +++ b/clippy_lints/src/loops/manual_flatten.rs @@ -20,7 +20,7 @@ pub(super) fn check<'tcx>( span: Span, ) { 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) // 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 diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 92dc4d57ab1..fdf8fa4e277 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -61,7 +61,7 @@ impl<'tcx> QuestionMark { && let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) { 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) && let Some(if_else) = if_else && is_never_expr(cx, if_else).is_some() diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs index 48fc5746b3c..91e6ca7fa8b 100644 --- a/clippy_lints/src/matches/collapsible_match.rs +++ b/clippy_lints/src/matches/collapsible_match.rs @@ -41,7 +41,7 @@ fn check_arm<'tcx>( let inner_expr = peel_blocks_with_stmt(outer_then_body); if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr) && 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()) // if there are more than two arms, collapsing would be non-trivial // one of the arms must be "wild-like" @@ -75,7 +75,7 @@ fn check_arm<'tcx>( ) // ...or anywhere in the inner expression && 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)) }, IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| is_local_used(cx, arm, binding_id)), diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 4c7568f39b4..af6894545b8 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -1104,6 +1104,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { if_let.let_pat, if_let.let_expr, if_let.if_else.is_some(), + if_let.let_span, ); needless_match::check_if_let(cx, expr, &if_let); } diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 2582f7edcf6..81c084b979a 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -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_lint::LateContext; use rustc_middle::ty::{self, GenericArgKind, Ty}; -use rustc_span::{sym, Symbol}; +use rustc_span::{sym, Span, Symbol}; use std::fmt::Write; use std::ops::ControlFlow; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx 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<'_>, scrutinee: &'tcx Expr<'_>, 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 @@ -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>, expr: &'tcx Expr<'_>, let_pat: &Pat<'_>, diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 844ab40cab1..db15bf2ad6c 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -186,7 +186,7 @@ impl<'tcx> OffendingFilterExpr<'tcx> { 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 // 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) => { (sc, else_, ident, span) diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index 89e4e3c740d..556c493d36c 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -238,6 +238,7 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> let_expr, if_then, if_else: Some(if_else), + .. }) = higher::IfLet::hir(cx, expr) && !cx.typeck_results().expr_ty(expr).is_unit() && !is_else_clause(cx.tcx, expr) diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index fc5835408a9..6add1a81a0a 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -256,6 +256,7 @@ impl QuestionMark { let_expr, if_then, if_else, + .. }) = higher::IfLet::hir(cx, expr) && !is_else_clause(cx.tcx, expr) && let PatKind::TupleStruct(ref path1, [field], ddpos) = let_pat.kind diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 3135a033648..b2054bf7d41 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -91,6 +91,9 @@ pub struct IfLet<'hir> { pub if_then: &'hir Expr<'hir>, /// `if let` else expression pub if_else: Option<&'hir Expr<'hir>>, + /// `if let PAT = EXPR` + /// ^^^^^^^^^^^^^^ + pub let_span: Span, } impl<'hir> IfLet<'hir> { @@ -99,9 +102,10 @@ impl<'hir> IfLet<'hir> { if let ExprKind::If( Expr { kind: - ExprKind::Let(hir::Let { + ExprKind::Let(&hir::Let { pat: let_pat, init: let_expr, + span: let_span, .. }), .. @@ -129,6 +133,7 @@ impl<'hir> IfLet<'hir> { let_expr, if_then, if_else, + let_span, }); } None @@ -146,6 +151,9 @@ pub enum IfLetOrMatch<'hir> { &'hir Pat<'hir>, &'hir Expr<'hir>, Option<&'hir Expr<'hir>>, + /// `if let PAT = EXPR` + /// ^^^^^^^^^^^^^^ + Span, ), } @@ -160,7 +168,8 @@ impl<'hir> IfLetOrMatch<'hir> { let_pat, if_then, 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) }, ), } } |