Auto merge of #11175 - y21:issue11174, r=Manishearth

[`redundant_pattern_matching`]: include guard in suggestion

Fixes #11174

changelog: [`redundant_pattern_matching`]: include guard in suggestion

Author: bors

clippy_lints/src/matches/redundant_pattern_match.rs

@@ -3,18 +3,20 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{snippet, walk_span_to_context};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop};
-use clippy_utils::visitors::any_temporaries_need_ordered_drop;
+use clippy_utils::visitors::{any_temporaries_need_ordered_drop, for_each_expr};
 use clippy_utils::{higher, is_expn_of, is_trait_method};
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::LangItem::{self, OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk};
-use rustc_hir::{Arm, Expr, ExprKind, Node, Pat, PatKind, QPath, UnOp};
+use rustc_hir::{Arm, Expr, ExprKind, Guard, Node, Pat, PatKind, QPath, UnOp};
 use rustc_lint::LateContext;
 use rustc_middle::ty::subst::GenericArgKind;
 use rustc_middle::ty::{self, Ty};
 use rustc_span::{sym, 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) {
@@ -202,30 +204,58 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op
     if arms.len() == 2 {
         let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
 
-        if let Some(good_method) = found_good_method(cx, arms, node_pair) {
+        if let Some((good_method, maybe_guard)) = found_good_method(cx, arms, node_pair) {
             let span = is_expn_of(expr.span, "matches").unwrap_or(expr.span.to(op.span));
             let result_expr = match &op.kind {
                 ExprKind::AddrOf(_, _, borrowed) => borrowed,
                 _ => op,
             };
+            let mut sugg = format!("{}.{good_method}", snippet(cx, result_expr.span, "_"));
+
+            if let Some(guard) = maybe_guard {
+                let Guard::If(guard) = *guard else { return }; // `...is_none() && let ...` is a syntax error
+
+                // wow, the HIR for match guards in `PAT if let PAT = expr && expr => ...` is annoying!
+                // `guard` here is `Guard::If` with the let expression somewhere deep in the tree of exprs,
+                // counter to the intuition that it should be `Guard::IfLet`, so we need another check
+                // to see that there aren't any let chains anywhere in the guard, as that would break
+                // if we suggest `t.is_none() && (let X = y && z)` for:
+                // `match t { None if let X = y && z => true, _ => false }`
+                let has_nested_let_chain = for_each_expr(guard, |expr| {
+                    if matches!(expr.kind, ExprKind::Let(..)) {
+                        ControlFlow::Break(())
+                    } else {
+                        ControlFlow::Continue(())
+                    }
+                })
+                .is_some();
+
+                if has_nested_let_chain {
+                    return;
+                }
+
+                let guard = Sugg::hir(cx, guard, "..");
+                let _ = write!(sugg, " && {}", guard.maybe_par());
+            }
+
             span_lint_and_sugg(
                 cx,
                 REDUNDANT_PATTERN_MATCHING,
                 span,
                 &format!("redundant pattern matching, consider using `{good_method}`"),
                 "try",
-                format!("{}.{good_method}", snippet(cx, result_expr.span, "_")),
+                sugg,
                 Applicability::MachineApplicable,
             );
         }
     }
 }
 
-fn found_good_method<'a>(
+fn found_good_method<'tcx>(
     cx: &LateContext<'_>,
-    arms: &[Arm<'_>],
+    arms: &'tcx [Arm<'tcx>],
     node: (&PatKind<'_>, &PatKind<'_>),
-) -> Option<&'a str> {
+) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> {
     match node {
         (
             PatKind::TupleStruct(ref path_left, patterns_left, _),
@@ -311,7 +341,11 @@ fn get_ident(path: &QPath<'_>) -> Option<rustc_span::symbol::Ident> {
     }
 }
 
-fn get_good_method<'a>(cx: &LateContext<'_>, arms: &[Arm<'_>], path_left: &QPath<'_>) -> Option<&'a str> {
+fn get_good_method<'tcx>(
+    cx: &LateContext<'_>,
+    arms: &'tcx [Arm<'tcx>],
+    path_left: &QPath<'_>,
+) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> {
     if let Some(name) = get_ident(path_left) {
         return match name.as_str() {
             "Ok" => {
@@ -377,16 +411,16 @@ fn is_pat_variant(cx: &LateContext<'_>, pat: &Pat<'_>, path: &QPath<'_>, expecte
 }
 
 #[expect(clippy::too_many_arguments)]
-fn find_good_method_for_match<'a>(
+fn find_good_method_for_match<'a, 'tcx>(
     cx: &LateContext<'_>,
-    arms: &[Arm<'_>],
+    arms: &'tcx [Arm<'tcx>],
     path_left: &QPath<'_>,
     path_right: &QPath<'_>,
     expected_item_left: Item,
     expected_item_right: Item,
     should_be_left: &'a str,
     should_be_right: &'a str,
-) -> Option<&'a str> {
+) -> Option<(&'a str, Option<&'tcx Guard<'tcx>>)> {
     let first_pat = arms[0].pat;
     let second_pat = arms[1].pat;
 
@@ -404,22 +438,22 @@ fn find_good_method_for_match<'a>(
 
     match body_node_pair {
         (ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) {
-            (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
-            (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
+            (LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard.as_ref())),
+            (LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard.as_ref())),
             _ => None,
         },
         _ => None,
     }
 }
 
-fn find_good_method_for_matches_macro<'a>(
+fn find_good_method_for_matches_macro<'a, 'tcx>(
     cx: &LateContext<'_>,
-    arms: &[Arm<'_>],
+    arms: &'tcx [Arm<'tcx>],
     path_left: &QPath<'_>,
     expected_item_left: Item,
     should_be_left: &'a str,
     should_be_right: &'a str,
-) -> Option<&'a str> {
+) -> Option<(&'a str, Option<&'tcx Guard<'tcx>>)> {
     let first_pat = arms[0].pat;
 
     let body_node_pair = if is_pat_variant(cx, first_pat, path_left, expected_item_left) {
@@ -430,8 +464,8 @@ fn find_good_method_for_matches_macro<'a>(
 
     match body_node_pair {
         (ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) {
-            (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
-            (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
+            (LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard.as_ref())),
+            (LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard.as_ref())),
             _ => None,
         },
         _ => None,

tests/ui/redundant_pattern_matching_option.fixed

@@ -10,6 +10,20 @@
     clippy::equatable_if_let,
     clippy::if_same_then_else
 )]
+#![feature(let_chains, if_let_guard)]
+
+fn issue_11174<T>(boolean: bool, maybe_some: Option<T>) -> bool {
+    maybe_some.is_none() && (!boolean)
+}
+
+fn issue_11174_edge_cases<T>(boolean: bool, boolean2: bool, maybe_some: Option<T>) {
+    let _ = maybe_some.is_none() && (boolean || boolean2); // guard needs parentheses
+    let _ = match maybe_some { // can't use `matches!` here
+                               // because `expr` metavars in macros don't allow let exprs
+        None if let Some(x) = Some(0) && x > 5 => true,
+        _ => false
+    };
+}
 
 fn main() {
     if None::<()>.is_none() {}

tests/ui/redundant_pattern_matching_option.rs

@@ -10,6 +10,20 @@
     clippy::equatable_if_let,
     clippy::if_same_then_else
 )]
+#![feature(let_chains, if_let_guard)]
+
+fn issue_11174<T>(boolean: bool, maybe_some: Option<T>) -> bool {
+    matches!(maybe_some, None if !boolean)
+}
+
+fn issue_11174_edge_cases<T>(boolean: bool, boolean2: bool, maybe_some: Option<T>) {
+    let _ = matches!(maybe_some, None if boolean || boolean2); // guard needs parentheses
+    let _ = match maybe_some { // can't use `matches!` here
+                               // because `expr` metavars in macros don't allow let exprs
+        None if let Some(x) = Some(0) && x > 5 => true,
+        _ => false
+    };
+}
 
 fn main() {
     if let None = None::<()> {}