about summary refs log tree commit diff
diff options
context:
space:
mode:
authorllogiq <bogusandre@gmail.com>2025-01-22 17:35:33 +0000
committerGitHub <noreply@github.com>2025-01-22 17:35:33 +0000
commit106db265f42cef77ebb0351cfa34a379428f82b6 (patch)
tree2e206f35a6ccd09e54eaaeba1c4f42d3cb9a1ca8
parentc024c1327bfd0540a0c85e62df1f1527f05fe03f (diff)
parent0c3deeb24693cfa295701215023f87852bf34cc0 (diff)
downloadrust-106db265f42cef77ebb0351cfa34a379428f82b6.tar.gz
rust-106db265f42cef77ebb0351cfa34a379428f82b6.zip
`match_bool`: fix suggestion if guard is present (#14039)
Without this check, the lint would suggest that

```rust
    match test {
        true if option == 5 => 10,
        _ => 1,
    };
```

is replaced by `if test { 10 } else { 1 }`.

changelog: [`match_bool`]: omit suggestion when guards are present in
`match` expression
-rw-r--r--clippy_lints/src/matches/match_bool.rs51
-rw-r--r--tests/ui/match_bool.fixed58
-rw-r--r--tests/ui/match_bool.rs44
-rw-r--r--tests/ui/match_bool.stderr96
4 files changed, 195 insertions, 54 deletions
diff --git a/clippy_lints/src/matches/match_bool.rs b/clippy_lints/src/matches/match_bool.rs
index 69105ff0d5c..02e12eb53fc 100644
--- a/clippy_lints/src/matches/match_bool.rs
+++ b/clippy_lints/src/matches/match_bool.rs
@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::is_unit_expr;
-use clippy_utils::source::{expr_block, snippet};
+use clippy_utils::source::expr_block;
 use clippy_utils::sugg::Sugg;
 use rustc_ast::LitKind;
 use rustc_errors::Applicability;
@@ -20,14 +20,25 @@ pub(crate) fn check(cx: &LateContext<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]
             "you seem to be trying to match on a boolean expression",
             move |diag| {
                 if arms.len() == 2 {
-                    // no guards
-                    let exprs = if let PatKind::Lit(arm_bool) = arms[0].pat.kind {
+                    let mut app = Applicability::MachineApplicable;
+                    let test_sugg = if let PatKind::Lit(arm_bool) = arms[0].pat.kind {
+                        let test = Sugg::hir_with_applicability(cx, scrutinee, "_", &mut app);
                         if let ExprKind::Lit(lit) = arm_bool.kind {
-                            match lit.node {
-                                LitKind::Bool(true) => Some((arms[0].body, arms[1].body)),
-                                LitKind::Bool(false) => Some((arms[1].body, arms[0].body)),
+                            match &lit.node {
+                                LitKind::Bool(true) => Some(test),
+                                LitKind::Bool(false) => Some(!test),
                                 _ => None,
                             }
+                            .map(|test| {
+                                if let Some(guard) = &arms[0]
+                                    .guard
+                                    .map(|g| Sugg::hir_with_applicability(cx, g, "_", &mut app))
+                                {
+                                    test.and(guard)
+                                } else {
+                                    test
+                                }
+                            })
                         } else {
                             None
                         }
@@ -35,39 +46,31 @@ pub(crate) fn check(cx: &LateContext<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]
                         None
                     };
 
-                    if let Some((true_expr, false_expr)) = exprs {
-                        let mut app = Applicability::HasPlaceholders;
+                    if let Some(test_sugg) = test_sugg {
                         let ctxt = expr.span.ctxt();
+                        let (true_expr, false_expr) = (arms[0].body, arms[1].body);
                         let sugg = match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
                             (false, false) => Some(format!(
                                 "if {} {} else {}",
-                                snippet(cx, scrutinee.span, "b"),
+                                test_sugg,
                                 expr_block(cx, true_expr, ctxt, "..", Some(expr.span), &mut app),
                                 expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
                             )),
                             (false, true) => Some(format!(
                                 "if {} {}",
-                                snippet(cx, scrutinee.span, "b"),
+                                test_sugg,
                                 expr_block(cx, true_expr, ctxt, "..", Some(expr.span), &mut app)
                             )),
-                            (true, false) => {
-                                let test = Sugg::hir(cx, scrutinee, "..");
-                                Some(format!(
-                                    "if {} {}",
-                                    !test,
-                                    expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
-                                ))
-                            },
+                            (true, false) => Some(format!(
+                                "if {} {}",
+                                !test_sugg,
+                                expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
+                            )),
                             (true, true) => None,
                         };
 
                         if let Some(sugg) = sugg {
-                            diag.span_suggestion(
-                                expr.span,
-                                "consider using an `if`/`else` expression",
-                                sugg,
-                                Applicability::HasPlaceholders,
-                            );
+                            diag.span_suggestion(expr.span, "consider using an `if`/`else` expression", sugg, app);
                         }
                     }
                 }
diff --git a/tests/ui/match_bool.fixed b/tests/ui/match_bool.fixed
new file mode 100644
index 00000000000..61a8e54fa10
--- /dev/null
+++ b/tests/ui/match_bool.fixed
@@ -0,0 +1,58 @@
+#![deny(clippy::match_bool)]
+#![allow(clippy::nonminimal_bool, clippy::eq_op)]
+
+fn match_bool() {
+    let test: bool = true;
+
+    if test { 0 } else { 42 };
+
+    let option = 1;
+    if option == 1 { 1 } else { 0 };
+
+    if !test {
+        println!("Noooo!");
+    };
+
+    if !test {
+        println!("Noooo!");
+    };
+
+    if !(test && test) {
+        println!("Noooo!");
+    };
+
+    if !test {
+        println!("Noooo!");
+    } else {
+        println!("Yes!");
+    };
+
+    // Not linted
+    match option {
+        1..=10 => 1,
+        11..=20 => 2,
+        _ => 3,
+    };
+
+    // Don't lint
+    let _ = match test {
+        #[cfg(feature = "foo")]
+        true if option == 5 => 10,
+        true => 0,
+        false => 1,
+    };
+
+    let _ = if test && option == 5 { 10 } else { 1 };
+
+    let _ = if !test && option == 5 { 10 } else { 1 };
+
+    if test && option == 5 { println!("Hello") };
+
+    if !(test && option == 5) { println!("Hello") };
+
+    if !test && option == 5 { println!("Hello") };
+
+    if !(!test && option == 5) { println!("Hello") };
+}
+
+fn main() {}
diff --git a/tests/ui/match_bool.rs b/tests/ui/match_bool.rs
index f84af393e47..4952a225c24 100644
--- a/tests/ui/match_bool.rs
+++ b/tests/ui/match_bool.rs
@@ -1,5 +1,5 @@
-//@no-rustfix: overlapping suggestions
 #![deny(clippy::match_bool)]
+#![allow(clippy::nonminimal_bool, clippy::eq_op)]
 
 fn match_bool() {
     let test: bool = true;
@@ -34,11 +34,7 @@ fn match_bool() {
     };
 
     match test && test {
-        //~^ ERROR: this boolean expression can be simplified
-        //~| NOTE: `-D clippy::nonminimal-bool` implied by `-D warnings`
-        //~| ERROR: you seem to be trying to match on a boolean expression
-        //~| ERROR: equal expressions as operands to `&&`
-        //~| NOTE: `#[deny(clippy::eq_op)]` on by default
+        //~^ ERROR: you seem to be trying to match on a boolean expression
         false => {
             println!("Noooo!");
         },
@@ -69,6 +65,42 @@ fn match_bool() {
         true => 0,
         false => 1,
     };
+
+    let _ = match test {
+        //~^ ERROR: you seem to be trying to match on a boolean expression
+        true if option == 5 => 10,
+        _ => 1,
+    };
+
+    let _ = match test {
+        //~^ ERROR: you seem to be trying to match on a boolean expression
+        false if option == 5 => 10,
+        _ => 1,
+    };
+
+    match test {
+        //~^ ERROR: you seem to be trying to match on a boolean expression
+        true if option == 5 => println!("Hello"),
+        _ => (),
+    };
+
+    match test {
+        //~^ ERROR: you seem to be trying to match on a boolean expression
+        true if option == 5 => (),
+        _ => println!("Hello"),
+    };
+
+    match test {
+        //~^ ERROR: you seem to be trying to match on a boolean expression
+        false if option == 5 => println!("Hello"),
+        _ => (),
+    };
+
+    match test {
+        //~^ ERROR: you seem to be trying to match on a boolean expression
+        false if option == 5 => (),
+        _ => println!("Hello"),
+    };
 }
 
 fn main() {}
diff --git a/tests/ui/match_bool.stderr b/tests/ui/match_bool.stderr
index fb24e67ecee..f76c79cd7f7 100644
--- a/tests/ui/match_bool.stderr
+++ b/tests/ui/match_bool.stderr
@@ -1,12 +1,3 @@
-error: this boolean expression can be simplified
-  --> tests/ui/match_bool.rs:36:11
-   |
-LL |     match test && test {
-   |           ^^^^^^^^^^^^ help: try: `test`
-   |
-   = note: `-D clippy::nonminimal-bool` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]`
-
 error: you seem to be trying to match on a boolean expression
   --> tests/ui/match_bool.rs:7:5
    |
@@ -18,7 +9,7 @@ LL | |     };
    | |_____^ help: consider using an `if`/`else` expression: `if test { 0 } else { 42 }`
    |
 note: the lint level is defined here
-  --> tests/ui/match_bool.rs:2:9
+  --> tests/ui/match_bool.rs:1:9
    |
 LL | #![deny(clippy::match_bool)]
    |         ^^^^^^^^^^^^^^^^^^
@@ -75,7 +66,10 @@ error: you seem to be trying to match on a boolean expression
   --> tests/ui/match_bool.rs:36:5
    |
 LL | /     match test && test {
-...  |
+LL | |
+LL | |         false => {
+LL | |             println!("Noooo!");
+LL | |         },
 LL | |         _ => (),
 LL | |     };
    | |_____^
@@ -87,16 +81,8 @@ LL +         println!("Noooo!");
 LL ~     };
    |
 
-error: equal expressions as operands to `&&`
-  --> tests/ui/match_bool.rs:36:11
-   |
-LL |     match test && test {
-   |           ^^^^^^^^^^^^
-   |
-   = note: `#[deny(clippy::eq_op)]` on by default
-
 error: you seem to be trying to match on a boolean expression
-  --> tests/ui/match_bool.rs:48:5
+  --> tests/ui/match_bool.rs:44:5
    |
 LL | /     match test {
 LL | |
@@ -109,12 +95,74 @@ LL | |     };
    |
 help: consider using an `if`/`else` expression
    |
-LL ~     if test {
-LL +         println!("Yes!");
-LL +     } else {
+LL ~     if !test {
 LL +         println!("Noooo!");
+LL +     } else {
+LL +         println!("Yes!");
 LL ~     };
    |
 
-error: aborting due to 8 previous errors
+error: you seem to be trying to match on a boolean expression
+  --> tests/ui/match_bool.rs:69:13
+   |
+LL |       let _ = match test {
+   |  _____________^
+LL | |
+LL | |         true if option == 5 => 10,
+LL | |         _ => 1,
+LL | |     };
+   | |_____^ help: consider using an `if`/`else` expression: `if test && option == 5 { 10 } else { 1 }`
+
+error: you seem to be trying to match on a boolean expression
+  --> tests/ui/match_bool.rs:75:13
+   |
+LL |       let _ = match test {
+   |  _____________^
+LL | |
+LL | |         false if option == 5 => 10,
+LL | |         _ => 1,
+LL | |     };
+   | |_____^ help: consider using an `if`/`else` expression: `if !test && option == 5 { 10 } else { 1 }`
+
+error: you seem to be trying to match on a boolean expression
+  --> tests/ui/match_bool.rs:81:5
+   |
+LL | /     match test {
+LL | |
+LL | |         true if option == 5 => println!("Hello"),
+LL | |         _ => (),
+LL | |     };
+   | |_____^ help: consider using an `if`/`else` expression: `if test && option == 5 { println!("Hello") }`
+
+error: you seem to be trying to match on a boolean expression
+  --> tests/ui/match_bool.rs:87:5
+   |
+LL | /     match test {
+LL | |
+LL | |         true if option == 5 => (),
+LL | |         _ => println!("Hello"),
+LL | |     };
+   | |_____^ help: consider using an `if`/`else` expression: `if !(test && option == 5) { println!("Hello") }`
+
+error: you seem to be trying to match on a boolean expression
+  --> tests/ui/match_bool.rs:93:5
+   |
+LL | /     match test {
+LL | |
+LL | |         false if option == 5 => println!("Hello"),
+LL | |         _ => (),
+LL | |     };
+   | |_____^ help: consider using an `if`/`else` expression: `if !test && option == 5 { println!("Hello") }`
+
+error: you seem to be trying to match on a boolean expression
+  --> tests/ui/match_bool.rs:99:5
+   |
+LL | /     match test {
+LL | |
+LL | |         false if option == 5 => (),
+LL | |         _ => println!("Hello"),
+LL | |     };
+   | |_____^ help: consider using an `if`/`else` expression: `if !(!test && option == 5) { println!("Hello") }`
+
+error: aborting due to 12 previous errors