Auto merge of #12897 - lochetti:manual_unwrap_or_default_result, r=dswij

Lint `manual_unwrap_or_default` for Result as well This PR is modifying the `manual_unwrap_or_default` to be applied/linted for `Result`s as well. It is part of the fixes desired by #12618 changelog:[`manual_unwrap_or_default`]: Lint for Result types.
rust-lang · Jun 7, 2024 · d9ae936 · d9ae936
2 parents 4e7f974 + 478d444
commit d9ae936
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 13 deletions.
diff --git a/clippy_lints/src/manual_unwrap_or_default.rs b/clippy_lints/src/manual_unwrap_or_default.rs
@@ -57,7 +57,8 @@ fn get_some<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<HirId> {
         // Since it comes from a pattern binding, we need to get the parent to actually match
         // against it.
         && let Some(def_id) = cx.tcx.opt_parent(def_id)
-        && cx.tcx.lang_items().get(LangItem::OptionSome) == Some(def_id)
+        && (cx.tcx.lang_items().get(LangItem::OptionSome) == Some(def_id)
+        || cx.tcx.lang_items().get(LangItem::ResultOk) == Some(def_id))
     {
         let mut bindings = Vec::new();
         pat.each_binding(|_, id, _, _| bindings.push(id));
@@ -80,6 +81,14 @@ fn get_none<'tcx>(cx: &LateContext<'tcx>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<
         && cx.tcx.lang_items().get(LangItem::OptionNone) == Some(def_id)
     {
         Some(arm.body)
+    } else if let PatKind::TupleStruct(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
+        // against it.
+        && let Some(def_id) = cx.tcx.opt_parent(def_id)
+        && cx.tcx.lang_items().get(LangItem::ResultErr) == Some(def_id)
+    {
+        Some(arm.body)
     } else if let PatKind::Wild = arm.pat.kind {
         // We consider that the `Some` check will filter it out if it's not right.
         Some(arm.body)

diff --git a/tests/ui/manual_unwrap_or_default.fixed b/tests/ui/manual_unwrap_or_default.fixed
@@ -26,6 +26,12 @@ fn main() {
         Some(x) => x,
         None => &[],
     };
+
+    let x: Result<String, i64> = Ok(String::new());
+    x.unwrap_or_default();
+
+    let x: Result<String, i64> = Ok(String::new());
+    x.unwrap_or_default();
 }
 
 // Issue #12531

diff --git a/tests/ui/manual_unwrap_or_default.rs b/tests/ui/manual_unwrap_or_default.rs
@@ -47,6 +47,21 @@ fn main() {
         Some(x) => x,
         None => &[],
     };
+
+    let x: Result<String, i64> = Ok(String::new());
+    match x {
+        //~^ ERROR: match can be simplified with `.unwrap_or_default()`
+        Ok(v) => v,
+        Err(_) => String::new(),
+    };
+
+    let x: Result<String, i64> = Ok(String::new());
+    if let Ok(v) = x {
+        //~^ ERROR: if let can be simplified with `.unwrap_or_default()`
+        v
+    } else {
+        String::new()
+    };
 }
 
 // Issue #12531

diff --git a/tests/ui/manual_unwrap_or_default.stderr b/tests/ui/manual_unwrap_or_default.stderr
@@ -53,7 +53,28 @@ 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:56:20
+  --> tests/ui/manual_unwrap_or_default.rs:52:5
+   |
+LL | /     match x {
+LL | |
+LL | |         Ok(v) => v,
+LL | |         Err(_) => String::new(),
+LL | |     };
+   | |_____^ help: replace it with: `x.unwrap_or_default()`
+
+error: if let can be simplified with `.unwrap_or_default()`
+  --> tests/ui/manual_unwrap_or_default.rs:59:5
+   |
+LL | /     if let Ok(v) = x {
+LL | |
+LL | |         v
+LL | |     } else {
+LL | |         String::new()
+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:71:20
    |
 LL |           Some(_) => match *b {
    |  ____________________^
@@ -62,5 +83,5 @@ LL | |             _ => 0,
 LL | |         },
    | |_________^ help: replace it with: `(*b).unwrap_or_default()`
 
-error: aborting due to 6 previous errors
+error: aborting due to 8 previous errors
 
diff --git a/tests/ui/useless_conversion_try.rs b/tests/ui/useless_conversion_try.rs
@@ -1,5 +1,9 @@
 #![deny(clippy::useless_conversion)]
-#![allow(clippy::needless_if, clippy::unnecessary_fallible_conversions)]
+#![allow(
+    clippy::needless_if,
+    clippy::unnecessary_fallible_conversions,
+    clippy::manual_unwrap_or_default
+)]
 
 fn test_generic<T: Copy>(val: T) -> T {
     let _ = T::try_from(val).unwrap();

diff --git a/tests/ui/useless_conversion_try.stderr b/tests/ui/useless_conversion_try.stderr
@@ -1,5 +1,5 @@
 error: useless conversion to the same type: `T`
-  --> tests/ui/useless_conversion_try.rs:5:13
+  --> tests/ui/useless_conversion_try.rs:9:13
    |
 LL |     let _ = T::try_from(val).unwrap();
    |             ^^^^^^^^^^^^^^^^
@@ -12,63 +12,63 @@ LL | #![deny(clippy::useless_conversion)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: useless conversion to the same type: `T`
-  --> tests/ui/useless_conversion_try.rs:7:5
+  --> tests/ui/useless_conversion_try.rs:11:5
    |
 LL |     val.try_into().unwrap()
    |     ^^^^^^^^^^^^^^
    |
    = help: consider removing `.try_into()`
 
 error: useless conversion to the same type: `std::string::String`
-  --> tests/ui/useless_conversion_try.rs:30:21
+  --> tests/ui/useless_conversion_try.rs:34:21
    |
 LL |     let _: String = "foo".to_string().try_into().unwrap();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `.try_into()`
 
 error: useless conversion to the same type: `std::string::String`
-  --> tests/ui/useless_conversion_try.rs:32:21
+  --> tests/ui/useless_conversion_try.rs:36:21
    |
 LL |     let _: String = TryFrom::try_from("foo".to_string()).unwrap();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `TryFrom::try_from()`
 
 error: useless conversion to the same type: `std::string::String`
-  --> tests/ui/useless_conversion_try.rs:34:13
+  --> tests/ui/useless_conversion_try.rs:38:13
    |
 LL |     let _ = String::try_from("foo".to_string()).unwrap();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `String::try_from()`
 
 error: useless conversion to the same type: `std::string::String`
-  --> tests/ui/useless_conversion_try.rs:36:13
+  --> tests/ui/useless_conversion_try.rs:40:13
    |
 LL |     let _ = String::try_from(format!("A: {:04}", 123)).unwrap();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `String::try_from()`
 
 error: useless conversion to the same type: `std::string::String`
-  --> tests/ui/useless_conversion_try.rs:38:21
+  --> tests/ui/useless_conversion_try.rs:42:21
    |
 LL |     let _: String = format!("Hello {}", "world").try_into().unwrap();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `.try_into()`
 
 error: useless conversion to the same type: `std::string::String`
-  --> tests/ui/useless_conversion_try.rs:40:21
+  --> tests/ui/useless_conversion_try.rs:44:21
    |
 LL |     let _: String = String::new().try_into().unwrap();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider removing `.try_into()`
 
 error: useless conversion to the same type: `std::string::String`
-  --> tests/ui/useless_conversion_try.rs:42:27
+  --> tests/ui/useless_conversion_try.rs:46:27
    |
 LL |     let _: String = match String::from("_").try_into() {
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^