about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAlejandra González <blyxyas@gmail.com>2025-02-09 01:22:58 +0000
committerGitHub <noreply@github.com>2025-02-09 01:22:58 +0000
commit8c01600e23f42ddbee2402e4e8e839a37b92ce3b (patch)
tree8ba650c427d956d3c0d2c726eb1d796c709c4c1c
parent77344b8c5868150b126a4fa6a0bb1398644e6524 (diff)
parentac0a11a8bc80befca52af0712e015b4c42ee3769 (diff)
downloadrust-8c01600e23f42ddbee2402e4e8e839a37b92ce3b.tar.gz
rust-8c01600e23f42ddbee2402e4e8e839a37b92ce3b.zip
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
-rw-r--r--clippy_lints/src/methods/obfuscated_if_else.rs12
-rw-r--r--tests/ui/obfuscated_if_else.fixed32
-rw-r--r--tests/ui/obfuscated_if_else.rs32
-rw-r--r--tests/ui/obfuscated_if_else.stderr60
4 files changed, 130 insertions, 6 deletions
diff --git a/clippy_lints/src/methods/obfuscated_if_else.rs b/clippy_lints/src/methods/obfuscated_if_else.rs
index b71f79f8482..2272e03ef26 100644
--- a/clippy_lints/src/methods/obfuscated_if_else.rs
+++ b/clippy_lints/src/methods/obfuscated_if_else.rs
@@ -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;
@@ -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,
diff --git a/tests/ui/obfuscated_if_else.fixed b/tests/ui/obfuscated_if_else.fixed
index bfe1c5e10cf..2cdbee90d52 100644
--- a/tests/ui/obfuscated_if_else.fixed
+++ b/tests/ui/obfuscated_if_else.fixed
@@ -3,16 +3,48 @@
 
 fn main() {
     if true { "a" } else { "b" };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
     if true { "a" } else { "b" };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
 
     let a = 1;
     if a == 1 { "a" } else { "b" };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
     if a == 1 { "a" } else { "b" };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
 
     let partial = (a == 1).then_some("a");
     partial.unwrap_or("b"); // not lint
 
     let mut a = 0;
     if true { a += 1 } else { () };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
     if true { () } else { a += 2 };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+}
+
+fn issue11141() {
+    // Parentheses are required around the left side of a binary expression
+    let _ = (if true { 40 } else { 17 }) | 2;
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are required only for the leftmost expression
+    let _ = (if true { 30 } else { 17 }) | if true { 2 } else { 3 } | if true { 10 } else { 1 };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are not required around the right side of a binary expression
+    let _ = 2 | if true { 40 } else { 17 };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are not required for a cast
+    let _ = if true { 42 } else { 17 } as u8;
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are not required for a deref
+    let _ = *if true { &42 } else { &17 };
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are not required for a deref followed by a cast
+    let _ = *if true { &42 } else { &17 } as u8;
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
 }
diff --git a/tests/ui/obfuscated_if_else.rs b/tests/ui/obfuscated_if_else.rs
index 0ded2a2ceed..20c67e72992 100644
--- a/tests/ui/obfuscated_if_else.rs
+++ b/tests/ui/obfuscated_if_else.rs
@@ -3,16 +3,48 @@
 
 fn main() {
     true.then_some("a").unwrap_or("b");
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
     true.then(|| "a").unwrap_or("b");
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
 
     let a = 1;
     (a == 1).then_some("a").unwrap_or("b");
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
     (a == 1).then(|| "a").unwrap_or("b");
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
 
     let partial = (a == 1).then_some("a");
     partial.unwrap_or("b"); // not lint
 
     let mut a = 0;
     true.then_some(a += 1).unwrap_or(());
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
     true.then_some(()).unwrap_or(a += 2);
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+}
+
+fn issue11141() {
+    // Parentheses are required around the left side of a binary expression
+    let _ = true.then_some(40).unwrap_or(17) | 2;
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // 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);
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are not required around the right side of a binary expression
+    let _ = 2 | true.then_some(40).unwrap_or(17);
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are not required for a cast
+    let _ = true.then_some(42).unwrap_or(17) as u8;
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are not required for a deref
+    let _ = *true.then_some(&42).unwrap_or(&17);
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
+
+    // Parentheses are not required for a deref followed by a cast
+    let _ = *true.then_some(&42).unwrap_or(&17) as u8;
+    //~^ ERROR: this method chain can be written more clearly with `if .. else ..`
 }
diff --git a/tests/ui/obfuscated_if_else.stderr b/tests/ui/obfuscated_if_else.stderr
index 9ce1f475c48..9b1aebb5894 100644
--- a/tests/ui/obfuscated_if_else.stderr
+++ b/tests/ui/obfuscated_if_else.stderr
@@ -8,34 +8,82 @@ LL |     true.then_some("a").unwrap_or("b");
    = help: to override `-D warnings` add `#[allow(clippy::obfuscated_if_else)]`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:6:5
+  --> tests/ui/obfuscated_if_else.rs:7:5
    |
 LL |     true.then(|| "a").unwrap_or("b");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:9:5
+  --> tests/ui/obfuscated_if_else.rs:11:5
    |
 LL |     (a == 1).then_some("a").unwrap_or("b");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:10:5
+  --> tests/ui/obfuscated_if_else.rs:13:5
    |
 LL |     (a == 1).then(|| "a").unwrap_or("b");
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:16:5
+  --> tests/ui/obfuscated_if_else.rs:20:5
    |
 LL |     true.then_some(a += 1).unwrap_or(());
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { a += 1 } else { () }`
 
 error: this method chain can be written more clearly with `if .. else ..`
-  --> tests/ui/obfuscated_if_else.rs:17:5
+  --> tests/ui/obfuscated_if_else.rs:22:5
    |
 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:28: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:32: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:32: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:32: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:36: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:40: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:44: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:48: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