Skip to content

Commit 22be689

Browse files
lapla-cogitolapla-cogito
committed
extend obfuscated_if_else to support .then().unwrap_or_default() and .then_some().unwrap_or_default() 
1 parent 775a5c2 commit 22be689

File tree

5 files changed

+87
-21
lines changed

5 files changed

+87
-21
lines changed

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2429,7 +2429,7 @@ declare_clippy_lint! {
24292429
///
24302430
/// ### Limitations
24312431
/// This lint currently only looks for usages of
2432-
/// `.then_some(..).unwrap_or(..)` and `.then(..).unwrap_or(..)`, but will be expanded
2432+
/// `.{then, then_some}(..).{unwrap_or, unwrap_or_else, unwrap_or_default}(..)`, but will be expanded
24332433
/// to account for similar patterns.
24342434
///
24352435
/// ### Example
@@ -5421,15 +5421,21 @@ impl Methods {
54215421
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, self.msrv);
54225422
},
54235423
Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
5424-
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method, "unwrap_or");
5424+
obfuscated_if_else::check(cx, expr, t_recv, t_arg, Some(u_arg), then_method, "unwrap_or");
54255425
},
54265426
_ => {},
54275427
}
54285428
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
54295429
},
54305430
("unwrap_or_default", []) => {
5431-
if let Some(("map", m_recv, [arg], span, _)) = method_call(recv) {
5432-
manual_is_variant_and::check(cx, expr, m_recv, arg, span, self.msrv);
5431+
match method_call(recv) {
5432+
Some(("map", m_recv, [arg], span, _)) => {
5433+
manual_is_variant_and::check(cx, expr, m_recv, arg, span, self.msrv);
5434+
},
5435+
Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
5436+
obfuscated_if_else::check(cx, expr, t_recv, t_arg, None, then_method, "unwrap_or_default");
5437+
},
5438+
_ => {},
54335439
}
54345440
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
54355441
},
@@ -5441,7 +5447,15 @@ impl Methods {
54415447
Some(("map", recv, [map_arg], _, _))
54425448
if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, self.msrv) => {},
54435449
Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => {
5444-
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method, "unwrap_or_else");
5450+
obfuscated_if_else::check(
5451+
cx,
5452+
expr,
5453+
t_recv,
5454+
t_arg,
5455+
Some(u_arg),
5456+
then_method,
5457+
"unwrap_or_else",
5458+
);
54455459
},
54465460
_ => {
54475461
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)