Skip to content

Commit e6d9641

Browse files
authored
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
2 parents 75e3a2e + 270e52f commit e6d9641

File tree

3 files changed

+81
-2
lines changed

3 files changed

+81
-2
lines changed

clippy_lints/src/manual_unwrap_or_default.rs

+32-2
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ use rustc_span::sym;
99
use clippy_utils::diagnostics::span_lint_and_sugg;
1010
use clippy_utils::higher::IfLetOrMatch;
1111
use clippy_utils::sugg::Sugg;
12-
use clippy_utils::ty::implements_trait;
13-
use clippy_utils::{is_default_equivalent, is_in_const_context, peel_blocks, span_contains_comment};
12+
use clippy_utils::ty::{expr_type_is_certain, implements_trait};
13+
use clippy_utils::{is_default_equivalent, is_in_const_context, path_res, peel_blocks, span_contains_comment};
1414

1515
declare_clippy_lint! {
1616
/// ### What it does
@@ -158,6 +158,36 @@ fn handle<'tcx>(cx: &LateContext<'tcx>, if_let_or_match: IfLetOrMatch<'tcx>, exp
158158
} else {
159159
Applicability::MachineApplicable
160160
};
161+
162+
// We now check if the condition is a None variant, in which case we need to specify the type
163+
if path_res(cx, condition)
164+
.opt_def_id()
165+
.is_some_and(|id| Some(cx.tcx.parent(id)) == cx.tcx.lang_items().option_none_variant())
166+
{
167+
return span_lint_and_sugg(
168+
cx,
169+
MANUAL_UNWRAP_OR_DEFAULT,
170+
expr.span,
171+
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
172+
"replace it with",
173+
format!("{receiver}::<{expr_type}>.unwrap_or_default()"),
174+
applicability,
175+
);
176+
}
177+
178+
// We check if the expression type is still uncertain, in which case we ask the user to specify it
179+
if !expr_type_is_certain(cx, condition) {
180+
return span_lint_and_sugg(
181+
cx,
182+
MANUAL_UNWRAP_OR_DEFAULT,
183+
expr.span,
184+
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
185+
format!("ascribe the type {expr_type} and replace your expression with"),
186+
format!("{receiver}.unwrap_or_default()"),
187+
Applicability::Unspecified,
188+
);
189+
}
190+
161191
span_lint_and_sugg(
162192
cx,
163193
MANUAL_UNWRAP_OR_DEFAULT,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//@no-rustfix
2+
fn issue_12670() {
3+
// no auto: type not found
4+
#[allow(clippy::match_result_ok)]
5+
let _ = if let Some(x) = "1".parse().ok() {
6+
x
7+
} else {
8+
i32::default()
9+
};
10+
let _ = if let Some(x) = None { x } else { i32::default() };
11+
// auto fix with unwrap_or_default
12+
let a: Option<i32> = None;
13+
let _ = if let Some(x) = a { x } else { i32::default() };
14+
let _ = if let Some(x) = Some(99) { x } else { i32::default() };
15+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
error: if let can be simplified with `.unwrap_or_default()`
2+
--> tests/ui/manual_unwrap_or_default_unfixable.rs:5:13
3+
|
4+
LL | let _ = if let Some(x) = "1".parse().ok() {
5+
| _____________^
6+
LL | | x
7+
LL | | } else {
8+
LL | | i32::default()
9+
LL | | };
10+
| |_____^ help: ascribe the type i32 and replace your expression with: `"1".parse().ok().unwrap_or_default()`
11+
|
12+
= note: `-D clippy::manual-unwrap-or-default` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::manual_unwrap_or_default)]`
14+
15+
error: if let can be simplified with `.unwrap_or_default()`
16+
--> tests/ui/manual_unwrap_or_default_unfixable.rs:10:13
17+
|
18+
LL | let _ = if let Some(x) = None { x } else { i32::default() };
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `None::<i32>.unwrap_or_default()`
20+
21+
error: if let can be simplified with `.unwrap_or_default()`
22+
--> tests/ui/manual_unwrap_or_default_unfixable.rs:13:13
23+
|
24+
LL | let _ = if let Some(x) = a { x } else { i32::default() };
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.unwrap_or_default()`
26+
27+
error: if let can be simplified with `.unwrap_or_default()`
28+
--> tests/ui/manual_unwrap_or_default_unfixable.rs:14:13
29+
|
30+
LL | let _ = if let Some(x) = Some(99) { x } else { i32::default() };
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(99).unwrap_or_default()`
32+
33+
error: aborting due to 4 previous errors
34+

0 commit comments

Comments
 (0)