fix: don't check for !x = y/x = !y

ada4a · ada4a · commit 5a89cd192b37 · 2025-08-15T18:30:58.000+02:00
already done by `nonminimal_bool`
diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
@@ -7,7 +7,7 @@ use clippy_utils::{
 };
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
-use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
+use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::Span;
@@ -293,30 +293,6 @@ impl<'tcx> LateLintPass<'tcx> for BoolComparison {
     }
 }
 
-struct ExpressionInfoWithSpan {
-    one_side_is_unary_not: bool,
-    left_span: Span,
-    right_span: Span,
-}
-
-fn is_unary_not(e: &Expr<'_>) -> (bool, Span) {
-    if let ExprKind::Unary(UnOp::Not, operand) = e.kind {
-        return (true, operand.span);
-    }
-    (false, e.span)
-}
-
-fn one_side_is_unary_not<'tcx>(left_side: &'tcx Expr<'_>, right_side: &'tcx Expr<'_>) -> ExpressionInfoWithSpan {
-    let left = is_unary_not(left_side);
-    let right = is_unary_not(right_side);
-
-    ExpressionInfoWithSpan {
-        one_side_is_unary_not: left.0 != right.0,
-        left_span: left.1,
-        right_span: right.1,
-    }
-}
-
 fn check_comparison<'a, 'tcx>(
     cx: &LateContext<'tcx>,
     e: &'tcx Expr<'_>,
@@ -326,41 +302,12 @@ fn check_comparison<'a, 'tcx>(
     right_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &'static str)>,
     no_literal: Option<(impl FnOnce(Sugg<'a>, Sugg<'a>) -> Sugg<'a>, &'static str)>,
 ) {
-    if let ExprKind::Binary(op, left_side, right_side) = e.kind {
+    if let ExprKind::Binary(_, left_side, right_side) = e.kind {
         let mut applicability = Applicability::MachineApplicable;
         // Eliminate parentheses in `e` by using the lo pos of lhs and hi pos of rhs,
         // calling `source_callsite` make sure macros are handled correctly, see issue #9907
         let binop_span = left_side.span.source_callsite().to(right_side.span.source_callsite());
 
-        if op.node == BinOpKind::Eq {
-            let expression_info = one_side_is_unary_not(left_side, right_side);
-            if expression_info.one_side_is_unary_not {
-                span_lint_and_sugg(
-                    cx,
-                    BOOL_COMPARISON,
-                    binop_span,
-                    "this comparison might be written more concisely",
-                    "try simplifying it",
-                    format!(
-                        "{} != {}",
-                        snippet_with_applicability(
-                            cx,
-                            expression_info.left_span.source_callsite(),
-                            "..",
-                            &mut applicability
-                        ),
-                        snippet_with_applicability(
-                            cx,
-                            expression_info.right_span.source_callsite(),
-                            "..",
-                            &mut applicability
-                        )
-                    ),
-                    applicability,
-                );
-            }
-        }
-
         match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
             (Some(true), None) => left_true.map_or((), |(h, m)| {
                 suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h);
diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed
@@ -1,6 +1,6 @@
 #![allow(non_local_definitions, clippy::needless_if)]
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::non_canonical_partial_ord_impl, clippy::nonminimal_bool)]
+#![allow(clippy::non_canonical_partial_ord_impl)]
 
 fn main() {
     let x = true;
@@ -72,26 +72,6 @@ fn issue3703() {
     if false < Foo {}
 }
 
-#[allow(dead_code)]
-fn issue4983() {
-    let a = true;
-    let b = false;
-
-    if a != b {};
-    //~^ bool_comparison
-    if a != b {};
-    //~^ bool_comparison
-    if a == b {};
-    if !a == !b {};
-
-    if b != a {};
-    //~^ bool_comparison
-    if b != a {};
-    //~^ bool_comparison
-    if b == a {};
-    if !b == !a {};
-}
-
 macro_rules! m {
     ($func:ident) => {
         $func()
@@ -139,8 +119,27 @@ fn issue9907() {
     //~^ bool_comparison
     let _ = (!m!(func)) as usize;
     //~^ bool_comparison
-    // This is not part of the issue, but an unexpected found when fixing the issue,
-    // the provided span was inside of macro rather than the macro callsite.
-    let _ = ((1 < 2) != m!(func)) as usize;
-    //~^ bool_comparison
+
+    // NOTE: Here's what used to be here:
+    //
+    // > This is not part of the issue, but an unexpected found when fixing the issue,
+    // > the provided span was inside of macro rather than the macro callsite.
+    // > let _ = ((1 < 2) == !m!(func)) as usize;
+    // > <error pattern for `bool_comparison`> (can't put it into a comment or ui_test complains)
+    //
+    // But we don't lint this case since #15367, and similar cases of `<` of `>` apparently still
+    // have this bug (see #15497), so I can't just modify the example. Consider doing that when the
+    // issue is fixed
+}
+
+#[allow(dead_code, clippy::nonminimal_bool)]
+fn issue15367() {
+    let a = true;
+    let b = false;
+
+    // these cases are handled by `nonminimal_bool`, so don't double-lint
+    if a == !b {};
+    if !a == b {};
+    if b == !a {};
+    if !b == a {};
 }
diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs
@@ -1,6 +1,6 @@
 #![allow(non_local_definitions, clippy::needless_if)]
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::non_canonical_partial_ord_impl, clippy::nonminimal_bool)]
+#![allow(clippy::non_canonical_partial_ord_impl)]
 
 fn main() {
     let x = true;
@@ -72,26 +72,6 @@ fn issue3703() {
     if false < Foo {}
 }
 
-#[allow(dead_code)]
-fn issue4983() {
-    let a = true;
-    let b = false;
-
-    if a == !b {};
-    //~^ bool_comparison
-    if !a == b {};
-    //~^ bool_comparison
-    if a == b {};
-    if !a == !b {};
-
-    if b == !a {};
-    //~^ bool_comparison
-    if !b == a {};
-    //~^ bool_comparison
-    if b == a {};
-    if !b == !a {};
-}
-
 macro_rules! m {
     ($func:ident) => {
         $func()
@@ -139,8 +119,27 @@ fn issue9907() {
     //~^ bool_comparison
     let _ = (false == m!(func)) as usize;
     //~^ bool_comparison
-    // This is not part of the issue, but an unexpected found when fixing the issue,
-    // the provided span was inside of macro rather than the macro callsite.
-    let _ = ((1 < 2) == !m!(func)) as usize;
-    //~^ bool_comparison
+
+    // NOTE: Here's what used to be here:
+    //
+    // > This is not part of the issue, but an unexpected found when fixing the issue,
+    // > the provided span was inside of macro rather than the macro callsite.
+    // > let _ = ((1 < 2) == !m!(func)) as usize;
+    // > <error pattern for `bool_comparison`> (can't put it into a comment or ui_test complains)
+    //
+    // But we don't lint this case since #15367, and similar cases of `<` of `>` apparently still
+    // have this bug (see #15497), so I can't just modify the example. Consider doing that when the
+    // issue is fixed
+}
+
+#[allow(dead_code, clippy::nonminimal_bool)]
+fn issue15367() {
+    let a = true;
+    let b = false;
+
+    // these cases are handled by `nonminimal_bool`, so don't double-lint
+    if a == !b {};
+    if !a == b {};
+    if b == !a {};
+    if !b == a {};
 }
diff --git a/tests/ui/bool_comparison.stderr b/tests/ui/bool_comparison.stderr
@@ -85,71 +85,41 @@ error: order comparisons between booleans can be simplified
 LL |     let _ = if x > y { "yes" } else { "no" };
    |                ^^^^^ help: try simplifying it: `x & !y`
 
-error: this comparison might be written more concisely
-  --> tests/ui/bool_comparison.rs:80:8
-   |
-LL |     if a == !b {};
-   |        ^^^^^^^ help: try simplifying it: `a != b`
-
-error: this comparison might be written more concisely
-  --> tests/ui/bool_comparison.rs:82:8
-   |
-LL |     if !a == b {};
-   |        ^^^^^^^ help: try simplifying it: `a != b`
-
-error: this comparison might be written more concisely
-  --> tests/ui/bool_comparison.rs:87:8
-   |
-LL |     if b == !a {};
-   |        ^^^^^^^ help: try simplifying it: `b != a`
-
-error: this comparison might be written more concisely
-  --> tests/ui/bool_comparison.rs:89:8
-   |
-LL |     if !b == a {};
-   |        ^^^^^^^ help: try simplifying it: `b != a`
-
 error: equality checks against false can be replaced by a negation
-  --> tests/ui/bool_comparison.rs:114:8
+  --> tests/ui/bool_comparison.rs:94:8
    |
 LL |     if false == m!(func) {}
    |        ^^^^^^^^^^^^^^^^^ help: try simplifying it: `!m!(func)`
 
 error: equality checks against false can be replaced by a negation
-  --> tests/ui/bool_comparison.rs:116:8
+  --> tests/ui/bool_comparison.rs:96:8
    |
 LL |     if m!(func) == false {}
    |        ^^^^^^^^^^^^^^^^^ help: try simplifying it: `!m!(func)`
 
 error: equality checks against true are unnecessary
-  --> tests/ui/bool_comparison.rs:118:8
+  --> tests/ui/bool_comparison.rs:98:8
    |
 LL |     if true == m!(func) {}
    |        ^^^^^^^^^^^^^^^^ help: try simplifying it: `m!(func)`
 
 error: equality checks against true are unnecessary
-  --> tests/ui/bool_comparison.rs:120:8
+  --> tests/ui/bool_comparison.rs:100:8
    |
 LL |     if m!(func) == true {}
    |        ^^^^^^^^^^^^^^^^ help: try simplifying it: `m!(func)`
 
 error: equality checks against false can be replaced by a negation
-  --> tests/ui/bool_comparison.rs:138:14
+  --> tests/ui/bool_comparison.rs:118:14
    |
 LL |     let _ = ((1 < 2) == false) as usize;
    |              ^^^^^^^^^^^^^^^^ help: try simplifying it: `1 >= 2`
 
 error: equality checks against false can be replaced by a negation
-  --> tests/ui/bool_comparison.rs:140:14
+  --> tests/ui/bool_comparison.rs:120:14
    |
 LL |     let _ = (false == m!(func)) as usize;
    |              ^^^^^^^^^^^^^^^^^ help: try simplifying it: `!m!(func)`
 
-error: this comparison might be written more concisely
-  --> tests/ui/bool_comparison.rs:144:14
-   |
-LL |     let _ = ((1 < 2) == !m!(func)) as usize;
-   |              ^^^^^^^^^^^^^^^^^^^^ help: try simplifying it: `(1 < 2) != m!(func)`
-
-error: aborting due to 25 previous errors
+error: aborting due to 20 previous errors
 
