Skip to content

Commit 1e78abc

Browse files
ManishearthManishearth
authored
expand obfuscated_if_else to support {then(), then_some()}.unwrap_or_default() (rust-lang#14431)
These method chains can be expressed concisely with `if` / `else`. changelog: [`obfuscated_if_else`]: support `then().unwrap_or_default()` and `then_some().unwrap_or_default()`
2 parents b96fecf + e4daa5a commit 1e78abc

File tree

6 files changed

+92
-22
lines changed

6 files changed

+92
-22
lines changed

‎clippy_lints/src/len_zero.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,11 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
202202
expr.span,
203203
lhs_expr,
204204
peel_ref_operators(cx, rhs_expr),
205-
(method.ident.name == sym::ne).then_some("!").unwrap_or_default(),
205+
if method.ident.name == sym::ne {
206+
"!"
207+
} else {
208+
Default::default()
209+
},
206210
);
207211
}
208212

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,7 +2426,7 @@ declare_clippy_lint! {
24262426
///
24272427
/// ### Limitations
24282428
/// This lint currently only looks for usages of
2429-
/// `.then_some(..).unwrap_or(..)` and `.then(..).unwrap_or(..)`, but will be expanded
2429+
/// `.{then, then_some}(..).{unwrap_or, unwrap_or_else, unwrap_or_default}(..)`, but will be expanded
24302430
/// to account for similar patterns.
24312431
///
24322432
/// ### Example
@@ -5419,15 +5419,21 @@ impl Methods {
54195419
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, self.msrv);
54205420
},
54215421
Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
5422-
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method, "unwrap_or");
5422+
obfuscated_if_else::check(cx, expr, t_recv, t_arg, Some(u_arg), then_method, "unwrap_or");
54235423
},
54245424
_ => {},
54255425
}
54265426
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
54275427
},
54285428
("unwrap_or_default", []) => {
5429-
if let Some(("map", m_recv, [arg], span, _)) = method_call(recv) {
5430-
manual_is_variant_and::check(cx, expr, m_recv, arg, span, self.msrv);
5429+
match method_call(recv) {
5430+
Some(("map", m_recv, [arg], span, _)) => {
5431+
manual_is_variant_and::check(cx, expr, m_recv, arg, span, self.msrv);
5432+
},
5433+
Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
5434+
obfuscated_if_else::check(cx, expr, t_recv, t_arg, None, then_method, "unwrap_or_default");
5435+
},
5436+
_ => {},
54315437
}
54325438
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
54335439
},
@@ -5439,7 +5445,15 @@ impl Methods {
54395445
Some(("map", recv, [map_arg], _, _))
54405446
if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, self.msrv) => {},
54415447
Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
5442-
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method, "unwrap_or_else");
5448+
obfuscated_if_else::check(
5449+
cx,
5450+
expr,
5451+
t_recv,
5452+
t_arg,
5453+
Some(u_arg),
5454+
then_method,
5455+
"unwrap_or_else",
5456+
);
54435457
},
54445458
_ => {
54455459
unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or");

‎clippy_lints/src/methods/obfuscated_if_else.rs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,17 @@ pub(super) fn check<'tcx>(
1414
expr: &'tcx hir::Expr<'_>,
1515
then_recv: &'tcx hir::Expr<'_>,
1616
then_arg: &'tcx hir::Expr<'_>,
17-
unwrap_arg: &'tcx hir::Expr<'_>,
17+
unwrap_arg: Option<&'tcx hir::Expr<'_>>,
1818
then_method_name: &str,
1919
unwrap_method_name: &str,
2020
) {
2121
let recv_ty = cx.typeck_results().expr_ty(then_recv);
2222

2323
if recv_ty.is_bool() {
24-
let mut applicability = if switch_to_eager_eval(cx, then_arg) && switch_to_eager_eval(cx, unwrap_arg) {
24+
let then_eager = switch_to_eager_eval(cx, then_arg);
25+
let unwrap_eager = unwrap_arg.is_none_or(|arg| switch_to_eager_eval(cx, arg));
26+
27+
let mut applicability = if then_eager && unwrap_eager {
2528
Applicability::MachineApplicable
2629
} else {
2730
Applicability::MaybeIncorrect
@@ -36,16 +39,17 @@ pub(super) fn check<'tcx>(
3639
_ => return,
3740
};
3841

39-
// FIXME: Add `unwrap_or_else` symbol
42+
// FIXME: Add `unwrap_or_else` and `unwrap_or_default` symbol
4043
let els = match unwrap_method_name {
41-
"unwrap_or" => snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability),
42-
"unwrap_or_else" if let ExprKind::Closure(closure) = unwrap_arg.kind => {
44+
"unwrap_or" => snippet_with_applicability(cx, unwrap_arg.unwrap().span, "..", &mut applicability),
45+
"unwrap_or_else" if let ExprKind::Closure(closure) = unwrap_arg.unwrap().kind => {
4346
let body = cx.tcx.hir_body(closure.body);
4447
snippet_with_applicability(cx, body.value.span, "..", &mut applicability)
4548
},
46-
"unwrap_or_else" if let ExprKind::Path(_) = unwrap_arg.kind => {
47-
snippet_with_applicability(cx, unwrap_arg.span, "_", &mut applicability) + "()"
49+
"unwrap_or_else" if let ExprKind::Path(_) = unwrap_arg.unwrap().kind => {
50+
snippet_with_applicability(cx, unwrap_arg.unwrap().span, "_", &mut applicability) + "()"
4851
},
52+
"unwrap_or_default" => "Default::default()".into(),
4953
_ => return,
5054
};
5155

‎tests/ui/obfuscated_if_else.fixed‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,18 @@ fn main() {
4646

4747
let partial = true.then_some(1);
4848
partial.unwrap_or_else(|| n * 2); // not lint
49+
50+
if true { () } else { Default::default() };
51+
//~^ obfuscated_if_else
52+
53+
if true { () } else { Default::default() };
54+
//~^ obfuscated_if_else
55+
56+
if true { 1 } else { Default::default() };
57+
//~^ obfuscated_if_else
58+
59+
if true { 1 } else { Default::default() };
60+
//~^ obfuscated_if_else
4961
}
5062

5163
fn issue11141() {

‎tests/ui/obfuscated_if_else.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,18 @@ fn main() {
4646

4747
let partial = true.then_some(1);
4848
partial.unwrap_or_else(|| n * 2); // not lint
49+
50+
true.then_some(()).unwrap_or_default();
51+
//~^ obfuscated_if_else
52+
53+
true.then(|| ()).unwrap_or_default();
54+
//~^ obfuscated_if_else
55+
56+
true.then_some(1).unwrap_or_default();
57+
//~^ obfuscated_if_else
58+
59+
true.then(|| 1).unwrap_or_default();
60+
//~^ obfuscated_if_else
4961
}
5062

5163
fn issue11141() {

‎tests/ui/obfuscated_if_else.stderr‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,52 +68,76 @@ LL | true.then_some(1).unwrap_or_else(Default::default);
6868
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { Default::default() }`
6969

7070
error: this method chain can be written more clearly with `if .. else ..`
71-
--> tests/ui/obfuscated_if_else.rs:53:13
71+
--> tests/ui/obfuscated_if_else.rs:50:5
72+
|
73+
LL | true.then_some(()).unwrap_or_default();
74+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { () } else { Default::default() }`
75+
76+
error: this method chain can be written more clearly with `if .. else ..`
77+
--> tests/ui/obfuscated_if_else.rs:53:5
78+
|
79+
LL | true.then(|| ()).unwrap_or_default();
80+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { () } else { Default::default() }`
81+
82+
error: this method chain can be written more clearly with `if .. else ..`
83+
--> tests/ui/obfuscated_if_else.rs:56:5
84+
|
85+
LL | true.then_some(1).unwrap_or_default();
86+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { Default::default() }`
87+
88+
error: this method chain can be written more clearly with `if .. else ..`
89+
--> tests/ui/obfuscated_if_else.rs:59:5
90+
|
91+
LL | true.then(|| 1).unwrap_or_default();
92+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { Default::default() }`
93+
94+
error: this method chain can be written more clearly with `if .. else ..`
95+
--> tests/ui/obfuscated_if_else.rs:65:13
7296
|
7397
LL | let _ = true.then_some(40).unwrap_or(17) | 2;
7498
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 40 } else { 17 })`
7599

76100
error: this method chain can be written more clearly with `if .. else ..`
77-
--> tests/ui/obfuscated_if_else.rs:57:13
101+
--> tests/ui/obfuscated_if_else.rs:69:13
78102
|
79103
LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
80104
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 30 } else { 17 })`
81105

82106
error: this method chain can be written more clearly with `if .. else ..`
83-
--> tests/ui/obfuscated_if_else.rs:57:48
107+
--> tests/ui/obfuscated_if_else.rs:69:48
84108
|
85109
LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
86110
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 2 } else { 3 }`
87111

88112
error: this method chain can be written more clearly with `if .. else ..`
89-
--> tests/ui/obfuscated_if_else.rs:57:81
113+
--> tests/ui/obfuscated_if_else.rs:69:81
90114
|
91115
LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
92116
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 10 } else { 1 }`
93117

94118
error: this method chain can be written more clearly with `if .. else ..`
95-
--> tests/ui/obfuscated_if_else.rs:63:17
119+
--> tests/ui/obfuscated_if_else.rs:75:17
96120
|
97121
LL | let _ = 2 | true.then_some(40).unwrap_or(17);
98122
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 40 } else { 17 }`
99123

100124
error: this method chain can be written more clearly with `if .. else ..`
101-
--> tests/ui/obfuscated_if_else.rs:67:13
125+
--> tests/ui/obfuscated_if_else.rs:79:13
102126
|
103127
LL | let _ = true.then_some(42).unwrap_or(17) as u8;
104128
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 42 } else { 17 }`
105129

106130
error: this method chain can be written more clearly with `if .. else ..`
107-
--> tests/ui/obfuscated_if_else.rs:71:14
131+
--> tests/ui/obfuscated_if_else.rs:83:14
108132
|
109133
LL | let _ = *true.then_some(&42).unwrap_or(&17);
110134
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }`
111135

112136
error: this method chain can be written more clearly with `if .. else ..`
113-
--> tests/ui/obfuscated_if_else.rs:75:14
137+
--> tests/ui/obfuscated_if_else.rs:87:14
114138
|
115139
LL | let _ = *true.then_some(&42).unwrap_or(&17) as u8;
116140
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }`
117141

118-
error: aborting due to 19 previous errors
142+
error: aborting due to 23 previous errors
119143

0 commit comments

Comments
 (0)