Add ASSERTIONS_ON_COLLECTION_EMPTINESS lint

CHANGELOG.md

@@ -6639,6 +6639,7 @@ Released 2018-09-13
 [`as_pointer_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_pointer_underscore
 [`as_ptr_cast_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut
 [`as_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore
+[`assert_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#assert_is_empty
 [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
 [`assertions_on_result_states`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_result_states
 [`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern

clippy_lints/src/assert_is_empty.rs

@@ -0,0 +1,105 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::macros::{find_assert_args, first_node_macro_backtrace};
+use clippy_utils::res::MaybeDef;
+use clippy_utils::source::snippet_with_context;
+use clippy_utils::sym;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, LangItem, UnOp};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::Ty;
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `assert!(s.is_empty())` and `assert!(!s.is_empty())`.
+    ///
+    /// ### Why is this bad?
+    /// `assert!` only reports the condition was false, not what the collection
+    /// contained. Using `assert_eq!` / `assert_ne!` with an empty literal
+    /// prints the collection contents on failure, making debugging faster.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # let items = vec![1, 2, 3];
+    /// assert!(items.is_empty());
+    /// assert!(!items.is_empty());
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// # let items = vec![1, 2, 3];
+    /// assert_eq!(items, []);
+    /// assert_ne!(items, []);
+    /// ```
+    #[clippy::version = "1.97.0"]
+    pub ASSERT_IS_EMPTY,
+    pedantic,
+    "asserting on `.is_empty()` without showing the collection contents"
+}
+
+declare_lint_pass!(AssertIsEmpty => [ASSERT_IS_EMPTY]);
+
+impl<'tcx> LateLintPass<'tcx> for AssertIsEmpty {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
+        if !matches!(e.kind, ExprKind::If(..)) {
+            return;
+        }
+
+        if let Some(macro_call) = first_node_macro_backtrace(cx, e).find(|macro_call| {
+            matches!(
+                cx.tcx.get_diagnostic_name(macro_call.def_id),
+                Some(sym::assert_macro | sym::debug_assert_macro),
+            )
+        }) && matches!(
+            cx.tcx.get_diagnostic_name(macro_call.def_id),
+            Some(sym::assert_macro | sym::debug_assert_macro),
+        ) && let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn)
+            && panic_expn.is_default_message()
+        {
+            let (method_name, recv, is_negated) = match condition.kind {
+                ExprKind::MethodCall(ms, r, [], _) => (ms.ident.name, r, false),
+                ExprKind::Unary(UnOp::Not, inner) if let ExprKind::MethodCall(ms, r, [], _) = inner.kind => {
+                    (ms.ident.name, r, true)
+                },
+                _ => return,
+            };
+
+            if method_name != sym::is_empty {
+                return;
+            }
+
+            let message = if is_negated {
+                "used `assert!` with a non-empty collection check"
+            } else {
+                "used `assert!` with an empty collection check"
+            };
+
+            let recv_ty = cx.typeck_results().expr_ty(recv);
+            let empty_literal = empty_literal_for_type(cx, recv_ty);
+
+            span_lint_and_then(cx, ASSERT_IS_EMPTY, macro_call.span, message, |diag| {
+                let mut app = Applicability::MachineApplicable;
+                let recv_snippet = snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0;
+
+                let sugg = if is_negated {
+                    format!("assert_ne!({recv_snippet}, {empty_literal})")
+                } else {
+                    format!("assert_eq!({recv_snippet}, {empty_literal})")
+                };
+                if macro_call.span.from_expansion() {
+                    diag.help(format!("replace with: `{sugg}`"));
+                } else {
+                    diag.span_suggestion(macro_call.span, "replace with", sugg, app);
+                }
+            });
+        }
+    }
+}
+
+fn empty_literal_for_type(cx: &LateContext<'_>, ty: Ty<'_>) -> &'static str {
+    if ty.is_str() || ty.peel_refs().is_str() || ty.is_lang_item(cx, LangItem::String) {
+        "\"\""
+    } else {
+        "[]"
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -11,6 +11,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::as_conversions::AS_CONVERSIONS_INFO,
     crate::asm_syntax::INLINE_ASM_X86_ATT_SYNTAX_INFO,
     crate::asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX_INFO,
+    crate::assert_is_empty::ASSERT_IS_EMPTY_INFO,
     crate::assertions_on_constants::ASSERTIONS_ON_CONSTANTS_INFO,
     crate::assertions_on_result_states::ASSERTIONS_ON_RESULT_STATES_INFO,
     crate::assigning_clones::ASSIGNING_CLONES_INFO,

clippy_lints/src/lib.rs

@@ -69,6 +69,7 @@ mod arbitrary_source_item_ordering;
 mod arc_with_non_send_sync;
 mod as_conversions;
 mod asm_syntax;
+mod assert_is_empty;
 mod assertions_on_constants;
 mod assertions_on_result_states;
 mod assigning_clones;
@@ -665,6 +666,7 @@ rustc_lint::late_lint_methods!(
         RedundantClone: redundant_clone::RedundantClone = redundant_clone::RedundantClone,
         SlowVectorInit: slow_vector_initialization::SlowVectorInit = slow_vector_initialization::SlowVectorInit,
         UnnecessaryWraps: unnecessary_wraps::UnnecessaryWraps = unnecessary_wraps::UnnecessaryWraps::new(conf),
+        AssertIsEmpty: assert_is_empty::AssertIsEmpty = assert_is_empty::AssertIsEmpty,
         AssertionsOnConstants: assertions_on_constants::AssertionsOnConstants = assertions_on_constants::AssertionsOnConstants::new(conf),
         AssertionsOnResultStates: assertions_on_result_states::AssertionsOnResultStates = assertions_on_result_states::AssertionsOnResultStates,
         InherentToString: inherent_to_string::InherentToString = inherent_to_string::InherentToString,

tests/lint_message_convention.rs

@@ -6,7 +6,7 @@ use std::sync::LazyLock;
 
 use regex::RegexSet;
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 struct Message {
     path: PathBuf,
     bad_lines: Vec<String>,
@@ -113,5 +113,5 @@ fn lint_message_convention() {
     eprintln!("Check out the rustc-dev-guide for more information:");
     eprintln!("https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-structure\n\n\n");
 
-    assert!(bad_tests.is_empty());
+    assert_eq!(bad_tests, []);
 }

tests/ui/assert_is_empty.fixed

@@ -0,0 +1,29 @@
+#![warn(clippy::assert_is_empty)]
+#![allow(clippy::len_zero, clippy::comparison_to_empty)]
+
+fn main() {
+    // Test with Vec
+    let v: Vec<i32> = vec![];
+    assert_eq!(v, []);
+    //~^ assert_is_empty
+    assert_ne!(v, []);
+    //~^ assert_is_empty
+
+    // Test with String
+    let s = String::new();
+    assert_eq!(s, "");
+    //~^ assert_is_empty
+    assert_ne!(s, "");
+    //~^ assert_is_empty
+
+    // Should not lint: custom message
+    assert!(v.is_empty(), "vec is not empty");
+    assert!(!v.is_empty(), "vec is empty");
+
+    // Should not lint: assert_ne!/assert_eq! already fine
+    assert_eq!(v, []);
+    assert_ne!(v, []);
+
+    // Should not lint: not is_empty
+    assert!(v.len() == 0);
+}

tests/ui/assert_is_empty.rs

@@ -0,0 +1,29 @@
+#![warn(clippy::assert_is_empty)]
+#![allow(clippy::len_zero, clippy::comparison_to_empty)]
+
+fn main() {
+    // Test with Vec
+    let v: Vec<i32> = vec![];
+    assert!(v.is_empty());
+    //~^ assert_is_empty
+    assert!(!v.is_empty());
+    //~^ assert_is_empty
+
+    // Test with String
+    let s = String::new();
+    assert!(s.is_empty());
+    //~^ assert_is_empty
+    assert!(!s.is_empty());
+    //~^ assert_is_empty
+
+    // Should not lint: custom message
+    assert!(v.is_empty(), "vec is not empty");
+    assert!(!v.is_empty(), "vec is empty");
+
+    // Should not lint: assert_ne!/assert_eq! already fine
+    assert_eq!(v, []);
+    assert_ne!(v, []);
+
+    // Should not lint: not is_empty
+    assert!(v.len() == 0);
+}

tests/ui/assert_is_empty.stderr

@@ -0,0 +1,29 @@
+error: used `assert!` with an empty collection check
+  --> tests/ui/assert_is_empty.rs:7:5
+   |
+LL |     assert!(v.is_empty());
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: replace with: `assert_eq!(v, [])`
+   |
+   = note: `-D clippy::assert-is-empty` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::assert_is_empty)]`
+
+error: used `assert!` with a non-empty collection check
+  --> tests/ui/assert_is_empty.rs:9:5
+   |
+LL |     assert!(!v.is_empty());
+   |     ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `assert_ne!(v, [])`
+
+error: used `assert!` with an empty collection check
+  --> tests/ui/assert_is_empty.rs:14:5
+   |
+LL |     assert!(s.is_empty());
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: replace with: `assert_eq!(s, "")`
+
+error: used `assert!` with a non-empty collection check
+  --> tests/ui/assert_is_empty.rs:16:5
+   |
+LL |     assert!(!s.is_empty());
+   |     ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `assert_ne!(s, "")`
+
+error: aborting due to 4 previous errors
+

tests/ui/assert_is_empty_macro.rs

@@ -0,0 +1,24 @@
+//@no-rustfix: macro-expanded suggestions are intentionally help-only
+#![warn(clippy::assert_is_empty)]
+
+macro_rules! assert_empty {
+    ($expr:expr) => {
+        assert!($expr.is_empty());
+        //~^ assert_is_empty
+    };
+}
+
+macro_rules! assert_not_empty {
+    ($expr:expr) => {
+        assert!(!$expr.is_empty());
+        //~^ assert_is_empty
+    };
+}
+
+fn main() {
+    let v: Vec<i32> = vec![];
+    let s = String::new();
+
+    assert_empty!(v);
+    assert_not_empty!(s);
+}

tests/ui/assert_is_empty_macro.stderr

@@ -0,0 +1,28 @@
+error: used `assert!` with an empty collection check
+  --> tests/ui/assert_is_empty_macro.rs:6:9
+   |
+LL |         assert!($expr.is_empty());
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |     assert_empty!(v);
+   |     ---------------- in this macro invocation
+   |
+   = help: replace with: `assert_eq!(v, [])`
+   = note: `-D clippy::assert-is-empty` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::assert_is_empty)]`
+   = note: this error originates in the macro `assert_empty` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: used `assert!` with a non-empty collection check
+  --> tests/ui/assert_is_empty_macro.rs:13:9
+   |
+LL |         assert!(!$expr.is_empty());
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |     assert_not_empty!(s);
+   |     -------------------- in this macro invocation
+   |
+   = help: replace with: `assert_ne!(s, "")`
+   = note: this error originates in the macro `assert_not_empty` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 2 previous errors
+