Skip to content

Commit

Permalink
Fix obfuscated_if_else suggestion on left side of a binary expr
Browse files Browse the repository at this point in the history
An `if … { … } else { … }` used as the left operand of a binary
expression requires parentheses to be parsed as an expression.
  • Loading branch information
samueltardieu committed Jan 31, 2025
1 parent d79f862 commit dd51e96
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 1 deletion.
12 changes: 12 additions & 0 deletions clippy_lints/src/methods/obfuscated_if_else.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::OBFUSCATED_IF_ELSE;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
use clippy_utils::get_parent_expr;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::sugg::Sugg;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -41,6 +42,17 @@ pub(super) fn check<'tcx>(
snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability)
);

// To be parsed as an expression, the `if { … } else { … }` as the left operand of a binary operator
// requires parentheses.
let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr)
&& let ExprKind::Binary(_, left, _) = parent_expr.kind
&& left.hir_id == expr.hir_id
{
format!("({sugg})")
} else {
sugg
};

span_lint_and_sugg(
cx,
OBFUSCATED_IF_ELSE,
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/obfuscated_if_else.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,20 @@ fn main() {
if true { a += 1 } else { () };
if true { () } else { a += 2 };
}

fn issue11141() {
// Parentheses are required around the left side of a binary expression
let _ = (if true { 40 } else { 17 }) | 2;
// Parentheses are required only for the leftmost expression
let _ = (if true { 30 } else { 17 }) | if true { 2 } else { 3 } | if true { 10 } else { 1 };

// Parentheses are not required around the right side of a binary expression
let _ = 2 | if true { 40 } else { 17 };

// Parentheses are not required for a cast
let _ = if true { 42 } else { 17 } as u8;
// Parentheses are not required for a deref
let _ = *if true { &42 } else { &17 };
// Parentheses are not required for a deref followed by a cast
let _ = *if true { &42 } else { &17 } as u8;
}
17 changes: 17 additions & 0 deletions tests/ui/obfuscated_if_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,20 @@ fn main() {
true.then_some(a += 1).unwrap_or(());
true.then_some(()).unwrap_or(a += 2);
}

fn issue11141() {
// Parentheses are required around the left side of a binary expression
let _ = true.then_some(40).unwrap_or(17) | 2;
// Parentheses are required only for the leftmost expression
let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1);

// Parentheses are not required around the right side of a binary expression
let _ = 2 | true.then_some(40).unwrap_or(17);

// Parentheses are not required for a cast
let _ = true.then_some(42).unwrap_or(17) as u8;
// Parentheses are not required for a deref
let _ = *true.then_some(&42).unwrap_or(&17);
// Parentheses are not required for a deref followed by a cast
let _ = *true.then_some(&42).unwrap_or(&17) as u8;
}
50 changes: 49 additions & 1 deletion tests/ui/obfuscated_if_else.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,53 @@ error: this method chain can be written more clearly with `if .. else ..`
LL | true.then_some(()).unwrap_or(a += 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { () } else { a += 2 }`

error: aborting due to 6 previous errors
error: this method chain can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:22:13
|
LL | let _ = true.then_some(40).unwrap_or(17) | 2;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 40 } else { 17 })`

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

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

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

error: this method chain can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:27:17
|
LL | let _ = 2 | true.then_some(40).unwrap_or(17);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 40 } else { 17 }`

error: this method chain can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:30:13
|
LL | let _ = true.then_some(42).unwrap_or(17) as u8;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 42 } else { 17 }`

error: this method chain can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:32:14
|
LL | let _ = *true.then_some(&42).unwrap_or(&17);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }`

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

error: aborting due to 14 previous errors

0 comments on commit dd51e96

Please sign in to comment.