summary refs log tree commit diff
diff options
context:
space:
mode:
authorManish Goregaokar <manishsmail@gmail.com>2025-03-31 16:42:36 +0000
committerGitHub <noreply@github.com>2025-03-31 16:42:36 +0000
commit7d3d824d648e80023c0245f55afdc66d5f51a821 (patch)
treeae7d4e8f3b9e44deb336487106b132b5d0078ad1
parent1e78abca676460c0ac3c57b9230c104c5d90e3ee (diff)
parent432a8a7a7cbf057e481e698f4478be1006612b03 (diff)
downloadrust-7d3d824d648e80023c0245f55afdc66d5f51a821.tar.gz
rust-7d3d824d648e80023c0245f55afdc66d5f51a821.zip
Properly handle expansion in `single_match` (#14495)
Having a macro call as the scrutinee is supported. However, the proposed
suggestion must use the macro call itself, not its expansion.

When the scrutinee is a macro call, do not complain about an irrefutable
match, as the user may not be aware of the result of the macro. A
comparaison will be suggested instead, as if we couldn't see the outcome
of the macro.

Similarly, do not accept macro calls as arm patterns.

changelog: [`single_match`]: proper suggestions in presence of macros

Fixes #14493
-rw-r--r--clippy_lints/src/matches/single_match.rs17
-rw-r--r--tests/ui/single_match.fixed36
-rw-r--r--tests/ui/single_match.rs42
-rw-r--r--tests/ui/single_match.stderr20
4 files changed, 106 insertions, 9 deletions
diff --git a/clippy_lints/src/matches/single_match.rs b/clippy_lints/src/matches/single_match.rs
index 56fbd626eef..735ba63eb77 100644
--- a/clippy_lints/src/matches/single_match.rs
+++ b/clippy_lints/src/matches/single_match.rs
@@ -1,5 +1,7 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::source::{SpanRangeExt, expr_block, snippet, snippet_block_with_context};
+use clippy_utils::source::{
+    SpanRangeExt, expr_block, snippet, snippet_block_with_context, snippet_with_applicability, snippet_with_context,
+};
 use clippy_utils::ty::implements_trait;
 use clippy_utils::{
     is_lint_allowed, is_unit_expr, peel_blocks, peel_hir_pat_refs, peel_middle_ty_refs, peel_n_hir_expr_refs,
@@ -34,8 +36,7 @@ fn empty_arm_has_comment(cx: &LateContext<'_>, span: Span) -> bool {
 #[rustfmt::skip]
 pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>], expr: &'tcx Expr<'_>, contains_comments: bool) {
     if let [arm1, arm2] = arms
-        && arm1.guard.is_none()
-        && arm2.guard.is_none()
+        && !arms.iter().any(|arm| arm.guard.is_some() || arm.pat.span.from_expansion())
         && !expr.span.from_expansion()
         // don't lint for or patterns for now, this makes
         // the lint noisy in unnecessary situations
@@ -106,7 +107,7 @@ fn report_single_pattern(
         format!(" else {}", expr_block(cx, els, ctxt, "..", Some(expr.span), &mut app))
     });
 
-    if snippet(cx, ex.span, "..") == snippet(cx, arm.pat.span, "..") {
+    if ex.span.eq_ctxt(expr.span) && snippet(cx, ex.span, "..") == snippet(cx, arm.pat.span, "..") {
         let msg = "this pattern is irrefutable, `match` is useless";
         let (sugg, help) = if is_unit_expr(arm.body) {
             (String::new(), "`match` expression can be removed")
@@ -163,10 +164,10 @@ fn report_single_pattern(
         let msg = "you seem to be trying to use `match` for an equality check. Consider using `if`";
         let sugg = format!(
             "if {} == {}{} {}{els_str}",
-            snippet(cx, ex.span, ".."),
+            snippet_with_context(cx, ex.span, ctxt, "..", &mut app).0,
             // PartialEq for different reference counts may not exist.
             "&".repeat(ref_count_diff),
-            snippet(cx, arm.pat.span, ".."),
+            snippet_with_applicability(cx, arm.pat.span, "..", &mut app),
             expr_block(cx, arm.body, ctxt, "..", Some(expr.span), &mut app),
         );
         (msg, sugg)
@@ -174,8 +175,8 @@ fn report_single_pattern(
         let msg = "you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`";
         let sugg = format!(
             "if let {} = {} {}{els_str}",
-            snippet(cx, arm.pat.span, ".."),
-            snippet(cx, ex.span, ".."),
+            snippet_with_applicability(cx, arm.pat.span, "..", &mut app),
+            snippet_with_context(cx, ex.span, ctxt, "..", &mut app).0,
             expr_block(cx, arm.body, ctxt, "..", Some(expr.span), &mut app),
         );
         (msg, sugg)
diff --git a/tests/ui/single_match.fixed b/tests/ui/single_match.fixed
index 0e198ec7934..db5107600ee 100644
--- a/tests/ui/single_match.fixed
+++ b/tests/ui/single_match.fixed
@@ -366,3 +366,39 @@ fn irrefutable_match() {
     //~^^^^^^^^^ single_match
     //~| NOTE: you might want to preserve the comments from inside the `match`
 }
+
+fn issue_14493() {
+    macro_rules! mac {
+        (some) => {
+            Some(42)
+        };
+        (any) => {
+            _
+        };
+        (str) => {
+            "foo"
+        };
+    }
+
+    if let Some(u) = mac!(some) { println!("{u}") }
+    //~^^^^ single_match
+
+    // When scrutinee comes from macro, do not tell that arm will always match
+    // and suggest an equality check instead.
+    if mac!(str) == "foo" { println!("eq") }
+    //~^^^^ ERROR: for an equality check
+
+    // Do not lint if any match arm come from expansion
+    match Some(0) {
+        mac!(some) => println!("eq"),
+        mac!(any) => println!("neq"),
+    }
+    match Some(0) {
+        Some(42) => println!("eq"),
+        mac!(any) => println!("neq"),
+    }
+    match Some(0) {
+        mac!(some) => println!("eq"),
+        _ => println!("neq"),
+    }
+}
diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs
index fcac65f8aaf..a367b94c4ca 100644
--- a/tests/ui/single_match.rs
+++ b/tests/ui/single_match.rs
@@ -461,3 +461,45 @@ fn irrefutable_match() {
     //~^^^^^^^^^ single_match
     //~| NOTE: you might want to preserve the comments from inside the `match`
 }
+
+fn issue_14493() {
+    macro_rules! mac {
+        (some) => {
+            Some(42)
+        };
+        (any) => {
+            _
+        };
+        (str) => {
+            "foo"
+        };
+    }
+
+    match mac!(some) {
+        Some(u) => println!("{u}"),
+        _ => (),
+    }
+    //~^^^^ single_match
+
+    // When scrutinee comes from macro, do not tell that arm will always match
+    // and suggest an equality check instead.
+    match mac!(str) {
+        "foo" => println!("eq"),
+        _ => (),
+    }
+    //~^^^^ ERROR: for an equality check
+
+    // Do not lint if any match arm come from expansion
+    match Some(0) {
+        mac!(some) => println!("eq"),
+        mac!(any) => println!("neq"),
+    }
+    match Some(0) {
+        Some(42) => println!("eq"),
+        mac!(any) => println!("neq"),
+    }
+    match Some(0) {
+        mac!(some) => println!("eq"),
+        _ => println!("neq"),
+    }
+}
diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr
index 2467423b9c1..1a4edc45c92 100644
--- a/tests/ui/single_match.stderr
+++ b/tests/ui/single_match.stderr
@@ -321,5 +321,23 @@ LL +         println!("{u}");
 LL +     }
    |
 
-error: aborting due to 29 previous errors
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> tests/ui/single_match.rs:478:5
+   |
+LL | /     match mac!(some) {
+LL | |         Some(u) => println!("{u}"),
+LL | |         _ => (),
+LL | |     }
+   | |_____^ help: try: `if let Some(u) = mac!(some) { println!("{u}") }`
+
+error: you seem to be trying to use `match` for an equality check. Consider using `if`
+  --> tests/ui/single_match.rs:486:5
+   |
+LL | /     match mac!(str) {
+LL | |         "foo" => println!("eq"),
+LL | |         _ => (),
+LL | |     }
+   | |_____^ help: try: `if mac!(str) == "foo" { println!("eq") }`
+
+error: aborting due to 31 previous errors