Skip to content

Commit 8c01600

Browse files
authored
Fix obfuscated_if_else suggestion on left side of a binary expr (#14124)
An `if … { … } else { … }` used as the left operand of a binary expression requires parentheses to be parsed as an expression. Fix #11141 changelog: [`obfuscated_if_else`]: fix bug in suggestion by issuing required parentheses around the left side of a binary expression
2 parents 77344b8 + ac0a11a commit 8c01600

File tree

4 files changed

+130
-6
lines changed

4 files changed

+130
-6
lines changed

clippy_lints/src/methods/obfuscated_if_else.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use super::OBFUSCATED_IF_ELSE;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
4+
use clippy_utils::get_parent_expr;
45
use clippy_utils::source::snippet_with_applicability;
56
use clippy_utils::sugg::Sugg;
67
use rustc_errors::Applicability;
@@ -41,6 +42,17 @@ pub(super) fn check<'tcx>(
4142
snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability)
4243
);
4344

45+
// To be parsed as an expression, the `if { … } else { … }` as the left operand of a binary operator
46+
// requires parentheses.
47+
let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr)
48+
&& let ExprKind::Binary(_, left, _) = parent_expr.kind
49+
&& left.hir_id == expr.hir_id
50+
{
51+
format!("({sugg})")
52+
} else {
53+
sugg
54+
};
55+
4456
span_lint_and_sugg(
4557
cx,
4658
OBFUSCATED_IF_ELSE,

tests/ui/obfuscated_if_else.fixed

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,48 @@
33

44
fn main() {
55
if true { "a" } else { "b" };
6+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
67
if true { "a" } else { "b" };
8+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
79

810
let a = 1;
911
if a == 1 { "a" } else { "b" };
12+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
1013
if a == 1 { "a" } else { "b" };
14+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
1115

1216
let partial = (a == 1).then_some("a");
1317
partial.unwrap_or("b"); // not lint
1418

1519
let mut a = 0;
1620
if true { a += 1 } else { () };
21+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
1722
if true { () } else { a += 2 };
23+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
24+
}
25+
26+
fn issue11141() {
27+
// Parentheses are required around the left side of a binary expression
28+
let _ = (if true { 40 } else { 17 }) | 2;
29+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
30+
31+
// Parentheses are required only for the leftmost expression
32+
let _ = (if true { 30 } else { 17 }) | if true { 2 } else { 3 } | if true { 10 } else { 1 };
33+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
34+
35+
// Parentheses are not required around the right side of a binary expression
36+
let _ = 2 | if true { 40 } else { 17 };
37+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
38+
39+
// Parentheses are not required for a cast
40+
let _ = if true { 42 } else { 17 } as u8;
41+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
42+
43+
// Parentheses are not required for a deref
44+
let _ = *if true { &42 } else { &17 };
45+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
46+
47+
// Parentheses are not required for a deref followed by a cast
48+
let _ = *if true { &42 } else { &17 } as u8;
49+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
1850
}

tests/ui/obfuscated_if_else.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,48 @@
33

44
fn main() {
55
true.then_some("a").unwrap_or("b");
6+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
67
true.then(|| "a").unwrap_or("b");
8+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
79

810
let a = 1;
911
(a == 1).then_some("a").unwrap_or("b");
12+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
1013
(a == 1).then(|| "a").unwrap_or("b");
14+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
1115

1216
let partial = (a == 1).then_some("a");
1317
partial.unwrap_or("b"); // not lint
1418

1519
let mut a = 0;
1620
true.then_some(a += 1).unwrap_or(());
21+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
1722
true.then_some(()).unwrap_or(a += 2);
23+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
24+
}
25+
26+
fn issue11141() {
27+
// Parentheses are required around the left side of a binary expression
28+
let _ = true.then_some(40).unwrap_or(17) | 2;
29+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
30+
31+
// Parentheses are required only for the leftmost expression
32+
let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
33+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
34+
35+
// Parentheses are not required around the right side of a binary expression
36+
let _ = 2 | true.then_some(40).unwrap_or(17);
37+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
38+
39+
// Parentheses are not required for a cast
40+
let _ = true.then_some(42).unwrap_or(17) as u8;
41+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
42+
43+
// Parentheses are not required for a deref
44+
let _ = *true.then_some(&42).unwrap_or(&17);
45+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
46+
47+
// Parentheses are not required for a deref followed by a cast
48+
let _ = *true.then_some(&42).unwrap_or(&17) as u8;
49+
//~^ ERROR: this method chain can be written more clearly with `if .. else ..`
1850
}

tests/ui/obfuscated_if_else.stderr

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,34 +8,82 @@ LL | true.then_some("a").unwrap_or("b");
88
= help: to override `-D warnings` add `#[allow(clippy::obfuscated_if_else)]`
99

1010
error: this method chain can be written more clearly with `if .. else ..`
11-
--> tests/ui/obfuscated_if_else.rs:6:5
11+
--> tests/ui/obfuscated_if_else.rs:7:5
1212
|
1313
LL | true.then(|| "a").unwrap_or("b");
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }`
1515

1616
error: this method chain can be written more clearly with `if .. else ..`
17-
--> tests/ui/obfuscated_if_else.rs:9:5
17+
--> tests/ui/obfuscated_if_else.rs:11:5
1818
|
1919
LL | (a == 1).then_some("a").unwrap_or("b");
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`
2121

