fix(equatable_if_let): don't lint if pattern or initializer come from expansion (#15958)

dswij · web-flow · commit df627dd68165 · 2025-11-27T15:59:02.000Z
This means some existing test cases won't get linted anymore, but imo that's preferable to false-positives. Fixes rust-lang/rust-clippy#14548 changelog: [`equatable_if_let`]: don't lint if pattern or initializer come from expansion
diff --git a/clippy_lints/src/equatable_if_let.rs b/clippy_lints/src/equatable_if_let.rs
@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::is_in_const_context;
 use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::implements_trait;
@@ -41,9 +41,9 @@ declare_clippy_lint! {
 declare_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]);
 
 /// detects if pattern matches just one thing
-fn unary_pattern(pat: &Pat<'_>) -> bool {
+fn is_unary_pattern(pat: &Pat<'_>) -> bool {
     fn array_rec(pats: &[Pat<'_>]) -> bool {
-        pats.iter().all(unary_pattern)
+        pats.iter().all(is_unary_pattern)
     }
     match &pat.kind {
         PatKind::Missing => unreachable!(),
@@ -54,9 +54,9 @@ fn unary_pattern(pat: &Pat<'_>) -> bool {
         | PatKind::Never
         | PatKind::Or(_)
         | PatKind::Err(_) => false,
-        PatKind::Struct(_, a, etc) => etc.is_none() && a.iter().all(|x| unary_pattern(x.pat)),
+        PatKind::Struct(_, a, etc) => etc.is_none() && a.iter().all(|x| is_unary_pattern(x.pat)),
         PatKind::Tuple(a, etc) | PatKind::TupleStruct(_, a, etc) => etc.as_opt_usize().is_none() && array_rec(a),
-        PatKind::Ref(x, _, _) | PatKind::Box(x) | PatKind::Deref(x) | PatKind::Guard(x, _) => unary_pattern(x),
+        PatKind::Ref(x, _, _) | PatKind::Box(x) | PatKind::Deref(x) | PatKind::Guard(x, _) => is_unary_pattern(x),
         PatKind::Expr(_) => true,
     }
 }
@@ -104,12 +104,16 @@ fn contains_type_mismatch(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {
 impl<'tcx> LateLintPass<'tcx> for PatternEquality {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
         if let ExprKind::Let(let_expr) = expr.kind
-            && unary_pattern(let_expr.pat)
+            && is_unary_pattern(let_expr.pat)
             && !expr.span.in_external_macro(cx.sess().source_map())
+            && !let_expr.pat.span.from_expansion()
+            && !let_expr.init.span.from_expansion()
         {
             let exp_ty = cx.typeck_results().expr_ty(let_expr.init);
             let pat_ty = cx.typeck_results().pat_ty(let_expr.pat);
-            let mut applicability = Applicability::MachineApplicable;
+
+            let mut app = Applicability::MachineApplicable;
+            let ctxt = expr.span.ctxt();
 
             if is_structural_partial_eq(cx, exp_ty, pat_ty)
                 && !contains_type_mismatch(cx, let_expr.pat)
@@ -121,40 +125,42 @@ impl<'tcx> LateLintPass<'tcx> for PatternEquality {
                 // but that didn't quite work out (see #15482), so we just reject outright in this case
                 && !is_in_const_context(cx)
             {
-                let pat_str = match let_expr.pat.kind {
-                    PatKind::Struct(..) => format!(
-                        "({})",
-                        snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0,
-                    ),
-                    _ => snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability)
-                        .0
-                        .to_string(),
-                };
-                span_lint_and_sugg(
+                span_lint_and_then(
                     cx,
                     EQUATABLE_IF_LET,
                     expr.span,
                     "this pattern matching can be expressed using equality",
-                    "try",
-                    format!(
-                        "{} == {pat_str}",
-                        snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0,
-                    ),
-                    applicability,
+                    |diag| {
+                        let pat_str = {
+                            let str = snippet_with_context(cx, let_expr.pat.span, ctxt, "..", &mut app).0;
+                            if let PatKind::Struct(..) = let_expr.pat.kind {
+                                format!("({str})").into()
+                            } else {
+                                str
+                            }
+                        };
+
+                        let sugg = format!(
+                            "{} == {pat_str}",
+                            snippet_with_context(cx, let_expr.init.span, ctxt, "..", &mut app).0,
+                        );
+                        diag.span_suggestion(expr.span, "try", sugg, app);
+                    },
                 );
             } else {
-                span_lint_and_sugg(
+                span_lint_and_then(
                     cx,
                     EQUATABLE_IF_LET,
                     expr.span,
                     "this pattern matching can be expressed using `matches!`",
-                    "try",
-                    format!(
-                        "matches!({}, {})",
-                        snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0,
-                        snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0,
-                    ),
-                    applicability,
+                    |diag| {
+                        let sugg = format!(
+                            "matches!({}, {})",
+                            snippet_with_context(cx, let_expr.init.span, ctxt, "..", &mut app).0,
+                            snippet_with_context(cx, let_expr.pat.span, ctxt, "..", &mut app).0,
+                        );
+                        diag.span_suggestion(expr.span, "try", sugg, app);
+                    },
                 );
             }
         }
diff --git a/tests/ui/equatable_if_let.fixed b/tests/ui/equatable_if_let.fixed
@@ -1,14 +1,6 @@
 //@aux-build:proc_macros.rs
-
-#![allow(
-    unused_variables,
-    dead_code,
-    clippy::derive_partial_eq_without_eq,
-    clippy::needless_ifs
-)]
+#![allow(clippy::derive_partial_eq_without_eq, clippy::needless_ifs)]
 #![warn(clippy::equatable_if_let)]
-
-extern crate proc_macros;
 use proc_macros::{external, inline_macros};
 
 use std::cmp::Ordering;
@@ -48,7 +40,6 @@ impl PartialEq for NotStructuralEq {
     }
 }
 
-#[inline_macros]
 fn main() {
     let a = 2;
     let b = 3;
@@ -95,13 +86,6 @@ fn main() {
     //~^ equatable_if_let
     if matches!(h, NoPartialEqStruct { a: 2, b: false }) {}
     //~^ equatable_if_let
-
-    if "abc" == inline!("abc") {
-        //~^ equatable_if_let
-        println!("OK");
-    }
-
-    external!({ if let 2 = $a {} });
 }
 
 mod issue8710 {
@@ -140,6 +124,74 @@ mod issue8710 {
     }
 }
 
+#[inline_macros]
+fn issue14548() {
+    if let inline!("abc") = "abc" {
+        println!("OK");
+    }
+
+    let a = 2;
+    external!({ if let 2 = $a {} });
+
+    // Don't lint: `==`/`matches!` might be correct for a particular `$($font)|*`, but not in general
+    macro_rules! m1 {
+        ($($font:pat_param)|*) => {
+            if let $($font)|* = "from_expansion" {}
+        }
+    }
+    m1!("foo");
+    m1!("Sans" | "Serif" | "Sans Mono");
+    m1!(inline!("foo"));
+
+    // Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
+    // general
+    macro_rules! m2 {
+        ($from_root_ctxt:pat) => {
+            if let $from_root_ctxt = "from_expansion" {}
+        };
+    }
+    m2!("foo");
+    m2!("Sans" | "Serif" | "Sans Mono");
+    m2!(inline!("foo"));
+
+    // Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
+    // general
+    macro_rules! m3 {
+        ($from_root_ctxt:expr) => {
+            if let "from_expansion" = $from_root_ctxt {}
+        };
+    }
+    m3!("foo");
+    m3!("foo");
+    m3!(inline!("foo"));
+
+    // Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
+    // general. Don't get confused by the scrutinee coming from macro invocation
+    macro_rules! m4 {
+        ($from_root_ctxt:pat) => {
+            if let $from_root_ctxt = inline!("from_expansion") {}
+        };
+    }
+    m4!("foo");
+    m4!("Sans" | "Serif" | "Sans Mono");
+    m4!(inline!("foo"));
+
+    // Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
+    // general. Don't get confused by the scrutinee coming from macro invocation
+    macro_rules! m5 {
+        ($from_root_ctxt:expr) => {
+            if let inline!("from_expansion") = $from_root_ctxt {}
+        };
+    }
+    m5!("foo");
+    m5!("foo");
+    m5!(inline!("foo"));
+
+    // Would be nice to lint: both sides are macro _invocations_, so the suggestion is correct in
+    // general
+    if let inline!("foo") = inline!("bar") {}
+}
+
 // PartialEq is not stable in consts yet
 fn issue15376() {
     enum NonConstEq {
diff --git a/tests/ui/equatable_if_let.rs b/tests/ui/equatable_if_let.rs
@@ -1,14 +1,6 @@
 //@aux-build:proc_macros.rs
-
-#![allow(
-    unused_variables,
-    dead_code,
-    clippy::derive_partial_eq_without_eq,
-    clippy::needless_ifs
-)]
+#![allow(clippy::derive_partial_eq_without_eq, clippy::needless_ifs)]
 #![warn(clippy::equatable_if_let)]
-
-extern crate proc_macros;
 use proc_macros::{external, inline_macros};
 
 use std::cmp::Ordering;
@@ -48,7 +40,6 @@ impl PartialEq for NotStructuralEq {
     }
 }
 
-#[inline_macros]
 fn main() {
     let a = 2;
     let b = 3;
@@ -95,13 +86,6 @@ fn main() {
     //~^ equatable_if_let
     if let NoPartialEqStruct { a: 2, b: false } = h {}
     //~^ equatable_if_let
-
-    if let inline!("abc") = "abc" {
-        //~^ equatable_if_let
-        println!("OK");
-    }
-
-    external!({ if let 2 = $a {} });
 }
 
 mod issue8710 {
@@ -140,6 +124,74 @@ mod issue8710 {
     }
 }
 
+#[inline_macros]
+fn issue14548() {
+    if let inline!("abc") = "abc" {
+        println!("OK");
+    }
+
+    let a = 2;
+    external!({ if let 2 = $a {} });
+
+    // Don't lint: `==`/`matches!` might be correct for a particular `$($font)|*`, but not in general
+    macro_rules! m1 {
+        ($($font:pat_param)|*) => {
+            if let $($font)|* = "from_expansion" {}
+        }
+    }
+    m1!("foo");
+    m1!("Sans" | "Serif" | "Sans Mono");
+    m1!(inline!("foo"));
+
+    // Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
+    // general
+    macro_rules! m2 {
+        ($from_root_ctxt:pat) => {
+            if let $from_root_ctxt = "from_expansion" {}
+        };
+    }
+    m2!("foo");
+    m2!("Sans" | "Serif" | "Sans Mono");
+    m2!(inline!("foo"));
+
+    // Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
+    // general
+    macro_rules! m3 {
+        ($from_root_ctxt:expr) => {
+            if let "from_expansion" = $from_root_ctxt {}
+        };
+    }
+    m3!("foo");
+    m3!("foo");
+    m3!(inline!("foo"));
+
+    // Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
+    // general. Don't get confused by the scrutinee coming from macro invocation
+    macro_rules! m4 {
+        ($from_root_ctxt:pat) => {
+            if let $from_root_ctxt = inline!("from_expansion") {}
+        };
+    }
+    m4!("foo");
+    m4!("Sans" | "Serif" | "Sans Mono");
+    m4!(inline!("foo"));
+
+    // Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
+    // general. Don't get confused by the scrutinee coming from macro invocation
+    macro_rules! m5 {
+        ($from_root_ctxt:expr) => {
+            if let inline!("from_expansion") = $from_root_ctxt {}
+        };
+    }
+    m5!("foo");
+    m5!("foo");
+    m5!(inline!("foo"));
+
+    // Would be nice to lint: both sides are macro _invocations_, so the suggestion is correct in
+    // general
+    if let inline!("foo") = inline!("bar") {}
+}
+
 // PartialEq is not stable in consts yet
 fn issue15376() {
     enum NonConstEq {
diff --git a/tests/ui/equatable_if_let.stderr b/tests/ui/equatable_if_let.stderr
