Extend `NONMINIMAL_BOOL` to check inverted boolean values

5 files changed, 191 insertions, 23 deletions

diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs
index 3c17f65f0d3..0d66f2d644d 100644
--- a/clippy_lints/src/booleans.rs
+++ b/clippy_lints/src/booleans.rs
@@ -87,31 +87,115 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
     }
 
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
-        if let ExprKind::Unary(UnOp::Not, sub) = expr.kind
-            && !expr.span.from_expansion()
-            && let ExprKind::Binary(op, left, right) = sub.kind
-        {
-            let new_op = match op.node {
-                BinOpKind::Eq => "!=",
-                BinOpKind::Ne => "==",
-                _ => return,
+        match expr.kind {
+            ExprKind::Unary(UnOp::Not, sub) => check_inverted_condition(cx, expr.span, sub),
+            // This check the case where an element in a boolean comparison is inverted, like:
+            //
+            // ```
+            // let a = true;
+            // !a == false;
+            // ```
+            ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) => {
+                check_inverted_bool_in_condition(cx, expr.span, op.node, left, right);
+            },
+            _ => {},
+        }
+    }
+}
+
+fn inverted_bin_op_eq_str(op: BinOpKind) -> Option<&'static str> {
+    match op {
+        BinOpKind::Eq => Some("!="),
+        BinOpKind::Ne => Some("=="),
+        _ => None,
+    }
+}
+
+fn bin_op_eq_str(op: BinOpKind) -> Option<&'static str> {
+    match op {
+        BinOpKind::Eq => Some("=="),
+        BinOpKind::Ne => Some("!="),
+        _ => None,
+    }
+}
+
+fn check_inverted_condition(cx: &LateContext<'_>, expr_span: Span, sub_expr: &Expr<'_>) {
+    if !expr_span.from_expansion()
+        && let ExprKind::Binary(op, left, right) = sub_expr.kind
+        && let Some(left) = snippet_opt(cx, left.span)
+        && let Some(right) = snippet_opt(cx, right.span)
+    {
+        let Some(op) = inverted_bin_op_eq_str(op.node) else {
+            return;
+        };
+        span_lint_and_sugg(
+            cx,
+            NONMINIMAL_BOOL,
+            expr_span,
+            "this boolean expression can be simplified",
+            "try",
+            format!("{left} {op} {right}",),
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
+fn check_inverted_bool_in_condition(
+    cx: &LateContext<'_>,
+    expr_span: Span,
+    op: BinOpKind,
+    left: &Expr<'_>,
+    right: &Expr<'_>,
+) {
+    if expr_span.from_expansion()
+        && (!cx.typeck_results().node_types()[left.hir_id].is_bool()
+            || !cx.typeck_results().node_types()[right.hir_id].is_bool())
+    {
+        return;
+    }
+
+    let suggestion = match (left.kind, right.kind) {
+        (ExprKind::Unary(UnOp::Not, left_sub), ExprKind::Unary(UnOp::Not, right_sub)) => {
+            let Some(left) = snippet_opt(cx, left_sub.span) else {
+                return;
+            };
+            let Some(right) = snippet_opt(cx, right_sub.span) else {
+                return;
+            };
+            let Some(op) = bin_op_eq_str(op) else { return };
+            format!("{left} {op} {right}")
+        },
+        (ExprKind::Unary(UnOp::Not, left_sub), _) => {
+            let Some(left) = snippet_opt(cx, left_sub.span) else {
+                return;
             };
-            let Some(left) = snippet_opt(cx, left.span) else { return };
             let Some(right) = snippet_opt(cx, right.span) else {
                 return;
             };
-            span_lint_and_sugg(
-                cx,
-                NONMINIMAL_BOOL,
-                expr.span,
-                "this boolean expression can be simplified",
-                "try",
-                format!("{left} {new_op} {right}"),
-                Applicability::MachineApplicable,
-            );
-        }
-    }
+            let Some(op) = inverted_bin_op_eq_str(op) else { return };
+            format!("{left} {op} {right}")
+        },
+        (_, ExprKind::Unary(UnOp::Not, right_sub)) => {
+            let Some(left) = snippet_opt(cx, left.span) else { return };
+            let Some(right) = snippet_opt(cx, right_sub.span) else {
+                return;
+            };
+            let Some(op) = inverted_bin_op_eq_str(op) else { return };
+            format!("{left} {op} {right}")
+        },
+        _ => return,
+    };
+    span_lint_and_sugg(
+        cx,
+        NONMINIMAL_BOOL,
+        expr_span,
+        "this boolean expression can be simplified",
+        "try",
+        suggestion,
+        Applicability::MachineApplicable,
+    );
 }
+
 struct NonminimalBoolVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
 }
diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed
index 02f1d09b833..54bdf7f5d70 100644
--- a/tests/ui/bool_comparison.fixed
+++ b/tests/ui/bool_comparison.fixed
@@ -1,6 +1,6 @@
 #![allow(clippy::needless_if)]
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::non_canonical_partial_ord_impl)]
+#![allow(clippy::non_canonical_partial_ord_impl, clippy::nonminimal_bool)]
 
 fn main() {
     let x = true;
diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs
index 5ef696d855e..4fdf2305242 100644
--- a/tests/ui/bool_comparison.rs
+++ b/tests/ui/bool_comparison.rs
@@ -1,6 +1,6 @@
 #![allow(clippy::needless_if)]
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::non_canonical_partial_ord_impl)]
+#![allow(clippy::non_canonical_partial_ord_impl, clippy::nonminimal_bool)]
 
 fn main() {
     let x = true;
diff --git a/tests/ui/nonminimal_bool.rs b/tests/ui/nonminimal_bool.rs
index ee092b9aca6..908ef6cb2a0 100644
--- a/tests/ui/nonminimal_bool.rs
+++ b/tests/ui/nonminimal_bool.rs
@@ -163,4 +163,13 @@ fn issue_5794() {
     if !(a == 12) {} //~ ERROR: this boolean expression can be simplified
     if !(12 != a) {} //~ ERROR: this boolean expression can be simplified
     if !(a != 12) {} //~ ERROR: this boolean expression can be simplified
+
+    let b = true;
+    let c = false;
+    if !b == true {} //~ ERROR: this boolean expression can be simplified
+    if !b != true {} //~ ERROR: this boolean expression can be simplified
+    if true == !b {} //~ ERROR: this boolean expression can be simplified
+    if true != !b {} //~ ERROR: this boolean expression can be simplified
+    if !b == !c {} //~ ERROR: this boolean expression can be simplified
+    if !b != !c {} //~ ERROR: this boolean expression can be simplified
 }
diff --git a/tests/ui/nonminimal_bool.stderr b/tests/ui/nonminimal_bool.stderr
index 856b5cd0869..a317c8328d9 100644
--- a/tests/ui/nonminimal_bool.stderr
+++ b/tests/ui/nonminimal_bool.stderr
@@ -138,5 +138,80 @@ error: this boolean expression can be simplified
 LL |     if !(a != 12) {}
    |        ^^^^^^^^^^ help: try: `a == 12`
 
-error: aborting due to 17 previous errors
+error: this boolean expression can be simplified
+  --> $DIR/nonminimal_bool.rs:169:8
+   |
+LL |     if !b == true {}
+   |        ^^^^^^^^^^ help: try: `b != true`
+
+error: this comparison might be written more concisely
+  --> $DIR/nonminimal_bool.rs:169:8
+   |
+LL |     if !b == true {}
+   |        ^^^^^^^^^^ help: try simplifying it as shown: `b != true`
+   |
+   = note: `-D clippy::bool-comparison` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::bool_comparison)]`
+
+error: equality checks against true are unnecessary
+  --> $DIR/nonminimal_bool.rs:169:8
+   |
+LL |     if !b == true {}
+   |        ^^^^^^^^^^ help: try simplifying it as shown: `!b`
+
+error: this boolean expression can be simplified
+  --> $DIR/nonminimal_bool.rs:170:8
+   |
+LL |     if !b != true {}
+   |        ^^^^^^^^^^ help: try: `b == true`
+
+error: inequality checks against true can be replaced by a negation
+  --> $DIR/nonminimal_bool.rs:170:8
+   |
+LL |     if !b != true {}
+   |        ^^^^^^^^^^ help: try simplifying it as shown: `!(!b)`
+
+error: this boolean expression can be simplified
+  --> $DIR/nonminimal_bool.rs:171:8
+   |
+LL |     if true == !b {}
+   |        ^^^^^^^^^^ help: try: `true != b`
+
+error: this comparison might be written more concisely
+  --> $DIR/nonminimal_bool.rs:171:8
+   |
+LL |     if true == !b {}
+   |        ^^^^^^^^^^ help: try simplifying it as shown: `true != b`
+
+error: equality checks against true are unnecessary
+  --> $DIR/nonminimal_bool.rs:171:8
+   |
+LL |     if true == !b {}
+   |        ^^^^^^^^^^ help: try simplifying it as shown: `!b`
+
+error: this boolean expression can be simplified
+  --> $DIR/nonminimal_bool.rs:172:8
+   |
+LL |     if true != !b {}
+   |        ^^^^^^^^^^ help: try: `true == b`
+
+error: inequality checks against true can be replaced by a negation
+  --> $DIR/nonminimal_bool.rs:172:8
+   |
+LL |     if true != !b {}
+   |        ^^^^^^^^^^ help: try simplifying it as shown: `!(!b)`
+
+error: this boolean expression can be simplified
+  --> $DIR/nonminimal_bool.rs:173:8
+   |
+LL |     if !b == !c {}
+   |        ^^^^^^^^ help: try: `b == c`
+
+error: this boolean expression can be simplified
+  --> $DIR/nonminimal_bool.rs:174:8
+   |
+LL |     if !b != !c {}
+   |        ^^^^^^^^ help: try: `b != c`
+
+error: aborting due to 29 previous errors