Skip to content

Commit 09d3097

Browse files
committed
manual_let_else: do not suggest semantically different replacements
1 parent d880cae commit 09d3097

File tree

3 files changed

+46
-5
lines changed

3 files changed

+46
-5
lines changed

clippy_lints/src/manual_let_else.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,13 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
115115
.enumerate()
116116
.find(|(_, arm)| expr_diverges(cx, arm.body) && pat_allowed_for_else(cx, arm.pat, check_types));
117117
let Some((idx, diverging_arm)) = diverging_arm_opt else { return; };
118+
// If the non-diverging arm is the first one, its pattern can be reused in a let/else statement.
119+
// However, if it arrives in second position, its pattern may cover some cases already covered
120+
// by the diverging one.
121+
// TODO: accept the non-diverging arm as a second position if patterns are disjointed.
122+
if idx == 0 {
123+
return;
124+
}
118125
let pat_arm = &arms[1 - idx];
119126
if !expr_is_simple_identity(pat_arm.pat, pat_arm.body) {
120127
return;

tests/ui/manual_let_else_match.rs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@ fn fire() {
4242
loop {
4343
// More complex pattern for the identity arm and diverging arm
4444
let v = match h() {
45-
(Some(_), Some(_)) | (None, None) => continue,
4645
(Some(v), None) | (None, Some(v)) => v,
46+
(Some(_), Some(_)) | (None, None) => continue,
4747
};
4848
// Custom enums are supported as long as the "else" arm is a simple _
4949
let v = match build_enum() {
50-
_ => continue,
5150
Variant::Bar(v) | Variant::Baz(v) => v,
51+
_ => continue,
5252
};
5353
}
5454

@@ -71,6 +71,12 @@ fn fire() {
7171
Variant::Bar(_) | Variant::Baz(_) => (),
7272
_ => return,
7373
};
74+
75+
let data = [1_u8, 2, 3, 4, 0, 0, 0, 0];
76+
let data = match data.as_slice() {
77+
[data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0] => data,
78+
_ => return,
79+
};
7480
}
7581

7682
fn not_fire() {
@@ -125,4 +131,23 @@ fn not_fire() {
125131
Ok(v) | Err(Variant::Bar(v) | Variant::Baz(v)) => v,
126132
Err(Variant::Foo) => return,
127133
};
134+
135+
// Issue 10241
136+
// The non-divergent arm arrives in second position and
137+
// may cover values already matched in the first arm.
138+
let v = match h() {
139+
(Some(_), Some(_)) | (None, None) => return,
140+
(Some(v), _) | (None, Some(v)) => v,
141+
};
142+
143+
let v = match build_enum() {
144+
_ => return,
145+
Variant::Bar(v) | Variant::Baz(v) => v,
146+
};
147+
148+
let data = [1_u8, 2, 3, 4, 0, 0, 0, 0];
149+
let data = match data.as_slice() {
150+
[] | [0, 0] => return,
151+
[data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ ..] => data,
152+
};
128153
}

tests/ui/manual_let_else_match.stderr

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,17 @@ error: this could be rewritten as `let...else`
2222
--> $DIR/manual_let_else_match.rs:44:9
2323
|
2424
LL | / let v = match h() {
25-
LL | | (Some(_), Some(_)) | (None, None) => continue,
2625
LL | | (Some(v), None) | (None, Some(v)) => v,
26+
LL | | (Some(_), Some(_)) | (None, None) => continue,
2727
LL | | };
2828
| |__________^ help: consider writing: `let ((Some(v), None) | (None, Some(v))) = h() else { continue };`
2929

3030
error: this could be rewritten as `let...else`
3131
--> $DIR/manual_let_else_match.rs:49:9
3232
|
3333
LL | / let v = match build_enum() {
34-
LL | | _ => continue,
3534
LL | | Variant::Bar(v) | Variant::Baz(v) => v,
35+
LL | | _ => continue,
3636
LL | | };
3737
| |__________^ help: consider writing: `let (Variant::Bar(v) | Variant::Baz(v)) = build_enum() else { continue };`
3838

@@ -63,5 +63,14 @@ LL | | _ => return,
6363
LL | | };
6464
| |______^ help: consider writing: `let (Variant::Bar(_) | Variant::Baz(_)) = f else { return };`
6565

66-
error: aborting due to 7 previous errors
66+
error: this could be rewritten as `let...else`
67+
--> $DIR/manual_let_else_match.rs:76:5
68+
|
69+
LL | / let data = match data.as_slice() {
70+
LL | | [data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0] => data,
71+
LL | | _ => return,
72+
LL | | };
73+
| |______^ help: consider writing: `let ([data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0]) = data.as_slice() else { return };`
74+
75+
error: aborting due to 8 previous errors
6776

0 commit comments

Comments
 (0)