Skip to content

Commit 56abdc6

Browse files
fix: collapsible_match suggests ref/derefs when needed
Fixes: #14155
1 parent 0dd5c4d commit 56abdc6

File tree

6 files changed

+116
-7
lines changed

6 files changed

+116
-7
lines changed

clippy_lints/src/matches/collapsible_match.rs

+20-4
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_and_then(cx, COLLAPSIBLE_MATCH, 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

+9-2
Original file line numberDiff line numberDiff line change
@@ -1073,7 +1073,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
10731073
significant_drop_in_scrutinee::check_match(cx, expr, ex, arms, source);
10741074
}
10751075

1076-
collapsible_match::check_match(cx, arms, &self.msrv);
1076+
collapsible_match::check_match(cx, ex, arms, &self.msrv);
10771077
if !from_expansion {
10781078
// These don't depend on a relationship between multiple arms
10791079
match_wild_err_arm::check(cx, ex, arms);
@@ -1138,7 +1138,14 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
11381138
match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
11391139
}
11401140
} else if let Some(if_let) = higher::IfLet::hir(cx, expr) {
1141-
collapsible_match::check_if_let(cx, if_let.let_pat, if_let.if_then, if_let.if_else, &self.msrv);
1141+
collapsible_match::check_if_let(
1142+
cx,
1143+
if_let.let_pat,
1144+
if_let.if_then,
1145+
if_let.if_else,
1146+
if_let.let_expr,
1147+
&self.msrv,
1148+
);
11421149
significant_drop_in_scrutinee::check_if_let(cx, expr, if_let.let_expr, if_let.if_then, if_let.if_else);
11431150
if !from_expansion {
11441151
if let Some(else_expr) = if_let.if_else {

clippy_utils/src/lib.rs

+19
Original file line numberDiff line numberDiff line change
@@ -2613,6 +2613,25 @@ pub fn peel_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>
26132613
expr
26142614
}
26152615

2616+
/// Counts `AddrOf` operators (`&`) or deref operators (`*`).
2617+
pub fn count_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>) -> isize {
2618+
let mut count = 0;
2619+
loop {
2620+
match expr.kind {
2621+
ExprKind::AddrOf(_, _, e) => {
2622+
expr = e;
2623+
count += 1;
2624+
},
2625+
ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty(e).is_ref() => {
2626+
expr = e;
2627+
count -= 1;
2628+
},
2629+
_ => break,
2630+
}
2631+
}
2632+
count
2633+
}
2634+
26162635
pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
26172636
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
26182637
if let Res::Def(_, def_id) = path.res {

tests/ui/collapsible_match.rs

+21
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,27 @@ pub fn test_2(x: Issue9647) {
296296
}
297297
}
298298

299+
pub fn issue_14155() {
300+
let arr = ["a", "b", "c"];
301+
if let Some(last) = arr.last() {
302+
match *last {
303+
"a" | "b" => {
304+
unimplemented!()
305+
},
306+
_ => (),
307+
}
308+
}
309+
310+
if let Some(last) = arr.last() {
311+
match &last {
312+
&&"a" | &&"b" => {
313+
unimplemented!()
314+
},
315+
_ => (),
316+
}
317+
}
318+
}
319+
299320
fn make<T>() -> T {
300321
unimplemented!()
301322
}

tests/ui/collapsible_match.stderr

+45-1
Original file line numberDiff line numberDiff line change
@@ -242,5 +242,49 @@ LL | if let Issue9647::A { a: Some(a), .. } = x {
242242
LL | if let Some(u) = a {
243243
| ^^^^^^^ with this pattern
244244

245-
error: aborting due to 13 previous errors
245+
error: this `match` can be collapsed into the outer `if let`
246+
--> tests/ui/collapsible_match.rs:302:9
247+
|
248+
LL | / match *last {
249+
LL | | "a" | "b" => {
250+
LL | | unimplemented!()
251+
LL | | },
252+
LL | | _ => (),
253+
LL | | }
254+
| |_________^
255+
|
256+
help: the outer pattern can be modified to include the inner pattern
257+
--> tests/ui/collapsible_match.rs:301:17
258+
|
259+
LL | if let Some(last) = arr.last() {
260+
| ^^^^ ---------- use: `arr.last().copied()`
261+
| |
262+
| replace this binding
263+
LL | match *last {
264+
LL | "a" | "b" => {
265+
| ^^^^^^^^^ with this pattern
266+
267+
error: this `match` can be collapsed into the outer `if let`
268+
--> tests/ui/collapsible_match.rs:311:9
269+
|
270+
LL | / match &last {
271+
LL | | &&"a" | &&"b" => {
272+
LL | | unimplemented!()
273+
LL | | },
274+
LL | | _ => (),
275+
LL | | }
276+
| |_________^
277+
|
278+
help: the outer pattern can be modified to include the inner pattern
279+
--> tests/ui/collapsible_match.rs:310:17
280+
|
281+
LL | if let Some(last) = arr.last() {
282+
| ^^^^ ---------- use: `arr.last().as_ref()`
283+
| |
284+
| replace this binding
285+
LL | match &last {
286+
LL | &&"a" | &&"b" => {
287+
| ^^^^^^^^^^^^^ with this pattern
288+
289+
error: aborting due to 15 previous errors
246290

tests/ui/collapsible_match2.stderr

+2
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ LL | | },
7676
help: the outer pattern can be modified to include the inner pattern
7777
--> tests/ui/collapsible_match2.rs:53:14
7878
|
79+
LL | match Some(&[1]) {
80+
| ---------- use: `Some(&[1]).copied()`
7981
LL | Some(s) => match *s {
8082
| ^ replace this binding
8183
LL |

0 commit comments

Comments
 (0)