Skip to content

Commit 2909b33

Browse files
committed
quick fix of issue#8542 for lint [needless_match]
remove `ref`/`ref mut` check
1 parent b83c632 commit 2909b33

File tree

4 files changed

+257
-151
lines changed

4 files changed

+257
-151
lines changed

clippy_lints/src/matches/needless_match.rs

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use clippy_utils::ty::is_type_diagnostic_item;
55
use clippy_utils::{eq_expr_value, get_parent_expr, higher, is_else_clause, is_lang_ctor, peel_blocks_with_stmt};
66
use rustc_errors::Applicability;
77
use rustc_hir::LangItem::OptionNone;
8-
use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, Path, PathSegment, QPath, UnOp};
8+
use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, Path, PathSegment, QPath};
99
use rustc_lint::LateContext;
1010
use rustc_span::sym;
1111

@@ -21,7 +21,7 @@ pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
2121
if !eq_expr_value(cx, ex, ret_expr) {
2222
return;
2323
}
24-
} else if !pat_same_as_expr(arm.pat, arm.body) {
24+
} else if !pat_same_as_expr(arm.pat, peel_blocks_with_stmt(arm.body)) {
2525
return;
2626
}
2727
}
@@ -92,6 +92,9 @@ fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
9292

9393
if matches!(if_else.kind, ExprKind::Block(..)) {
9494
let else_expr = peel_blocks_with_stmt(if_else);
95+
if matches!(else_expr.kind, ExprKind::Block(..)) {
96+
return false;
97+
}
9598
let ret = strip_return(else_expr);
9699
let let_expr_ty = cx.typeck_results().expr_ty(if_let.let_expr);
97100
if is_type_diagnostic_item(cx, let_expr_ty, sym::Option) {
@@ -120,40 +123,25 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
120123
let expr = strip_return(expr);
121124
match (&pat.kind, &expr.kind) {
122125
// Example: `Some(val) => Some(val)`
123-
(
124-
PatKind::TupleStruct(QPath::Resolved(_, path), [first_pat, ..], _),
125-
ExprKind::Call(call_expr, [first_param, ..]),
126-
) => {
126+
(PatKind::TupleStruct(QPath::Resolved(_, path), tuple_params, _), ExprKind::Call(call_expr, call_params)) => {
127127
if let ExprKind::Path(QPath::Resolved(_, call_path)) = call_expr.kind {
128-
if has_identical_segments(path.segments, call_path.segments)
129-
&& has_same_non_ref_symbol(first_pat, first_param)
130-
{
131-
return true;
132-
}
128+
return has_identical_segments(path.segments, call_path.segments)
129+
&& has_same_non_ref_symbols(tuple_params, call_params);
133130
}
134131
},
135-
// Example: `val => val`, or `ref val => *val`
136-
(PatKind::Binding(annot, _, pat_ident, _), _) => {
137-
let new_expr = if let (
138-
BindingAnnotation::Ref | BindingAnnotation::RefMut,
139-
ExprKind::Unary(UnOp::Deref, operand_expr),
140-
) = (annot, &expr.kind)
141-
{
142-
operand_expr
143-
} else {
144-
expr
145-
};
146-
147-
if let ExprKind::Path(QPath::Resolved(
132+
// Example: `val => val`
133+
(
134+
PatKind::Binding(annot, _, pat_ident, _),
135+
ExprKind::Path(QPath::Resolved(
148136
_,
149137
Path {
150138
segments: [first_seg, ..],
151139
..
152140
},
153-
)) = new_expr.kind
154-
{
155-
return pat_ident.name == first_seg.ident.name;
156-
}
141+
)),
142+
) => {
143+
return !matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut)
144+
&& pat_ident.name == first_seg.ident.name;
157145
},
158146
// Example: `Custom::TypeA => Custom::TypeB`, or `None => None`
159147
(PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => {
@@ -183,15 +171,16 @@ fn has_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegme
183171
true
184172
}
185173

186-
fn has_same_non_ref_symbol(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
187-
if_chain! {
188-
if let PatKind::Binding(annot, _, pat_ident, _) = pat.kind;
189-
if !matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut);
190-
if let ExprKind::Path(QPath::Resolved(_, Path {segments: [first_seg, ..], .. })) = expr.kind;
191-
then {
192-
return pat_ident.name == first_seg.ident.name;
174+
fn has_same_non_ref_symbols(pats: &[Pat<'_>], exprs: &[Expr<'_>]) -> bool {
175+
if pats.len() != exprs.len() {
176+
return false;
177+
}
178+
179+
for i in 0..pats.len() {
180+
if !pat_same_as_expr(&pats[i], &exprs[i]) {
181+
return false;
193182
}
194183
}
195184

196-
false
185+
true
197186
}

tests/ui/needless_match.fixed

Lines changed: 90 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,38 +4,35 @@
44
#![allow(dead_code)]
55

66
#[derive(Clone, Copy)]
7-
enum Choice {
7+
enum Simple {
88
A,
99
B,
1010
C,
1111
D,
1212
}
1313

14-
#[allow(unused_mut)]
1514
fn useless_match() {
16-
let mut i = 10;
15+
let i = 10;
1716
let _: i32 = i;
18-
let _: i32 = i;
19-
let mut _i_mut = i;
20-
2117
let s = "test";
2218
let _: &str = s;
2319
}
2420

25-
fn custom_type_match(se: Choice) {
26-
let _: Choice = se;
21+
fn custom_type_match() {
22+
let se = Simple::A;
23+
let _: Simple = se;
2724
// Don't trigger
28-
let _: Choice = match se {
29-
Choice::A => Choice::A,
30-
Choice::B => Choice::B,
31-
_ => Choice::C,
25+
let _: Simple = match se {
26+
Simple::A => Simple::A,
27+
Simple::B => Simple::B,
28+
_ => Simple::C,
3229
};
3330
// Mingled, don't trigger
34-
let _: Choice = match se {
35-
Choice::A => Choice::B,
36-
Choice::B => Choice::C,
37-
Choice::C => Choice::D,
38-
Choice::D => Choice::A,
31+
let _: Simple = match se {
32+
Simple::A => Simple::B,
33+
Simple::B => Simple::C,
34+
Simple::C => Simple::D,
35+
Simple::D => Simple::A,
3936
};
4037
}
4138

@@ -55,29 +52,96 @@ fn func_ret_err<T>(err: T) -> Result<i32, T> {
5552
fn result_match() {
5653
let _: Result<i32, i32> = Ok(1);
5754
let _: Result<i32, i32> = func_ret_err(0_i32);
55+
// as ref, don't trigger
56+
let res = &func_ret_err(0_i32);
57+
let _: Result<&i32, &i32> = match *res {
58+
Ok(ref x) => Ok(x),
59+
Err(ref x) => Err(x),
60+
};
5861
}
5962

6063
fn if_let_option() -> Option<i32> {
6164
Some(1)
6265
}
6366

64-
fn if_let_result(x: Result<(), i32>) {
65-
let _: Result<(), i32> = x;
66-
let _: Result<(), i32> = x;
67+
fn if_let_result() {
68+
let x: Result<i32, i32> = Ok(1);
69+
let _: Result<i32, i32> = x;
70+
let _: Result<i32, i32> = x;
6771
// Input type mismatch, don't trigger
68-
let _: Result<(), i32> = if let Err(e) = Ok(1) { Err(e) } else { x };
72+
let _: Result<i32, i32> = if let Err(e) = Ok(1) { Err(e) } else { x };
6973
}
7074

71-
fn if_let_custom_enum(x: Choice) {
72-
let _: Choice = x;
75+
fn if_let_custom_enum(x: Simple) {
76+
let _: Simple = x;
77+
7378
// Don't trigger
74-
let _: Choice = if let Choice::A = x {
75-
Choice::A
79+
let _: Simple = if let Simple::A = x {
80+
Simple::A
7681
} else if true {
77-
Choice::B
82+
Simple::B
7883
} else {
7984
x
8085
};
8186
}
8287

88+
mod issue8542 {
89+
#[derive(Clone, Copy)]
90+
enum E {
91+
VariantA(u8, u8),
92+
VariantB(u8, bool),
93+
}
94+
95+
enum Complex {
96+
A(u8),
97+
B(u8, bool),
98+
C(u8, i32, f64),
99+
D(E, bool),
100+
}
101+
102+
fn match_test() {
103+
let ce = Complex::B(8, false);
104+
let aa = 0_u8;
105+
let bb = false;
106+
107+
let _: Complex = ce;
108+
109+
// Don't trigger
110+
let _: Complex = match ce {
111+
Complex::A(_) => Complex::A(aa),
112+
Complex::B(_, b) => Complex::B(aa, b),
113+
Complex::C(_, b, _) => Complex::C(aa, b, 64_f64),
114+
Complex::D(e, b) => Complex::D(e, b),
115+
};
116+
117+
// Don't trigger
118+
let _: Complex = match ce {
119+
Complex::A(a) => Complex::A(a),
120+
Complex::B(a, _) => Complex::B(a, bb),
121+
Complex::C(a, _, _) => Complex::C(a, 32_i32, 64_f64),
122+
_ => ce,
123+
};
124+
}
125+
126+
fn if_let_test() {
127+
fn do_something() {}
128+
129+
// Don't trigger
130+
let _ = if let Some(a) = Some(1) {
131+
Some(a)
132+
} else {
133+
do_something();
134+
None
135+
};
136+
137+
// Don't trigger
138+
let _ = if let Some(a) = Some(1) {
139+
do_something();
140+
Some(a)
141+
} else {
142+
None
143+
};
144+
}
145+
}
146+
83147
fn main() {}

0 commit comments

Comments
 (0)