manual_unwrap_or: fix FP edge case

Author: notriddle

clippy_lints/src/matches/manual_unwrap_or.rs

@@ -31,7 +31,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
@@ -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 {

tests/ui/manual_unwrap_or_default.fixed

@@ -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

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

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 | |