Skip to content

Commit bc85de9

Browse files
fix: collapsible_match suggests ref/derefs when needed
Fixes: #14155
1 parent e0e2a93 commit bc85de9

File tree

6 files changed

+120
-7
lines changed

6 files changed

+120
-7
lines changed

clippy_lints/src/matches/collapsible_match.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ use clippy_utils::msrvs::Msrv;
44
use clippy_utils::source::snippet;
55
use clippy_utils::visitors::is_local_used;
66
use clippy_utils::{
7-
SpanlessEq, is_res_lang_ctor, is_unit_expr, path_to_local, peel_blocks_with_stmt, peel_ref_operators,
7+
SpanlessEq, count_ref_operators, is_res_lang_ctor, is_unit_expr, path_to_local, peel_blocks_with_stmt,
8+
peel_ref_operators,
89
};
910
use rustc_errors::MultiSpan;
1011
use rustc_hir::LangItem::OptionNone;
@@ -14,10 +15,10 @@ use rustc_span::Span;
1415

1516
use super::{COLLAPSIBLE_MATCH, pat_contains_disallowed_or};
1617

17-
pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], msrv: Msrv) {
18+
pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>], msrv: Msrv) {
1819
if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
1920
for arm in arms {
20-
check_arm(cx, true, arm.pat, arm.body, arm.guard, Some(els_arm.body), msrv);
21+
check_arm(cx, true, arm.pat, expr, arm.body, arm.guard, Some(els_arm.body), msrv);
2122
}
2223
}
2324
}
@@ -27,15 +28,18 @@ pub(super) fn check_if_let<'tcx>(
2728
pat: &'tcx Pat<'_>,
2829
body: &'tcx Expr<'_>,
2930
else_expr: Option<&'tcx Expr<'_>>,
31+
let_expr: &'tcx Expr<'_>,
3032
msrv: Msrv,
3133
) {
32-
check_arm(cx, false, pat, body, None, else_expr, msrv);
34+
check_arm(cx, false, pat, let_expr, body, None, else_expr, msrv);
3335
}
3436

37+
#[allow(clippy::too_many_arguments)]
3538
fn check_arm<'tcx>(
3639
cx: &LateContext<'tcx>,
3740
outer_is_match: bool,
3841
outer_pat: &'tcx Pat<'tcx>,
42+
outer_cond: &'tcx Expr<'tcx>,
3943
outer_then_body: &'tcx Expr<'tcx>,
4044
outer_guard: Option<&'tcx Expr<'tcx>>,
4145
outer_else_body: Option<&'tcx Expr<'tcx>>,
@@ -99,10 +103,22 @@ fn check_arm<'tcx>(
99103
} else {
100104
String::new()
101105
};
106+
107+
let refs_count = count_ref_operators(cx, inner_scrutinee);
102108
span_lint_hir_and_then(cx, COLLAPSIBLE_MATCH, inner_expr.hir_id, inner_expr.span, msg, |diag| {
103109
let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_then_pat.span]);
104110
help_span.push_span_label(binding_span, "replace this binding");
105111
help_span.push_span_label(inner_then_pat.span, format!("with this pattern{replace_msg}"));
112+
113+
if refs_count != 0 {
114+
let method = if refs_count < 0 { ".copied()" } else { ".as_ref()" };
115+
let outer_cond_msg = format!(
116+
"use: `{}{}`",
117+
snippet(cx, outer_cond.span, ".."),
118+
method.repeat(refs_count.unsigned_abs())
119+
);
120+
help_span.push_span_label(outer_cond.span, outer_cond_msg);
121+
}
106122
diag.span_help(
107123
help_span,
108124
"the outer pattern can be modified to include the inner pattern",

clippy_lints/src/matches/mod.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,7 +1111,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
11111111
significant_drop_in_scrutinee::check_match(cx, expr, ex, arms, source);
11121112
}
11131113

