Skip to content

Commit 9cafdbd

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

File tree

3 files changed

+65
-7
lines changed

3 files changed

+65
-7
lines changed

clippy_lints/src/manual_unwrap_or_default.rs

Lines changed: 7 additions & 7 deletions
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};
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(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(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(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(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

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

tests/ui/manual_unwrap_or_default.rs

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

0 commit comments

Comments
 (0)