Skip to content

Commit 7d3d824

Browse files
authored
Properly handle expansion in single_match (rust-lang#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 rust-lang#14493
2 parents 1e78abc + 432a8a7 commit 7d3d824

File tree

4 files changed

+106
-9
lines changed

4 files changed

+106
-9
lines changed

clippy_lints/src/matches/single_match.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::source::{SpanRangeExt, expr_block, snippet, snippet_block_with_context};
2+
use clippy_utils::source::{
3+
SpanRangeExt, expr_block, snippet, snippet_block_with_context, snippet_with_applicability, snippet_with_context,
4+
};
35
use clippy_utils::ty::implements_trait;
46
use clippy_utils::{
57
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 {
3436
#[rustfmt::skip]
3537
pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>], expr: &'tcx Expr<'_>, contains_comments: bool) {
3638
if let [arm1, arm2] = arms
37-
&& arm1.guard.is_none()
38-
&& arm2.guard.is_none()
39+
&& !arms.iter().any(|arm| arm.guard.is_some() || arm.pat.span.from_expansion())
3940
&& !expr.span.from_expansion()
4041
// don't lint for or patterns for now, this makes
4142
// the lint noisy in unnecessary situations
@@ -106,7 +107,7 @@ fn report_single_pattern(
106107
format!(" else {}", expr_block(cx, els, ctxt, "..", Some(expr.span), &mut app))
107108
});
108109

109-
if snippet(cx, ex.span, "..") == snippet(cx, arm.pat.span, "..") {
110+
if ex.span.eq_ctxt(expr.span) && snippet(cx, ex.span, "..") == snippet(cx, arm.pat.span, "..") {
110111
let msg = "this pattern is irrefutable, `match` is useless";
111112
let (sugg, help) = if is_unit_expr(arm.body) {
112113
(String::new(), "`match` expression can be removed")
@@ -163,19 +164,19 @@ fn report_single_pattern(
163164
let msg = "you seem to be trying to use `match` for an equality check. Consider using `if`";
164165
let sugg = format!(
165166
"if {} == {}{} {}{els_str}",
166-
snippet(cx, ex.span, ".."),
167+
snippet_with_context(cx, ex.span, ctxt, "..", &mut app).0,
167168
// PartialEq for different reference counts may not exist.
168169
"&".repeat(ref_count_diff),
169-
snippet(cx, arm.pat.span, ".."),
170+
snippet_with_applicability(cx, arm.pat.span, "..", &mut app),
170171
expr_block(cx, arm.body, ctxt, "..", Some(expr.span), &mut app),
171172
);
172173
(msg, sugg)
173174
} else {
174175
let msg = "you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`";
175176
let sugg = format!(
176177
"if let {} = {} {}{els_str}",
177-
snippet(cx, arm.pat.span, ".."),
178-
snippet(cx, ex.span, ".."),
178+
snippet_with_applicability(cx, arm.pat.span, "..", &mut app),
179+
snippet_with_context(cx, ex.span, ctxt, "..", &mut app).0,
179180
expr_block(cx, arm.body, ctxt, "..", Some(expr.span), &mut app),
180181
);
181182
(msg, sugg)

tests/ui/single_match.fixed

+36
Original file line numberDiff line numberDiff line change
@@ -366,3 +366,39 @@ fn irrefutable_match() {
366366
//~^^^^^^^^^ single_match
367367
//~| NOTE: you might want to preserve the comments from inside the `match`
368368
}
369+
370+
fn issue_14493() {
371+
macro_rules! mac {
372+
(some) => {
373+
Some(42)
374+
};
375+
(any) => {
376+
_
377+
};
378+
(str) => {
379+
"foo"
380+
};
381+
}
382+
383+
if let Some(u) = mac!(some) { println!("{u}") }
384+
//~^^^^ single_match
385+
386+
// When scrutinee comes from macro, do not tell that arm will always match
387+
// and suggest an equality check instead.
388+
if mac!(str) == "foo" { println!("eq") }
389+
//~^^^^ ERROR: for an equality check
390+
391+
// Do not lint if any match arm come from expansion
392+
match Some(0) {
393+
mac!(some) => println!("eq"),
394+
mac!(any) => println!("neq"),
395+
}
396+
match Some(0) {
397+
Some(42) => println!("eq"),
398+
mac!(any) => println!("neq"),
399+
}
400+
match Some(0) {
401+
mac!(some) => println!("eq"),
402+
_ => println!("neq"),
403+
}
404+
}

tests/ui/single_match.rs

+42
Original file line numberDiff line numberDiff line change
@@ -461,3 +461,45 @@ fn irrefutable_match() {
461461
//~^^^^^^^^^ single_match
462462
//~| NOTE: you might want to preserve the comments from inside the `match`
463463
}
464+
465+
fn issue_14493() {
466+
macro_rules! mac {
467+
(some) => {
468+
Some(42)
469+
};
470+
(any) => {
471+
_
472+
};
473+
(str) => {
474+
"foo"
475+
};
476+
}
477+
478+
match mac!(some) {
479+
Some(u) => println!("{u}"),
480+
_ => (),
481+
}
482+
//~^^^^ single_match
483+
484+
// When scrutinee comes from macro, do not tell that arm will always match
485+
// and suggest an equality check instead.
486+
match mac!(str) {
487+
"foo" => println!("eq"),
488+
_ => (),
489+
}
490+
//~^^^^ ERROR: for an equality check
491+
492+
// Do not lint if any match arm come from expansion
493+
match Some(0) {
494+
mac!(some) => println!("eq"),
495+
mac!(any) => println!("neq"),
496+
}
497+
match Some(0) {
498+
Some(42) => println!("eq"),
499+
mac!(any) => println!("neq"),
500+
}
501+
match Some(0) {
502+
mac!(some) => println!("eq"),
503+
_ => println!("neq"),
504+
}
505+
}

tests/ui/single_match.stderr

+19-1
Original file line numberDiff line numberDiff line change
@@ -321,5 +321,23 @@ LL + println!("{u}");
321321
LL + }
322322
|
323323

324-
error: aborting due to 29 previous errors
324+
error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
325+
--> tests/ui/single_match.rs:478:5
326+
|
327+
LL | / match mac!(some) {
328+
LL | | Some(u) => println!("{u}"),
329+
LL | | _ => (),
330+
LL | | }
331+
| |_____^ help: try: `if let Some(u) = mac!(some) { println!("{u}") }`
332+
333+
error: you seem to be trying to use `match` for an equality check. Consider using `if`
334+
--> tests/ui/single_match.rs:486:5
335+
|
336+
LL | / match mac!(str) {
337+
LL | | "foo" => println!("eq"),
338+
LL | | _ => (),
339+
LL | | }
340+
| |_____^ help: try: `if mac!(str) == "foo" { println!("eq") }`
341+
342+
error: aborting due to 31 previous errors
325343

0 commit comments

Comments
 (0)