2222
error: this method chain can be written more clearly with `if .. else ..`
23-
--> tests/ui/obfuscated_if_else.rs:10:5
23+
--> tests/ui/obfuscated_if_else.rs:13:5
2424
|
2525
LL | (a == 1).then(|| "a").unwrap_or("b");
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`
2727

2828
error: this method chain can be written more clearly with `if .. else ..`
29-
--> tests/ui/obfuscated_if_else.rs:16:5
29+
--> tests/ui/obfuscated_if_else.rs:20:5
3030
|
3131
LL | true.then_some(a += 1).unwrap_or(());
3232
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { a += 1 } else { () }`
3333

3434
error: this method chain can be written more clearly with `if .. else ..`
35-
--> tests/ui/obfuscated_if_else.rs:17:5
35+
--> tests/ui/obfuscated_if_else.rs:22:5
3636
|
3737
LL | true.then_some(()).unwrap_or(a += 2);
3838
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { () } else { a += 2 }`
3939

40-
error: aborting due to 6 previous errors
40+
error: this method chain can be written more clearly with `if .. else ..`
41+
--> tests/ui/obfuscated_if_else.rs:28:13
42+
|
43+
LL | let _ = true.then_some(40).unwrap_or(17) | 2;
44+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 40 } else { 17 })`
45+
46+
error: this method chain can be written more clearly with `if .. else ..`
47+
--> tests/ui/obfuscated_if_else.rs:32:13
48+
|
49+
LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 30 } else { 17 })`
51+
52+
error: this method chain can be written more clearly with `if .. else ..`
53+
--> tests/ui/obfuscated_if_else.rs:32:48
54+
|
55+
LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
56+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 2 } else { 3 }`
57+
58+
error: this method chain can be written more clearly with `if .. else ..`
59+
--> tests/ui/obfuscated_if_else.rs:32:81
60+
|
61+
LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 10 } else { 1 }`
63+
64+
error: this method chain can be written more clearly with `if .. else ..`
65+
--> tests/ui/obfuscated_if_else.rs:36:17
66+
|
67+
LL | let _ = 2 | true.then_some(40).unwrap_or(17);
68+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 40 } else { 17 }`
69+
70+
error: this method chain can be written more clearly with `if .. else ..`
71+
--> tests/ui/obfuscated_if_else.rs:40:13
72+
|
73+
LL | let _ = true.then_some(42).unwrap_or(17) as u8;
74+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 42 } else { 17 }`
75+
76+
error: this method chain can be written more clearly with `if .. else ..`
77+
--> tests/ui/obfuscated_if_else.rs:44:14
78+
|
79+
LL | let _ = *true.then_some(&42).unwrap_or(&17);
80+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }`
81+
82+
error: this method chain can be written more clearly with `if .. else ..`
83+
--> tests/ui/obfuscated_if_else.rs:48:14
84+
|
85+
LL | let _ = *true.then_some(&42).unwrap_or(&17) as u8;
86+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }`
87+
88+
error: aborting due to 14 previous errors
4189

0 commit comments

Comments
 (0)