1114-
collapsible_match::check_match(cx, arms, self.msrv);
1114+
collapsible_match::check_match(cx, ex, arms, self.msrv);
11151115
if !from_expansion {
11161116
// These don't depend on a relationship between multiple arms
11171117
match_wild_err_arm::check(cx, ex, arms);
@@ -1176,7 +1176,14 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
11761176
match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
11771177
}
11781178
} else if let Some(if_let) = higher::IfLet::hir(cx, expr) {
1179-
collapsible_match::check_if_let(cx, if_let.let_pat, if_let.if_then, if_let.if_else, self.msrv);
1179+
collapsible_match::check_if_let(
1180+
cx,
1181+
if_let.let_pat,
1182+
if_let.if_then,
1183+
if_let.if_else,
1184+
if_let.let_expr,
1185+
self.msrv,
1186+
);
11801187
significant_drop_in_scrutinee::check_if_let(cx, expr, if_let.let_expr, if_let.if_then, if_let.if_else);
11811188
if !from_expansion {
11821189
if let Some(else_expr) = if_let.if_else {

clippy_utils/src/lib.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2620,6 +2620,25 @@ pub fn peel_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>
26202620
expr
26212621
}
26222622

2623+
/// Counts `AddrOf` operators (`&`) or deref operators (`*`).
2624+
pub fn count_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>) -> isize {
2625+
let mut count = 0;
2626+
loop {
2627+
match expr.kind {
2628+
ExprKind::AddrOf(_, _, e) => {
2629+
expr = e;
2630+
count += 1;
2631+
},
2632+
ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty(e).is_ref() => {
2633+
expr = e;
2634+
count -= 1;
2635+
},
2636+
_ => break,
2637+
}
2638+
}
2639+
count
2640+
}
2641+
26232642
pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
26242643
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind
26252644
&& let Res::Def(_, def_id) = path.res

tests/ui/collapsible_match.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,29 @@ fn lint_emitted_at_right_node(opt: Option<Result<u64, String>>) {
315315
};
316316
}
317317

318+
pub fn issue_14155() {
319+
let arr = ["a", "b", "c"];
320+
if let Some(last) = arr.last() {
321+
match *last {
322+
//~^ collapsible_match
323+
"a" | "b" => {
324+
unimplemented!()
325+
},
326+
_ => (),
327+
}
328+
}
329+
330+
if let Some(last) = arr.last() {
331+
match &last {
332+
//~^ collapsible_match
333+
&&"a" | &&"b" => {
334+
unimplemented!()
335+
},
336+
_ => (),
337+
}
338+
}
339+
}
340+
318341
fn make<T>() -> T {
319342
unimplemented!()
320343
}

tests/ui/collapsible_match.stderr

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,5 +250,51 @@ LL | if let Issue9647::A { a: Some(a), .. } = x {
250250
LL | if let Some(u) = a {
251251
| ^^^^^^^ with this pattern
252252

253-
error: aborting due to 13 previous errors
253+
error: this `match` can be collapsed into the outer `if let`
254+
--> tests/ui/collapsible_match.rs:321:9
255+
|
256+
LL | / match *last {
257+
LL | |
258+
LL | | "a" | "b" => {
259+
LL | | unimplemented!()
260+
LL | | },
261+
LL | | _ => (),
262+
LL | | }
263+
| |_________^
264+
|
265+
help: the outer pattern can be modified to include the inner pattern
266+
--> tests/ui/collapsible_match.rs:320:17
267+
|
268+
LL | if let Some(last) = arr.last() {
269+
| ^^^^ ---------- use: `arr.last().copied()`
270+
| |
271+
| replace this binding
272+
...
273+
LL | "a" | "b" => {
274+
| ^^^^^^^^^ with this pattern
275+
276+
error: this `match` can be collapsed into the outer `if let`
277+
--> tests/ui/collapsible_match.rs:331:9
278+
|
279+
LL | / match &last {
280+
LL | |
281+
LL | | &&"a" | &&"b" => {
282+
LL | | unimplemented!()
283+
LL | | },
284+
LL | | _ => (),
285+
LL | | }
286+
| |_________^
287+
|
288+
help: the outer pattern can be modified to include the inner pattern
289+
--> tests/ui/collapsible_match.rs:330:17
290+
|
291+
LL | if let Some(last) = arr.last() {
292+
| ^^^^ ---------- use: `arr.last().as_ref()`
293+
| |
294+
| replace this binding
295+
...
296+
LL | &&"a" | &&"b" => {
297+
| ^^^^^^^^^^^^^ with this pattern
298+
299+
error: aborting due to 15 previous errors
254300

tests/ui/collapsible_match2.stderr

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ LL | | },
7777
help: the outer pattern can be modified to include the inner pattern
7878
--> tests/ui/collapsible_match2.rs:54:14
7979
|
80+
LL | match Some(&[1]) {
81+
| ---------- use: `Some(&[1]).copied()`
8082
LL | Some(s) => match *s {
8183
| ^ replace this binding
8284
LL |

0 commit comments

Comments
 (0)