Skip to content

Commit 087c7c8

Browse files
roifeAlexendoo
roife
authored and
Alexendoo
committed
Add check for same guards in match_same_arms
1 parent 74f611f commit 087c7c8

File tree

4 files changed

+87
-47
lines changed

4 files changed

+87
-47
lines changed

‎clippy_lints/src/casts/cast_possible_truncation.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,7 @@ pub(super) fn check(
131131

132132
let cast_from_ptr_size = def.repr().int.map_or(true, |ty| matches!(ty, IntegerType::Pointer(_),));
133133
let suffix = match (cast_from_ptr_size, is_isize_or_usize(cast_to)) {
134-
(false, false) if from_nbits > to_nbits => "",
135-
(true, false) if from_nbits > to_nbits => "",
134+
(_, false) if from_nbits > to_nbits => "",
136135
(false, true) if from_nbits > 64 => "",
137136
(false, true) if from_nbits > 32 => " on targets with 32-bit wide pointers",
138137
_ => return,

‎clippy_lints/src/matches/match_same_arms.rs‎

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -64,38 +64,50 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
6464
let min_index = usize::min(lindex, rindex);
6565
let max_index = usize::max(lindex, rindex);
6666

67-
let mut local_map: HirIdMap<HirId> = HirIdMap::default();
68-
let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| {
69-
if let Some(a_id) = path_to_local(a)
70-
&& let Some(b_id) = path_to_local(b)
71-
&& let entry = match local_map.entry(a_id) {
72-
HirIdMapEntry::Vacant(entry) => entry,
73-
// check if using the same bindings as before
74-
HirIdMapEntry::Occupied(entry) => return *entry.get() == b_id,
75-
}
67+
let check_eq_with_pat = |expr_a: &Expr<'_>, expr_b: &Expr<'_>| {
68+
let mut local_map: HirIdMap<HirId> = HirIdMap::default();
69+
let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| {
70+
if let Some(a_id) = path_to_local(a)
71+
&& let Some(b_id) = path_to_local(b)
72+
&& let entry = match local_map.entry(a_id) {
73+
HirIdMapEntry::Vacant(entry) => entry,
74+
// check if using the same bindings as before
75+
HirIdMapEntry::Occupied(entry) => return *entry.get() == b_id,
76+
}
7677
// the names technically don't have to match; this makes the lint more conservative
7778
&& cx.tcx.hir().name(a_id) == cx.tcx.hir().name(b_id)
78-
&& cx.typeck_results().expr_ty(a) == cx.typeck_results().expr_ty(b)
79-
&& pat_contains_local(lhs.pat, a_id)
80-
&& pat_contains_local(rhs.pat, b_id)
81-
{
82-
entry.insert(b_id);
83-
true
84-
} else {
85-
false
86-
}
87-
};
88-
// Arms with a guard are ignored, those can’t always be merged together
89-
// If both arms overlap with an arm in between then these can't be merged either.
90-
!(backwards_blocking_idxs[max_index] > min_index && forwards_blocking_idxs[min_index] < max_index)
91-
&& lhs.guard.is_none()
92-
&& rhs.guard.is_none()
93-
&& SpanlessEq::new(cx)
94-
.expr_fallback(eq_fallback)
95-
.eq_expr(lhs.body, rhs.body)
79+
&& cx.typeck_results().expr_ty(a) == cx.typeck_results().expr_ty(b)
80+
&& pat_contains_local(lhs.pat, a_id)
81+
&& pat_contains_local(rhs.pat, b_id)
82+
{
83+
entry.insert(b_id);
84+
true
85+
} else {
86+
false
87+
}
88+
};
89+
90+
SpanlessEq::new(cx)
91+
.expr_fallback(eq_fallback)
92+
.eq_expr(expr_a, expr_b)
9693
// these checks could be removed to allow unused bindings
9794
&& bindings_eq(lhs.pat, local_map.keys().copied().collect())
9895
&& bindings_eq(rhs.pat, local_map.values().copied().collect())
96+
};
97+
98+
let check_same_guard = || match (&lhs.guard, &rhs.guard) {
99+
(None, None) => true,
100+
(Some(lhs_guard), Some(rhs_guard)) => check_eq_with_pat(lhs_guard, rhs_guard),
101+
_ => false,
102+
};
103+
104+
let check_same_body = || check_eq_with_pat(lhs.body, rhs.body);
105+
106+
// Arms with different guard are ignored, those can’t always be merged together
107+
// If both arms overlap with an arm in between then these can't be merged either.
108+
!(backwards_blocking_idxs[max_index] > min_index && forwards_blocking_idxs[min_index] < max_index)
109+
&& check_same_guard()
110+
&& check_same_body()
99111
};
100112

101113
let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect();

‎tests/ui/match_same_arms2.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,20 @@ fn match_same_arms() {
6767
_ => (),
6868
}
6969

70+
// No warning because guards are different
71+
let _ = match Some(42) {
72+
Some(a) if a == 42 => a,
73+
Some(a) if a == 24 => a,
74+
Some(_) => 24,
75+
None => 0,
76+
};
77+
78+
let _ = match (Some(42), Some(42)) {
79+
(Some(a), None) if a == 42 => a,
80+
(None, Some(a)) if a == 42 => a, //~ ERROR: this match arm has an identical body to another arm
81+
_ => 0,
82+
};
83+
7084
match (Some(42), Some(42)) {
7185
(Some(a), ..) => bar(a), //~ ERROR: this match arm has an identical body to another arm
7286
(.., Some(a)) => bar(a),

‎tests/ui/match_same_arms2.stderr‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,22 @@ LL | (Some(a), None) => bar(a),
7171
| ^^^^^^^^^^^^^^^^^^^^^^^^^
7272

7373
error: this match arm has an identical body to another arm
74-
--> tests/ui/match_same_arms2.rs:71:9
74+
--> tests/ui/match_same_arms2.rs:80:9
75+
|
76+
LL | (None, Some(a)) if a == 42 => a,
77+
| ---------------^^^^^^^^^^^^^^^^
78+
| |
79+
| help: try merging the arm patterns: `(None, Some(a)) | (Some(a), None)`
80+
|
81+
= help: or try changing either arm body
82+
note: other arm here
83+
--> tests/ui/match_same_arms2.rs:79:9
84+
|
85+
LL | (Some(a), None) if a == 42 => a,
86+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
87+
88+
error: this match arm has an identical body to another arm
89+
--> tests/ui/match_same_arms2.rs:85:9
7590
|
7691
LL | (Some(a), ..) => bar(a),
7792
| -------------^^^^^^^^^^
@@ -80,13 +95,13 @@ LL | (Some(a), ..) => bar(a),
8095
|
8196
= help: or try changing either arm body
8297
note: other arm here
83-
--> tests/ui/match_same_arms2.rs:72:9
98+
--> tests/ui/match_same_arms2.rs:86:9
8499
|
85100
LL | (.., Some(a)) => bar(a),
86101
| ^^^^^^^^^^^^^^^^^^^^^^^
87102

88103
error: this match arm has an identical body to another arm
89-
--> tests/ui/match_same_arms2.rs:105:9
104+
--> tests/ui/match_same_arms2.rs:119:9
90105
|
91106
LL | (Ok(x), Some(_)) => println!("ok {}", x),
92107
| ----------------^^^^^^^^^^^^^^^^^^^^^^^^
@@ -95,13 +110,13 @@ LL | (Ok(x), Some(_)) => println!("ok {}", x),
95110
|
96111
= help: or try changing either arm body
97112
note: other arm here
98-
--> tests/ui/match_same_arms2.rs:106:9
113+
--> tests/ui/match_same_arms2.rs:120:9
99114
|
100115
LL | (Ok(_), Some(x)) => println!("ok {}", x),
101116
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
102117

103118
error: this match arm has an identical body to another arm
104-
--> tests/ui/match_same_arms2.rs:121:9
119+
--> tests/ui/match_same_arms2.rs:135:9
105120
|
106121
LL | Ok(_) => println!("ok"),
107122
| -----^^^^^^^^^^^^^^^^^^
@@ -110,13 +125,13 @@ LL | Ok(_) => println!("ok"),
110125
|
111126
= help: or try changing either arm body
112127
note: other arm here
113-
--> tests/ui/match_same_arms2.rs:120:9
128+
--> tests/ui/match_same_arms2.rs:134:9
114129
|
115130
LL | Ok(3) => println!("ok"),
116131
| ^^^^^^^^^^^^^^^^^^^^^^^
117132

118133
error: this match arm has an identical body to another arm
119-
--> tests/ui/match_same_arms2.rs:148:9
134+
--> tests/ui/match_same_arms2.rs:162:9
120135
|
121136
LL | 1 => {
122137
| ^ help: try merging the arm patterns: `1 | 0`
@@ -128,15 +143,15 @@ LL | | },
128143
|
129144
= help: or try changing either arm body
130145
note: other arm here
131-
--> tests/ui/match_same_arms2.rs:145:9
146+
--> tests/ui/match_same_arms2.rs:159:9
132147
|
133148
LL | / 0 => {
134149
LL | | empty!(0);
135150
LL | | },
136151
| |_________^
137152

138153
error: match expression looks like `matches!` macro
139-
--> tests/ui/match_same_arms2.rs:167:16
154+
--> tests/ui/match_same_arms2.rs:181:16
140155
|
141156
LL | let _ans = match x {
142157
| ________________^
@@ -150,7 +165,7 @@ LL | | };
150165
= help: to override `-D warnings` add `#[allow(clippy::match_like_matches_macro)]`
151166

152167
error: this match arm has an identical body to another arm
153-
--> tests/ui/match_same_arms2.rs:199:9
168+
--> tests/ui/match_same_arms2.rs:213:9
154169
|
155170
LL | Foo::X(0) => 1,
156171
| ---------^^^^^
@@ -159,13 +174,13 @@ LL | Foo::X(0) => 1,
159174
|
160175
= help: or try changing either arm body
161176
note: other arm here
162-
--> tests/ui/match_same_arms2.rs:201:9
177+
--> tests/ui/match_same_arms2.rs:215:9
163178
|
164179
LL | Foo::Z(_) => 1,
165180
| ^^^^^^^^^^^^^^
166181

167182
error: this match arm has an identical body to another arm
168-
--> tests/ui/match_same_arms2.rs:209:9
183+
--> tests/ui/match_same_arms2.rs:223:9
169184
|
170185
LL | Foo::Z(_) => 1,
171186
| ---------^^^^^
@@ -174,13 +189,13 @@ LL | Foo::Z(_) => 1,
174189
|
175190
= help: or try changing either arm body
176191
note: other arm here
177-
--> tests/ui/match_same_arms2.rs:207:9
192+
--> tests/ui/match_same_arms2.rs:221:9
178193
|
179194
LL | Foo::X(0) => 1,
180195
| ^^^^^^^^^^^^^^
181196

182197
error: this match arm has an identical body to another arm
183-
--> tests/ui/match_same_arms2.rs:232:9
198+
--> tests/ui/match_same_arms2.rs:246:9
184199
|
185200
LL | Some(Bar { y: 0, x: 5, .. }) => 1,
186201
| ----------------------------^^^^^
@@ -189,13 +204,13 @@ LL | Some(Bar { y: 0, x: 5, .. }) => 1,
189204
|
190205
= help: or try changing either arm body
191206
note: other arm here
192-
--> tests/ui/match_same_arms2.rs:229:9
207+
--> tests/ui/match_same_arms2.rs:243:9
193208
|
194209
LL | Some(Bar { x: 0, y: 5, .. }) => 1,
195210
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
196211

197212
error: this match arm has an identical body to another arm
198-
--> tests/ui/match_same_arms2.rs:246:9
213+
--> tests/ui/match_same_arms2.rs:260:9
199214
|
200215
LL | 1 => cfg!(not_enable),
201216
| -^^^^^^^^^^^^^^^^^^^^
@@ -204,10 +219,10 @@ LL | 1 => cfg!(not_enable),
204219
|
205220
= help: or try changing either arm body
206221
note: other arm here
207-
--> tests/ui/match_same_arms2.rs:245:9
222+
--> tests/ui/match_same_arms2.rs:259:9
208223
|
209224
LL | 0 => cfg!(not_enable),
210225
| ^^^^^^^^^^^^^^^^^^^^^
211226

212-
error: aborting due to 13 previous errors
227+
error: aborting due to 14 previous errors
213228

0 commit comments

Comments
 (0)