Auto merge of #10293 - Alexendoo:bool-assert-comparison-negation, r=dswij

Negate suggestions when needed in `bool_assert_comparison`

changelog: none assuming this gets into the same release as #10218

Fixes #10291

r? `@dswij`

Thanks to `@black-puppydog` for spotting it early

4 files changed, 176 insertions, 51 deletions

diff --git a/clippy_lints/src/bool_assert_comparison.rs b/clippy_lints/src/bool_assert_comparison.rs
index 556fa579000..1d9096ea64d 100644
--- a/clippy_lints/src/bool_assert_comparison.rs
+++ b/clippy_lints/src/bool_assert_comparison.rs
@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::macros::{find_assert_eq_args, root_macro_call_first_node};
+use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::{implements_trait, is_copy};
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
@@ -34,14 +35,16 @@ declare_clippy_lint! {
 
 declare_lint_pass!(BoolAssertComparison => [BOOL_ASSERT_COMPARISON]);
 
-fn is_bool_lit(e: &Expr<'_>) -> bool {
-    matches!(
-        e.kind,
-        ExprKind::Lit(Lit {
-            node: LitKind::Bool(_),
-            ..
-        })
-    ) && !e.span.from_expansion()
+fn extract_bool_lit(e: &Expr<'_>) -> Option<bool> {
+    if let ExprKind::Lit(Lit {
+        node: LitKind::Bool(b), ..
+    }) = e.kind
+        && !e.span.from_expansion()
+    {
+        Some(b)
+    } else {
+        None
+    }
 }
 
 fn is_impl_not_trait_with_bool_out<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
@@ -69,24 +72,23 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return };
         let macro_name = cx.tcx.item_name(macro_call.def_id);
-        if !matches!(
-            macro_name.as_str(),
-            "assert_eq" | "debug_assert_eq" | "assert_ne" | "debug_assert_ne"
-        ) {
-            return;
-        }
+        let eq_macro = match macro_name.as_str() {
+            "assert_eq" | "debug_assert_eq" => true,
+            "assert_ne" | "debug_assert_ne" => false,
+            _ => return,
+        };
         let Some ((a, b, _)) = find_assert_eq_args(cx, expr, macro_call.expn) else { return };
 
         let a_span = a.span.source_callsite();
         let b_span = b.span.source_callsite();
 
-        let (lit_span, non_lit_expr) = match (is_bool_lit(a), is_bool_lit(b)) {
-            // assert_eq!(true, b)
-            //            ^^^^^^
-            (true, false) => (a_span.until(b_span), b),
-            // assert_eq!(a, true)
-            //             ^^^^^^
-            (false, true) => (b_span.with_lo(a_span.hi()), a),
+        let (lit_span, bool_value, non_lit_expr) = match (extract_bool_lit(a), extract_bool_lit(b)) {
+            // assert_eq!(true/false, b)
+            //            ^^^^^^^^^^^^
+            (Some(bool_value), None) => (a_span.until(b_span), bool_value, b),
+            // assert_eq!(a, true/false)
+            //             ^^^^^^^^^^^^
+            (None, Some(bool_value)) => (b_span.with_lo(a_span.hi()), bool_value, a),
             // If there are two boolean arguments, we definitely don't understand
             // what's going on, so better leave things as is...
             //
@@ -121,9 +123,16 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
                 // ^^^^^^^^^
                 let name_span = cx.sess().source_map().span_until_char(macro_call.span, '!');
 
+                let mut suggestions = vec![(name_span, non_eq_mac.to_string()), (lit_span, String::new())];
+
+                if bool_value ^ eq_macro {
+                    let Some(sugg) = Sugg::hir_opt(cx, non_lit_expr) else { return };
+                    suggestions.push((non_lit_expr.span, (!sugg).to_string()));
+                }
+
                 diag.multipart_suggestion(
                     format!("replace it with `{non_eq_mac}!(..)`"),
-                    vec![(name_span, non_eq_mac.to_string()), (lit_span, String::new())],
+                    suggestions,
                     Applicability::MachineApplicable,
                 );
             },
diff --git a/tests/ui/bool_assert_comparison.fixed b/tests/ui/bool_assert_comparison.fixed
index 95f35a61bb2..b8dd92906c8 100644
--- a/tests/ui/bool_assert_comparison.fixed
+++ b/tests/ui/bool_assert_comparison.fixed
@@ -86,7 +86,7 @@ fn main() {
     let b = ImplNotTraitWithBool;
 
     assert_eq!("a".len(), 1);
-    assert!("a".is_empty());
+    assert!(!"a".is_empty());
     assert!("".is_empty());
     assert!("".is_empty());
     assert_eq!(a!(), b!());
@@ -97,16 +97,16 @@ fn main() {
 
     assert_ne!("a".len(), 1);
     assert!("a".is_empty());
-    assert!("".is_empty());
-    assert!("".is_empty());
+    assert!(!"".is_empty());
+    assert!(!"".is_empty());
     assert_ne!(a!(), b!());
     assert_ne!(a!(), "".is_empty());
     assert_ne!("".is_empty(), b!());
     assert_ne!(a, true);
-    assert!(b);
+    assert!(!b);
 
     debug_assert_eq!("a".len(), 1);
-    debug_assert!("a".is_empty());
+    debug_assert!(!"a".is_empty());
     debug_assert!("".is_empty());
     debug_assert!("".is_empty());
     debug_assert_eq!(a!(), b!());
@@ -117,27 +117,27 @@ fn main() {
 
     debug_assert_ne!("a".len(), 1);
     debug_assert!("a".is_empty());
-    debug_assert!("".is_empty());
-    debug_assert!("".is_empty());
+    debug_assert!(!"".is_empty());
+    debug_assert!(!"".is_empty());
     debug_assert_ne!(a!(), b!());
     debug_assert_ne!(a!(), "".is_empty());
     debug_assert_ne!("".is_empty(), b!());
     debug_assert_ne!(a, true);
-    debug_assert!(b);
+    debug_assert!(!b);
 
     // assert with error messages
     assert_eq!("a".len(), 1, "tadam {}", 1);
     assert_eq!("a".len(), 1, "tadam {}", true);
-    assert!("a".is_empty(), "tadam {}", 1);
-    assert!("a".is_empty(), "tadam {}", true);
-    assert!("a".is_empty(), "tadam {}", true);
+    assert!(!"a".is_empty(), "tadam {}", 1);
+    assert!(!"a".is_empty(), "tadam {}", true);
+    assert!(!"a".is_empty(), "tadam {}", true);
     assert_eq!(a, true, "tadam {}", false);
 
     debug_assert_eq!("a".len(), 1, "tadam {}", 1);
     debug_assert_eq!("a".len(), 1, "tadam {}", true);
-    debug_assert!("a".is_empty(), "tadam {}", 1);
-    debug_assert!("a".is_empty(), "tadam {}", true);
-    debug_assert!("a".is_empty(), "tadam {}", true);
+    debug_assert!(!"a".is_empty(), "tadam {}", 1);
+    debug_assert!(!"a".is_empty(), "tadam {}", true);
+    debug_assert!(!"a".is_empty(), "tadam {}", true);
     debug_assert_eq!(a, true, "tadam {}", false);
 
     assert!(a!());
@@ -158,4 +158,14 @@ fn main() {
         }};
     }
     in_macro!(a);
+
+    assert!("".is_empty());
+    assert!("".is_empty());
+    assert!(!"requires negation".is_empty());
+    assert!(!"requires negation".is_empty());
+
+    debug_assert!("".is_empty());
+    debug_assert!("".is_empty());
+    debug_assert!(!"requires negation".is_empty());
+    debug_assert!(!"requires negation".is_empty());
 }
diff --git a/tests/ui/bool_assert_comparison.rs b/tests/ui/bool_assert_comparison.rs
index 88e7560b4f9..0a8ad34fda5 100644
--- a/tests/ui/bool_assert_comparison.rs
+++ b/tests/ui/bool_assert_comparison.rs
@@ -158,4 +158,14 @@ fn main() {
         }};
     }
     in_macro!(a);
