diff --git a/clippy_lints/src/matches/manual_unwrap_or.rs b/clippy_lints/src/matches/manual_unwrap_or.rs index 8c3f52542d91..d225bd35cb8b 100644 --- a/clippy_lints/src/matches/manual_unwrap_or.rs +++ b/clippy_lints/src/matches/manual_unwrap_or.rs @@ -31,7 +31,7 @@ fn get_some(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option { } } -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 @@ -48,7 +48,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 { @@ -62,11 +64,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 { diff --git a/tests/ui/manual_unwrap_or_default.fixed b/tests/ui/manual_unwrap_or_default.fixed index 189fe876aa5d..11023ac1142a 100644 --- a/tests/ui/manual_unwrap_or_default.fixed +++ b/tests/ui/manual_unwrap_or_default.fixed @@ -32,6 +32,15 @@ fn main() { let x: Result = 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 diff --git a/tests/ui/manual_unwrap_or_default.rs b/tests/ui/manual_unwrap_or_default.rs index ca87926763c9..bf06d9af7d21 100644 --- a/tests/ui/manual_unwrap_or_default.rs +++ b/tests/ui/manual_unwrap_or_default.rs @@ -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 diff --git a/tests/ui/manual_unwrap_or_default.stderr b/tests/ui/manual_unwrap_or_default.stderr index e8f38a2e3899..031100832b16 100644 --- a/tests/ui/manual_unwrap_or_default.stderr +++ b/tests/ui/manual_unwrap_or_default.stderr @@ -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 | |