Skip to content

Commit 3b405de

Browse files
committed
fix [manual_unwrap_or_default] suggestion ignoring side-effects
1 parent 124e68b commit 3b405de

File tree

3 files changed

+67
-7
lines changed

3 files changed

+67
-7
lines changed

clippy_lints/src/manual_unwrap_or_default.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use rustc_span::sym;
99
use clippy_utils::diagnostics::span_lint_and_sugg;
1010
use clippy_utils::source::snippet_opt;
1111
use clippy_utils::ty::implements_trait;
12-
use clippy_utils::{in_constant, is_default_equivalent};
12+
use clippy_utils::{in_constant, is_default_equivalent, peel_blocks_with_stmt};
1313

1414
declare_clippy_lint! {
1515
/// ### What it does
@@ -119,11 +119,11 @@ fn handle_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
119119
// We now get the bodies for both the `Some` and `None` arms.
120120
&& let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2)
121121
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
122-
&& let ExprKind::Path(QPath::Resolved(_, path)) = body_some.peel_blocks().kind
122+
&& let ExprKind::Path(QPath::Resolved(_, path)) = peel_blocks_with_stmt(body_some).kind
123123
&& let Res::Local(local_id) = path.res
124124
&& local_id == binding_id
125125
// We now check the `None` arm is calling a method equivalent to `Default::default`.
126-
&& let body_none = body_none.peel_blocks()
126+
&& let body_none = peel_blocks_with_stmt(body_none)
127127
&& is_default_equivalent(cx, body_none)
128128
&& let Some(receiver) = Sugg::hir_opt(cx, match_expr).map(Sugg::maybe_par)
129129
{
@@ -134,7 +134,7 @@ fn handle_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
134134
"match can be simplified with `.unwrap_or_default()`",
135135
"replace it with",
136136
format!("{receiver}.unwrap_or_default()"),
137-
Applicability::MachineApplicable,
137+
Applicability::MaybeIncorrect,
138138
);
139139
}
140140
true
@@ -150,11 +150,11 @@ fn handle_if_let<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
150150
&& implements_trait(cx, match_ty, default_trait_id, &[])
151151
&& let Some(binding_id) = get_some(cx, let_.pat)
152152
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
153-
&& let ExprKind::Path(QPath::Resolved(_, path)) = if_block.peel_blocks().kind
153+
&& let ExprKind::Path(QPath::Resolved(_, path)) = peel_blocks_with_stmt(if_block).kind
154154
&& let Res::Local(local_id) = path.res
155155
&& local_id == binding_id
156156
// We now check the `None` arm is calling a method equivalent to `Default::default`.
157-
&& let body_else = else_expr.peel_blocks()
157+
&& let body_else = peel_blocks_with_stmt(else_expr)
158158
&& is_default_equivalent(cx, body_else)
159159
&& let Some(if_let_expr_snippet) = snippet_opt(cx, let_.init.span)
160160
{
@@ -165,7 +165,7 @@ fn handle_if_let<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
165165
"if let can be simplified with `.unwrap_or_default()`",
166166
"replace it with",
167167
format!("{if_let_expr_snippet}.unwrap_or_default()"),
168-
Applicability::MachineApplicable,
168+
Applicability::MaybeIncorrect,
169169
);
170170
}
171171
}

tests/ui/manual_unwrap_or_default.fixed

+30
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,33 @@ const fn issue_12568(opt: Option<bool>) -> bool {
3333
None => false,
3434
}
3535
}
36+
37+
#[allow(clippy::manual_unwrap_or)]
38+
fn issue_12569() {
39+
let match_none_se = match 1u32.checked_div(0) {
40+
Some(v) => v,
41+
None => {
42+
println!("important");
43+
0
44+
},
45+
};
46+
let match_some_se = match 1u32.checked_div(0) {
47+
Some(v) => {
48+
println!("important");
49+
v
50+
},
51+
None => 0,
52+
};
53+
let iflet_else_se = if let Some(v) = 1u32.checked_div(0) {
54+
v
55+
} else {
56+
println!("important");
57+
0
58+
};
59+
let iflet_then_se = if let Some(v) = 1u32.checked_div(0) {
60+
println!("important");
61+
v
62+
} else {
63+
0
64+
};
65+
}

tests/ui/manual_unwrap_or_default.rs

+30
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,33 @@ const fn issue_12568(opt: Option<bool>) -> bool {
5757
None => false,
5858
}
5959
}
60+
61+
#[allow(clippy::manual_unwrap_or)]
62+
fn issue_12569() {
63+
let match_none_se = match 1u32.checked_div(0) {
64+
Some(v) => v,
65+
None => {
66+
println!("important");
67+
0
68+
},
69+
};
70+
let match_some_se = match 1u32.checked_div(0) {
71+
Some(v) => {
72+
println!("important");
73+
v
74+
},
75+
None => 0,
76+
};
77+
let iflet_else_se = if let Some(v) = 1u32.checked_div(0) {
78+
v
79+
} else {
80+
println!("important");
81+
0
82+
};
83+
let iflet_then_se = if let Some(v) = 1u32.checked_div(0) {
84+
println!("important");
85+
v
86+
} else {
87+
0
88+
};
89+
}

0 commit comments

Comments
 (0)