manual_unwrap_or: fix FP edge case (#15812)
changelog: [`manual_unwrap_or`]: fix FP edge case Found this problem while investigating a different bug.
This commit is contained in:
commit
d66e5db0f7
4 changed files with 27 additions and 7 deletions
|
|
@ -32,7 +32,7 @@ fn get_some(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option<HirId> {
|
|||
}
|
||||
}
|
||||
|
||||
fn get_none<'tcx>(cx: &LateContext<'_>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<'tcx>> {
|
||||
fn get_none<'tcx>(cx: &LateContext<'_>, arm: &Arm<'tcx>, allow_wildcard: bool) -> Option<&'tcx Expr<'tcx>> {
|
||||
if let PatKind::Expr(PatExpr { kind: PatExprKind::Path(QPath::Resolved(_, path)), .. }) = arm.pat.kind
|
||||
&& let Some(def_id) = path.res.opt_def_id()
|
||||
// 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
|
|||
&& cx.tcx.lang_items().get(LangItem::ResultErr) == Some(def_id)
|
||||
{
|
||||
Some(arm.body)
|
||||
} else if let PatKind::Wild = arm.pat.kind {
|
||||
} else if let PatKind::Wild = arm.pat.kind
|
||||
&& allow_wildcard
|
||||
{
|
||||
// We consider that the `Some` check will filter it out if it's not right.
|
||||
Some(arm.body)
|
||||
} else {
|
||||
|
|
@ -63,11 +65,11 @@ fn get_some_and_none_bodies<'tcx>(
|
|||
arm2: &'tcx Arm<'tcx>,
|
||||
) -> Option<((&'tcx Expr<'tcx>, HirId), &'tcx Expr<'tcx>)> {
|
||||
if let Some(binding_id) = get_some(cx, arm1.pat)
|
||||
&& let Some(body_none) = get_none(cx, arm2)
|
||||
&& let Some(body_none) = get_none(cx, arm2, true)
|
||||
{
|
||||
Some(((arm1.body, binding_id), body_none))
|
||||
} else if let Some(binding_id) = get_some(cx, arm2.pat)
|
||||
&& let Some(body_none) = get_none(cx, arm1)
|
||||
} else if let Some(body_none) = get_none(cx, arm1, false)
|
||||
&& let Some(binding_id) = get_some(cx, arm2.pat)
|
||||
{
|
||||
Some(((arm2.body, binding_id), body_none))
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -32,6 +32,15 @@ fn main() {
|
|||
|
||||
let x: Result<String, i64> = Ok(String::new());
|
||||
x.unwrap_or_default();
|
||||
|
||||
// edge case
|
||||
// because the `Some(bizarro)` pattern is not actually reachable,
|
||||
// changing this match to `unwrap_or_default` would have side effects
|
||||
let bizarro = Some(String::new());
|
||||
match bizarro {
|
||||
_ => String::new(),
|
||||
Some(bizarro) => bizarro,
|
||||
};
|
||||
}
|
||||
|
||||
// Issue #12531
|
||||
|
|
|
|||
|
|
@ -64,6 +64,15 @@ fn main() {
|
|||
} else {
|
||||
String::new()
|
||||
};
|
||||
|
||||
// edge case
|
||||
// because the `Some(bizarro)` pattern is not actually reachable,
|
||||
// changing this match to `unwrap_or_default` would have side effects
|
||||
let bizarro = Some(String::new());
|
||||
match bizarro {
|
||||
_ => String::new(),
|
||||
Some(bizarro) => bizarro,
|
||||
};
|
||||
}
|
||||
|
||||
// Issue #12531
|
||||
|
|
|
|||
|
|
@ -76,7 +76,7 @@ LL | | };
|
|||
| |_____^ help: replace it with: `x.unwrap_or_default()`
|
||||
|
||||
error: match can be simplified with `.unwrap_or_default()`
|
||||
--> tests/ui/manual_unwrap_or_default.rs:74:24
|
||||
--> tests/ui/manual_unwrap_or_default.rs:83:24
|
||||
|
|
||||
LL | Some(_) => match *b {
|
||||
| ________________________^
|
||||
|
|
@ -87,7 +87,7 @@ LL | | },
|
|||
| |_____________^ help: replace it with: `(*b).unwrap_or_default()`
|
||||
|
||||
error: if let can be simplified with `.unwrap_or_default()`
|
||||
--> tests/ui/manual_unwrap_or_default.rs:143:5
|
||||
--> tests/ui/manual_unwrap_or_default.rs:152:5
|
||||
|
|
||||
LL | / if let Some(x) = Some(42) {
|
||||
LL | |
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue