feat(manual_assert_eq): new lint

CHANGELOG.md

@@ -6526,6 +6526,7 @@ Released 2018-09-13
 [`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
 [`manual_abs_diff`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff
 [`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
+[`manual_assert_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert_eq
 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
 [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
 [`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals

clippy_lints/src/declared_lints.rs

@@ -292,6 +292,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::main_recursion::MAIN_RECURSION_INFO,
     crate::manual_abs_diff::MANUAL_ABS_DIFF_INFO,
     crate::manual_assert::MANUAL_ASSERT_INFO,
+    crate::manual_assert_eq::MANUAL_ASSERT_EQ_INFO,
     crate::manual_async_fn::MANUAL_ASYNC_FN_INFO,
     crate::manual_bits::MANUAL_BITS_INFO,
     crate::manual_clamp::MANUAL_CLAMP_INFO,

clippy_lints/src/eta_reduction.rs

@@ -371,7 +371,7 @@ fn has_late_bound_to_non_late_bound_regions(from_sig: FnSig<'_>, to_sig: FnSig<'
         }
     }
 
-    assert!(from_sig.inputs_and_output.len() == to_sig.inputs_and_output.len());
+    assert_eq!(from_sig.inputs_and_output.len(), to_sig.inputs_and_output.len());
     from_sig
         .inputs_and_output
         .iter()

clippy_lints/src/functions/renamed_function_params.rs

@@ -57,7 +57,7 @@ impl RenamedFnArgs {
     {
         let mut renamed: Vec<(Span, String)> = vec![];
 
-        debug_assert!(default_idents.size_hint() == current_idents.size_hint());
+        debug_assert_eq!(default_idents.size_hint(), current_idents.size_hint());
         for (default_ident, current_ident) in iter::zip(default_idents, current_idents) {
             let has_name_to_check = |ident: Option<Ident>| {
                 ident

clippy_lints/src/lib.rs

@@ -195,6 +195,7 @@ mod macro_use;
 mod main_recursion;
 mod manual_abs_diff;
 mod manual_assert;
+mod manual_assert_eq;
 mod manual_async_fn;
 mod manual_bits;
 mod manual_clamp;
@@ -848,6 +849,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)),
         Box::new(|_| Box::new(volatile_composites::VolatileComposites)),
         Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
+        Box::new(|_| Box::new(manual_assert_eq::ManualAssertEq)),
         // add late passes here, used by `cargo dev new_lint`
     ];
     store.late_passes.extend(late_lints);

clippy_lints/src/manual_assert_eq.rs

@@ -0,0 +1,88 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::macros::{find_assert_args, root_macro_call_first_node};
+use clippy_utils::source::walk_span_to_context;
+use clippy_utils::ty::implements_trait;
+use clippy_utils::{is_in_const_context, sym};
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `assert!` and `debug_assert!` that consist of only an (in)equality check
+    ///
+    /// ### Why is this bad?
+    /// `assert_{eq,ne}!` and `debug_assert_{eq,ne}!` achieves the same goal, and provides some
+    /// additional debug information
+    ///
+    /// ### Example
+    /// ```no_run
+    /// assert!(2 * 2 == 4);
+    /// assert!(2 * 2 != 5);
+    /// debug_assert!(2 * 2 == 4);
+    /// debug_assert!(2 * 2 != 5);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// assert_eq!(2 * 2, 4);
+    /// assert_ne!(2 * 2, 5);
+    /// debug_assert_eq!(2 * 2, 4);
+    /// debug_assert_ne!(2 * 2, 5);
+    /// ```
+    #[clippy::version = "1.92.0"]
+    pub MANUAL_ASSERT_EQ,
+    complexity,
+    "checks for assertions consisting of an (in)equality check"
+}
+declare_lint_pass!(ManualAssertEq => [MANUAL_ASSERT_EQ]);
+
+impl LateLintPass<'_> for ManualAssertEq {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        if let Some(macro_call) = root_macro_call_first_node(cx, expr)
+            && let macro_name = match cx.tcx.get_diagnostic_name(macro_call.def_id) {
+                Some(sym::assert_macro) => "assert",
+                Some(sym::debug_assert_macro) => "debug_assert",
+                _ => return,
+            }
+            // `assert_eq` isn't allowed in const context because it calls non-const `core::panicking::assert_failed`
+            // XXX: this might change in the future, so might want to relax this restriction
+            && !is_in_const_context(cx)
+            && let Some((cond, _)) = find_assert_args(cx, expr, macro_call.expn)
+            && let ExprKind::Binary(op, lhs, rhs) = cond.kind
+            && matches!(op.node, BinOpKind::Eq | BinOpKind::Ne)
+            && !cond.span.from_expansion()
+            && let Some(debug_trait) = cx.tcx.get_diagnostic_item(sym::Debug)
+            && implements_trait(cx, cx.typeck_results().expr_ty(lhs), debug_trait, &[])
+            && implements_trait(cx, cx.typeck_results().expr_ty(rhs), debug_trait, &[])
+        {
+            span_lint_and_then(
+                cx,
+                MANUAL_ASSERT_EQ,
+                macro_call.span,
+                format!("used `{macro_name}!` with an equality comparison"),
+                |diag| {
+                    let kind = if op.node == BinOpKind::Eq { "eq" } else { "ne" };
+                    let new_name = format!("{macro_name}_{kind}");
+                    let msg = format!("replace it with `{new_name}!(..)`");
+
+                    let ctxt = cond.span.ctxt();
+                    if let Some(lhs_span) = walk_span_to_context(lhs.span, ctxt)
+                        && let Some(rhs_span) = walk_span_to_context(rhs.span, ctxt)
+                    {
+                        let macro_name_span = cx.sess().source_map().span_until_char(macro_call.span, '!');
+                        let eq_span = cond.span.with_lo(lhs_span.hi()).with_hi(rhs_span.lo());
+                        let suggestions = vec![
+                            (macro_name_span.shrink_to_hi(), format!("_{kind}")),
+                            (eq_span, ", ".to_string()),
+                        ];
+
+                        diag.multipart_suggestion(msg, suggestions, Applicability::MachineApplicable);
+                    } else {
+                        diag.span_help(expr.span, msg);
+                    }
+                },
+            );
+        }
+    }
+}

clippy_utils/src/ty/mod.rs

@@ -915,14 +915,20 @@ fn assert_generic_args_match<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, args: &[Generi
         .chain(&g.own_params)
         .map(|x| &x.kind);
 
-    assert!(
-        count == args.len(),
-        "wrong number of arguments for `{did:?}`: expected `{count}`, found {}\n\
+    #[expect(
+        clippy::manual_assert_eq,
+        reason = "the message contains `assert_eq!`-like formatting itself"
+    )]
+    {
+        assert!(
+            count == args.len(),
+            "wrong number of arguments for `{did:?}`: expected `{count}`, found {}\n\
             note: the expected arguments are: `[{}]`\n\
             the given arguments are: `{args:#?}`",
-        args.len(),
-        params.clone().map(ty::GenericParamDefKind::descr).format(", "),
-    );
+            args.len(),
+            params.clone().map(ty::GenericParamDefKind::descr).format(", "),
+        );
+    }
 
     if let Some((idx, (param, arg))) =
         params

lintcheck/src/output.rs

@@ -228,8 +228,8 @@ fn print_stats(old_stats: HashMap<String, usize>, new_stats: HashMap<&String, us
 
     // remove duplicates from both hashmaps
     for (k, v) in &same_in_both_hashmaps {
-        assert!(old_stats_deduped.remove(k) == Some(*v));
-        assert!(new_stats_deduped.remove(k) == Some(*v));
+        assert_eq!(old_stats_deduped.remove(k), Some(*v));
+        assert_eq!(new_stats_deduped.remove(k), Some(*v));
     }
 
     println!("\nStats:");

tests/ui/assertions_on_constants.rs

@@ -54,8 +54,11 @@ fn main() {
     const _: () = assert!(true);
     //~^ assertions_on_constants
 
-    assert!(8 == (7 + 1));
-    //~^ assertions_on_constants
+    #[allow(clippy::manual_assert_eq, reason = "tests `assert!` specifically")]
+    {
+        assert!(8 == (7 + 1));
+        //~^ assertions_on_constants
+    }
 
     // Don't lint if the value is dependent on a defined constant:
     const N: usize = 1024;
@@ -68,8 +71,11 @@ const _: () = {
     assert!(true);
     //~^ assertions_on_constants
 
-    assert!(8 == (7 + 1));
-    //~^ assertions_on_constants
+    #[allow(clippy::manual_assert_eq, reason = "tests `assert!` specifically")]
+    {
+        assert!(8 == (7 + 1));
+        //~^ assertions_on_constants
+    }
 
     assert!(C);
 };

tests/ui/assertions_on_constants.stderr

@@ -89,47 +89,47 @@ LL |     const _: () = assert!(true);
    = help: remove the assertion
 
 error: this assertion is always `true`
-  --> tests/ui/assertions_on_constants.rs:57:5
+  --> tests/ui/assertions_on_constants.rs:59:9
    |
-LL |     assert!(8 == (7 + 1));
-   |     ^^^^^^^^^^^^^^^^^^^^^
+LL |         assert!(8 == (7 + 1));
+   |         ^^^^^^^^^^^^^^^^^^^^^
    |
    = help: remove the assertion
 
 error: this assertion is always `true`
-  --> tests/ui/assertions_on_constants.rs:68:5
+  --> tests/ui/assertions_on_constants.rs:71:5
    |
 LL |     assert!(true);
    |     ^^^^^^^^^^^^^
    |
    = help: remove the assertion
 
 error: this assertion is always `true`
-  --> tests/ui/assertions_on_constants.rs:71:5
+  --> tests/ui/assertions_on_constants.rs:76:9
    |
-LL |     assert!(8 == (7 + 1));
-   |     ^^^^^^^^^^^^^^^^^^^^^
+LL |         assert!(8 == (7 + 1));
+   |         ^^^^^^^^^^^^^^^^^^^^^
    |
    = help: remove the assertion
 
 error: this assertion has a constant value
-  --> tests/ui/assertions_on_constants.rs:79:5
+  --> tests/ui/assertions_on_constants.rs:85:5
    |
 LL |     assert!(C);
    |     ^^^^^^^^^^
    |
    = help: consider moving this to an anonymous constant: `const _: () = { assert!(..); }`
 
 error: this assertion has a constant value
-  --> tests/ui/assertions_on_constants.rs:90:5
+  --> tests/ui/assertions_on_constants.rs:96:5
    |
 LL |     assert!(C);
    |     ^^^^^^^^^^
    |
    = help: consider moving this into a const block: `const { assert!(..) }`
 
 error: this assertion has a constant value
-  --> tests/ui/assertions_on_constants.rs:96:5
+  --> tests/ui/assertions_on_constants.rs:102:5
    |
 LL |     assert!(C);
    |     ^^^^^^^^^^

tests/ui/cmp_null.fixed

@@ -33,6 +33,10 @@ fn main() {
     //~^ cmp_null
 }
 
+#[allow(
+    clippy::manual_assert_eq,
+    reason = "the lint only works on `assert`, not `assert_eq`"
+)]
 fn issue15010() {
     let f: *mut i32 = std::ptr::null_mut();
     debug_assert!(!f.is_null());

tests/ui/cmp_null.rs

@@ -33,6 +33,10 @@ fn main() {
     //~^ cmp_null
 }
 
+#[allow(
+    clippy::manual_assert_eq,
+    reason = "the lint only works on `assert`, not `assert_eq`"
+)]
 fn issue15010() {
     let f: *mut i32 = std::ptr::null_mut();
     debug_assert!(f != std::ptr::null_mut());

tests/ui/cmp_null.stderr

@@ -32,7 +32,7 @@ LL |     let _ = x as *const () == ptr::null();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(x as *const ()).is_null()`
 
 error: comparing with null is better expressed by the `.is_null()` method
-  --> tests/ui/cmp_null.rs:38:19
+  --> tests/ui/cmp_null.rs:42:19
    |
 LL |     debug_assert!(f != std::ptr::null_mut());
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!f.is_null()`

tests/ui/infinite_loops.rs

@@ -294,9 +294,7 @@ fn panic_like_macros_1() {
     }
 }
 
-fn panic_like_macros_2() {
-    let mut x = 0;
-
+fn panic_like_macros_2(mut x: i32) {
     loop {
         do_something();
         if true {
@@ -310,7 +308,7 @@ fn panic_like_macros_2() {
     }
     loop {
         do_something();
-        assert!(x % 2 == 0);
+        assert!(x.is_positive());
     }
     loop {
         do_something();