+
+    assert_eq!("".is_empty(), true);
+    assert_ne!("".is_empty(), false);
+    assert_ne!("requires negation".is_empty(), true);
+    assert_eq!("requires negation".is_empty(), false);
+
+    debug_assert_eq!("".is_empty(), true);
+    debug_assert_ne!("".is_empty(), false);
+    debug_assert_ne!("requires negation".is_empty(), true);
+    debug_assert_eq!("requires negation".is_empty(), false);
 }
diff --git a/tests/ui/bool_assert_comparison.stderr b/tests/ui/bool_assert_comparison.stderr
index 3d9f8573e61..89cefc95a9f 100644
--- a/tests/ui/bool_assert_comparison.stderr
+++ b/tests/ui/bool_assert_comparison.stderr
@@ -8,7 +8,7 @@ LL |     assert_eq!("a".is_empty(), false);
 help: replace it with `assert!(..)`
    |
 LL -     assert_eq!("a".is_empty(), false);
-LL +     assert!("a".is_empty());
+LL +     assert!(!"a".is_empty());
    |
 
 error: used `assert_eq!` with a literal bool
@@ -68,7 +68,7 @@ LL |     assert_ne!("".is_empty(), true);
 help: replace it with `assert!(..)`
    |
 LL -     assert_ne!("".is_empty(), true);
-LL +     assert!("".is_empty());
+LL +     assert!(!"".is_empty());
    |
 
 error: used `assert_ne!` with a literal bool
@@ -80,7 +80,7 @@ LL |     assert_ne!(true, "".is_empty());
 help: replace it with `assert!(..)`
    |
 LL -     assert_ne!(true, "".is_empty());
-LL +     assert!("".is_empty());
+LL +     assert!(!"".is_empty());
    |
 
 error: used `assert_ne!` with a literal bool
@@ -92,7 +92,7 @@ LL |     assert_ne!(b, true);
 help: replace it with `assert!(..)`
    |
 LL -     assert_ne!(b, true);
-LL +     assert!(b);
+LL +     assert!(!b);
    |
 
 error: used `debug_assert_eq!` with a literal bool
@@ -104,7 +104,7 @@ LL |     debug_assert_eq!("a".is_empty(), false);
 help: replace it with `debug_assert!(..)`
    |
 LL -     debug_assert_eq!("a".is_empty(), false);
-LL +     debug_assert!("a".is_empty());
+LL +     debug_assert!(!"a".is_empty());
    |
 
 error: used `debug_assert_eq!` with a literal bool
@@ -164,7 +164,7 @@ LL |     debug_assert_ne!("".is_empty(), true);
 help: replace it with `debug_assert!(..)`
    |
 LL -     debug_assert_ne!("".is_empty(), true);
-LL +     debug_assert!("".is_empty());
+LL +     debug_assert!(!"".is_empty());
    |
 
 error: used `debug_assert_ne!` with a literal bool
@@ -176,7 +176,7 @@ LL |     debug_assert_ne!(true, "".is_empty());
 help: replace it with `debug_assert!(..)`
    |
 LL -     debug_assert_ne!(true, "".is_empty());
-LL +     debug_assert!("".is_empty());
+LL +     debug_assert!(!"".is_empty());
    |
 
 error: used `debug_assert_ne!` with a literal bool
@@ -188,7 +188,7 @@ LL |     debug_assert_ne!(b, true);
 help: replace it with `debug_assert!(..)`
    |
 LL -     debug_assert_ne!(b, true);
-LL +     debug_assert!(b);
+LL +     debug_assert!(!b);
    |
 
 error: used `assert_eq!` with a literal bool
@@ -200,7 +200,7 @@ LL |     assert_eq!("a".is_empty(), false, "tadam {}", 1);
 help: replace it with `assert!(..)`
    |
 LL -     assert_eq!("a".is_empty(), false, "tadam {}", 1);
-LL +     assert!("a".is_empty(), "tadam {}", 1);
+LL +     assert!(!"a".is_empty(), "tadam {}", 1);
    |
 
 error: used `assert_eq!` with a literal bool
@@ -212,7 +212,7 @@ LL |     assert_eq!("a".is_empty(), false, "tadam {}", true);
 help: replace it with `assert!(..)`
    |
 LL -     assert_eq!("a".is_empty(), false, "tadam {}", true);
-LL +     assert!("a".is_empty(), "tadam {}", true);
+LL +     assert!(!"a".is_empty(), "tadam {}", true);
    |
 
 error: used `assert_eq!` with a literal bool
@@ -224,7 +224,7 @@ LL |     assert_eq!(false, "a".is_empty(), "tadam {}", true);
 help: replace it with `assert!(..)`
    |
 LL -     assert_eq!(false, "a".is_empty(), "tadam {}", true);
-LL +     assert!("a".is_empty(), "tadam {}", true);
+LL +     assert!(!"a".is_empty(), "tadam {}", true);
    |
 
 error: used `debug_assert_eq!` with a literal bool
@@ -236,7 +236,7 @@ LL |     debug_assert_eq!("a".is_empty(), false, "tadam {}", 1);
 help: replace it with `debug_assert!(..)`
    |
 LL -     debug_assert_eq!("a".is_empty(), false, "tadam {}", 1);
