Skip to content

Commit d66e5db

Browse files
authored
manual_unwrap_or: fix FP edge case (rust-lang#15812)
changelog: [`manual_unwrap_or`]: fix FP edge case Found this problem while investigating a different bug.
2 parents 7c7bd6e + e0e5d47 commit d66e5db

File tree

4 files changed

+27
-7
lines changed

4 files changed

+27
-7
lines changed

clippy_lints/src/matches/manual_unwrap_or.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ fn get_some(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option<HirId> {
3232
}
3333
}
3434

35-
fn get_none<'tcx>(cx: &LateContext<'_>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<'tcx>> {
35+
fn get_none<'tcx>(cx: &LateContext<'_>, arm: &Arm<'tcx>, allow_wildcard: bool) -> Option<&'tcx Expr<'tcx>> {
3636
if let PatKind::Expr(PatExpr { kind: PatExprKind::Path(QPath::Resolved(_, path)), .. }) = arm.pat.kind
3737
&& let Some(def_id) = path.res.opt_def_id()
3838
// Since it comes from a pattern binding, we need to get the parent to actually match
@@ -49,7 +49,9 @@ fn get_none<'tcx>(cx: &LateContext<'_>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<'t
4949
&& cx.tcx.lang_items().get(LangItem::ResultErr) == Some(def_id)
5050
{
5151
Some(arm.body)
52-
} else if let PatKind::Wild = arm.pat.kind {
52+
} else if let PatKind::Wild = arm.pat.kind
53+
&& allow_wildcard
54+
{
5355
// We consider that the `Some` check will filter it out if it's not right.
5456
Some(arm.body)
5557
} else {
@@ -63,11 +65,11 @@ fn get_some_and_none_bodies<'tcx>(
6365
arm2: &'tcx Arm<'tcx>,
6466
) -> Option<((&'tcx Expr<'tcx>, HirId), &'tcx Expr<'tcx>)> {
6567
if let Some(binding_id) = get_some(cx, arm1.pat)
66-
&& let Some(body_none) = get_none(cx, arm2)
68+
&& let Some(body_none) = get_none(cx, arm2, true)
6769
{
6870
Some(((arm1.body, binding_id), body_none))
69-
} else if let Some(binding_id) = get_some(cx, arm2.pat)
70-
&& let Some(body_none) = get_none(cx, arm1)
71+
} else if let Some(body_none) = get_none(cx, arm1, false)
72+
&& let Some(binding_id) = get_some(cx, arm2.pat)
7173
{
7274
Some(((arm2.body, binding_id), body_none))
7375
} else {

tests/ui/manual_unwrap_or_default.fixed

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ fn main() {
3232

3333
let x: Result<String, i64> = Ok(String::new());
3434
x.unwrap_or_default();
35+
36+
// edge case
37+
// because the `Some(bizarro)` pattern is not actually reachable,
38+
// changing this match to `unwrap_or_default` would have side effects
39+
let bizarro = Some(String::new());
40+
match bizarro {
41+
_ => String::new(),
42+
Some(bizarro) => bizarro,
43+
};
3544
}
3645

3746
// Issue #12531

tests/ui/manual_unwrap_or_default.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ fn main() {
6464
} else {
6565
String::new()
6666
};
67+
68+
// edge case
69+
// because the `Some(bizarro)` pattern is not actually reachable,
70+
// changing this match to `unwrap_or_default` would have side effects
71+
let bizarro = Some(String::new());
72+
match bizarro {
73+
_ => String::new(),
74+
Some(bizarro) => bizarro,
75+
};
6776
}
6877

6978
// Issue #12531

tests/ui/manual_unwrap_or_default.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ LL | | };
7676
| |_____^ help: replace it with: `x.unwrap_or_default()`
7777

7878
error: match can be simplified with `.unwrap_or_default()`
79-
--> tests/ui/manual_unwrap_or_default.rs:74:24
79+
--> tests/ui/manual_unwrap_or_default.rs:83:24
8080
|
8181
LL | Some(_) => match *b {
8282
| ________________________^
@@ -87,7 +87,7 @@ LL | | },
8787
| |_____________^ help: replace it with: `(*b).unwrap_or_default()`
8888

8989
error: if let can be simplified with `.unwrap_or_default()`
90-
--> tests/ui/manual_unwrap_or_default.rs:143:5
90+
--> tests/ui/manual_unwrap_or_default.rs:152:5
9191
|
9292
LL | / if let Some(x) = Some(42) {
9393
LL | |

0 commit comments

Comments
 (0)