about summary refs log tree commit diff
diff options
context:
space:
mode:
authorllogiq <bogusandre@gmail.com>2025-02-05 06:05:58 +0000
committerGitHub <noreply@github.com>2025-02-05 06:05:58 +0000
commite6d96412723a9d8187931116df08581de70f0bc9 (patch)
tree5f22a7442e916ba25c06d7276bdf8c30dab4162e
parent75e3a2e905ec17ae430a936d620862c598b022bb (diff)
parent270e52f6da05f40ee320ea1b0f1a914d2463babf (diff)
downloadrust-e6d96412723a9d8187931116df08581de70f0bc9.tar.gz
rust-e6d96412723a9d8187931116df08581de70f0bc9.zip
fix: `manual_unwrap_or_default` suggests falsely when condition type is uncertain (#13889)
fixes #12670

Continuation of #12688. r? @Jarcho if you don't mind?

changelog: [`manual_unwrap_or_default`] fix wrong suggestions when
condition type is uncertain
-rw-r--r--clippy_lints/src/manual_unwrap_or_default.rs34
-rw-r--r--tests/ui/manual_unwrap_or_default_unfixable.rs15
-rw-r--r--tests/ui/manual_unwrap_or_default_unfixable.stderr34
3 files changed, 81 insertions, 2 deletions
diff --git a/clippy_lints/src/manual_unwrap_or_default.rs b/clippy_lints/src/manual_unwrap_or_default.rs
index 8f8390b6f3f..8ad2ef96cfc 100644
--- a/clippy_lints/src/manual_unwrap_or_default.rs
+++ b/clippy_lints/src/manual_unwrap_or_default.rs
@@ -9,8 +9,8 @@ use rustc_span::sym;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::higher::IfLetOrMatch;
 use clippy_utils::sugg::Sugg;
-use clippy_utils::ty::implements_trait;
-use clippy_utils::{is_default_equivalent, is_in_const_context, peel_blocks, span_contains_comment};
+use clippy_utils::ty::{expr_type_is_certain, implements_trait};
+use clippy_utils::{is_default_equivalent, is_in_const_context, path_res, peel_blocks, span_contains_comment};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -158,6 +158,36 @@ fn handle<'tcx>(cx: &LateContext<'tcx>, if_let_or_match: IfLetOrMatch<'tcx>, exp
         } else {
             Applicability::MachineApplicable
         };
+
+        // We now check if the condition is a None variant, in which case we need to specify the type
+        if path_res(cx, condition)
+            .opt_def_id()
+            .is_some_and(|id| Some(cx.tcx.parent(id)) == cx.tcx.lang_items().option_none_variant())
+        {
+            return span_lint_and_sugg(
+                cx,
+                MANUAL_UNWRAP_OR_DEFAULT,
+                expr.span,
+                format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
+                "replace it with",
+                format!("{receiver}::<{expr_type}>.unwrap_or_default()"),
+                applicability,
+            );
+        }
+
+        // We check if the expression type is still uncertain, in which case we ask the user to specify it
+        if !expr_type_is_certain(cx, condition) {
+            return span_lint_and_sugg(
+                cx,
+                MANUAL_UNWRAP_OR_DEFAULT,
+                expr.span,
+                format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
+                format!("ascribe the type {expr_type} and replace your expression with"),
+                format!("{receiver}.unwrap_or_default()"),
+                Applicability::Unspecified,
+            );
+        }
+
         span_lint_and_sugg(
             cx,
             MANUAL_UNWRAP_OR_DEFAULT,
diff --git a/tests/ui/manual_unwrap_or_default_unfixable.rs b/tests/ui/manual_unwrap_or_default_unfixable.rs
new file mode 100644
index 00000000000..acc54b52816
--- /dev/null
+++ b/tests/ui/manual_unwrap_or_default_unfixable.rs
@@ -0,0 +1,15 @@
+//@no-rustfix
+fn issue_12670() {
+    // no auto: type not found
+    #[allow(clippy::match_result_ok)]
+    let _ = if let Some(x) = "1".parse().ok() {
+        x
+    } else {
+        i32::default()
+    };
+    let _ = if let Some(x) = None { x } else { i32::default() };
+    // auto fix with unwrap_or_default
+    let a: Option<i32> = None;
+    let _ = if let Some(x) = a { x } else { i32::default() };
+    let _ = if let Some(x) = Some(99) { x } else { i32::default() };
+}
diff --git a/tests/ui/manual_unwrap_or_default_unfixable.stderr b/tests/ui/manual_unwrap_or_default_unfixable.stderr
new file mode 100644
index 00000000000..3849d33cf25
--- /dev/null
+++ b/tests/ui/manual_unwrap_or_default_unfixable.stderr
@@ -0,0 +1,34 @@
+error: if let can be simplified with `.unwrap_or_default()`
+  --> tests/ui/manual_unwrap_or_default_unfixable.rs:5:13
+   |
+LL |       let _ = if let Some(x) = "1".parse().ok() {
+   |  _____________^
+LL | |         x
+LL | |     } else {
+LL | |         i32::default()
+LL | |     };
+   | |_____^ help: ascribe the type i32 and replace your expression with: `"1".parse().ok().unwrap_or_default()`
+   |
+   = note: `-D clippy::manual-unwrap-or-default` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_unwrap_or_default)]`
+
+error: if let can be simplified with `.unwrap_or_default()`
+  --> tests/ui/manual_unwrap_or_default_unfixable.rs:10:13
+   |
+LL |     let _ = if let Some(x) = None { x } else { i32::default() };
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `None::<i32>.unwrap_or_default()`
+
+error: if let can be simplified with `.unwrap_or_default()`
+  --> tests/ui/manual_unwrap_or_default_unfixable.rs:13:13
+   |
+LL |     let _ = if let Some(x) = a { x } else { i32::default() };
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.unwrap_or_default()`
+
+error: if let can be simplified with `.unwrap_or_default()`
+  --> tests/ui/manual_unwrap_or_default_unfixable.rs:14:13
+   |
+LL |     let _ = if let Some(x) = Some(99) { x } else { i32::default() };
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(99).unwrap_or_default()`
+
+error: aborting due to 4 previous errors
+