-LL +     debug_assert!("a".is_empty(), "tadam {}", 1);
+LL +     debug_assert!(!"a".is_empty(), "tadam {}", 1);
    |
 
 error: used `debug_assert_eq!` with a literal bool
@@ -248,7 +248,7 @@ LL |     debug_assert_eq!("a".is_empty(), false, "tadam {}", true);
 help: replace it with `debug_assert!(..)`
    |
 LL -     debug_assert_eq!("a".is_empty(), false, "tadam {}", true);
-LL +     debug_assert!("a".is_empty(), "tadam {}", true);
+LL +     debug_assert!(!"a".is_empty(), "tadam {}", true);
    |
 
 error: used `debug_assert_eq!` with a literal bool
@@ -260,7 +260,7 @@ LL |     debug_assert_eq!(false, "a".is_empty(), "tadam {}", true);
 help: replace it with `debug_assert!(..)`
    |
 LL -     debug_assert_eq!(false, "a".is_empty(), "tadam {}", true);
-LL +     debug_assert!("a".is_empty(), "tadam {}", true);
+LL +     debug_assert!(!"a".is_empty(), "tadam {}", true);
    |
 
 error: used `assert_eq!` with a literal bool
@@ -299,5 +299,101 @@ LL -     renamed!(b, true);
 LL +     debug_assert!(b);
    |
 
-error: aborting due to 25 previous errors
+error: used `assert_eq!` with a literal bool
+  --> $DIR/bool_assert_comparison.rs:162:5
+   |
+LL |     assert_eq!("".is_empty(), true);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: replace it with `assert!(..)`
+   |
+LL -     assert_eq!("".is_empty(), true);
+LL +     assert!("".is_empty());
+   |
+
+error: used `assert_ne!` with a literal bool
+  --> $DIR/bool_assert_comparison.rs:163:5
+   |
+LL |     assert_ne!("".is_empty(), false);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: replace it with `assert!(..)`
+   |
+LL -     assert_ne!("".is_empty(), false);
+LL +     assert!("".is_empty());
+   |
+
+error: used `assert_ne!` with a literal bool
+  --> $DIR/bool_assert_comparison.rs:164:5
+   |
+LL |     assert_ne!("requires negation".is_empty(), true);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: replace it with `assert!(..)`
+   |
+LL -     assert_ne!("requires negation".is_empty(), true);
+LL +     assert!(!"requires negation".is_empty());
+   |
+
+error: used `assert_eq!` with a literal bool
+  --> $DIR/bool_assert_comparison.rs:165:5
+   |
+LL |     assert_eq!("requires negation".is_empty(), false);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: replace it with `assert!(..)`
+   |
+LL -     assert_eq!("requires negation".is_empty(), false);
+LL +     assert!(!"requires negation".is_empty());
+   |
+
+error: used `debug_assert_eq!` with a literal bool
+  --> $DIR/bool_assert_comparison.rs:167:5
+   |
+LL |     debug_assert_eq!("".is_empty(), true);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: replace it with `debug_assert!(..)`
+   |
+LL -     debug_assert_eq!("".is_empty(), true);
+LL +     debug_assert!("".is_empty());
+   |
+
+error: used `debug_assert_ne!` with a literal bool
+  --> $DIR/bool_assert_comparison.rs:168:5
+   |
+LL |     debug_assert_ne!("".is_empty(), false);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: replace it with `debug_assert!(..)`
+   |
+LL -     debug_assert_ne!("".is_empty(), false);
+LL +     debug_assert!("".is_empty());
+   |
+
+error: used `debug_assert_ne!` with a literal bool
+  --> $DIR/bool_assert_comparison.rs:169:5
+   |
+LL |     debug_assert_ne!("requires negation".is_empty(), true);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: replace it with `debug_assert!(..)`
+   |
+LL -     debug_assert_ne!("requires negation".is_empty(), true);
+LL +     debug_assert!(!"requires negation".is_empty());
+   |
+
+error: used `debug_assert_eq!` with a literal bool
+  --> $DIR/bool_assert_comparison.rs:170:5
+   |
+LL |     debug_assert_eq!("requires negation".is_empty(), false);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: replace it with `debug_assert!(..)`
+   |
+LL -     debug_assert_eq!("requires negation".is_empty(), false);
+LL +     debug_assert!(!"requires negation".is_empty());
+   |
+
+error: aborting due to 33 previous errors