about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2024-02-19 23:30:17 +0100
committerSamuel Tardieu <sam@rfc1149.net>2024-02-26 08:51:28 +0100
commitdbfbd0e77fd12a46ac81d0ec6fca4012aad0ea2d (patch)
tree0327006424071a3033d6b352f7359e72ccec08a2
parentaa2c94e416d346341fe3a875160a7d064596f4d3 (diff)
downloadrust-dbfbd0e77fd12a46ac81d0ec6fca4012aad0ea2d.tar.gz
rust-dbfbd0e77fd12a46ac81d0ec6fca4012aad0ea2d.zip
feat: add `const_is_empty` lint
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/methods/is_empty.rs40
-rw-r--r--clippy_lints/src/methods/mod.rs35
-rw-r--r--tests/ui/const_is_empty.rs52
-rw-r--r--tests/ui/const_is_empty.stderr124
6 files changed, 252 insertions, 1 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 26dce1f9265..9182e61f6d6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5109,6 +5109,7 @@ Released 2018-09-13
 [`collection_is_never_read`]: https://rust-lang.github.io/rust-clippy/master/index.html#collection_is_never_read
 [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
 [`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
+[`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
 [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
 [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
 [`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index b6a35bb3e03..fe9bc77ce55 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -350,6 +350,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::CLONE_ON_COPY_INFO,
     crate::methods::CLONE_ON_REF_PTR_INFO,
     crate::methods::COLLAPSIBLE_STR_REPLACE_INFO,
+    crate::methods::CONST_IS_EMPTY_INFO,
     crate::methods::DRAIN_COLLECT_INFO,
     crate::methods::ERR_EXPECT_INFO,
     crate::methods::EXPECT_FUN_CALL_INFO,
diff --git a/clippy_lints/src/methods/is_empty.rs b/clippy_lints/src/methods/is_empty.rs
new file mode 100644
index 00000000000..4713c99d33b
--- /dev/null
+++ b/clippy_lints/src/methods/is_empty.rs
@@ -0,0 +1,40 @@
+use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::expr_or_init;
+use rustc_ast::LitKind;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_span::source_map::Spanned;
+
+use super::CONST_IS_EMPTY;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, receiver: &Expr<'_>) {
+    if in_external_macro(cx.sess(), expr.span) || !receiver.span.eq_ctxt(expr.span) {
+        return;
+    }
+    let init_expr = expr_or_init(cx, receiver);
+    if let Some(init_is_empty) = is_empty(init_expr)
+        && init_expr.span.eq_ctxt(receiver.span)
+    {
+        span_lint_and_note(
+            cx,
+            CONST_IS_EMPTY,
+            expr.span,
+            &format!("this expression always evaluates to {init_is_empty:?}"),
+            Some(init_expr.span),
+            "because its initialization value is constant",
+        );
+    }
+}
+
+fn is_empty(expr: &'_ rustc_hir::Expr<'_>) -> Option<bool> {
+    if let ExprKind::Lit(Spanned { node, .. }) = expr.kind {
+        match node {
+            LitKind::Str(sym, _) => Some(sym.is_empty()),
+            LitKind::ByteStr(value, _) | LitKind::CStr(value, _) => Some(value.is_empty()),
+            _ => None,
+        }
+    } else {
+        None
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 820f4c85890..b6894971acc 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -36,6 +36,7 @@ mod inefficient_to_string;
 mod inspect_for_each;
 mod into_iter_on_ref;
 mod is_digit_ascii_radix;
+mod is_empty;
 mod iter_cloned_collect;
 mod iter_count;
 mod iter_filter;
@@ -4041,6 +4042,31 @@ declare_clippy_lint! {
     "calling `.get().is_some()` or `.get().is_none()` instead of `.contains()` or `.contains_key()`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// It identifies calls to `.is_empty()` on constant values.
+    ///
+    /// ### Why is this bad?
+    /// String literals and constant values are known at compile time. Checking if they
+    /// are empty will always return the same value. This might not be the intention of
+    /// the expression.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let value = "";
+    /// if value.is_empty() {
+    ///     println!("the string is empty");
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// println!("the string is empty");
+    /// ```
+    #[clippy::version = "1.78.0"]
+    pub CONST_IS_EMPTY,
+    suspicious,
+    "is_empty() called on strings known at compile time"
+}
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4089,6 +4115,7 @@ impl_lint_pass!(Methods => [
     CLONE_ON_COPY,
     CLONE_ON_REF_PTR,
     COLLAPSIBLE_STR_REPLACE,
+    CONST_IS_EMPTY,
     ITER_OVEREAGER_CLONED,
     CLONED_INSTEAD_OF_COPIED,
     FLAT_MAP_OPTION,
@@ -4442,7 +4469,7 @@ impl Methods {
                 ("as_deref" | "as_deref_mut", []) => {
                     needless_option_as_deref::check(cx, expr, recv, name);
                 },
-                ("as_bytes" | "is_empty", []) => {
+                ("as_bytes", []) => {
                     if let Some(("as_str", recv, [], as_str_span, _)) = method_call(recv) {
                         redundant_as_str::check(cx, expr, recv, as_str_span, span);
                     }
@@ -4616,6 +4643,12 @@ impl Methods {
                 ("hash", [arg]) => {
                     unit_hash::check(cx, expr, recv, arg);
                 },
+                ("is_empty", []) => {
+                    if let Some(("as_str", recv, [], as_str_span, _)) = method_call(recv) {
+                        redundant_as_str::check(cx, expr, recv, as_str_span, span);
+                    }
+                    is_empty::check(cx, expr, recv);
+                },
                 ("is_file", []) => filetype_is_file::check(cx, expr, recv),
                 ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, &self.msrv),
                 ("is_none", []) => check_is_some_is_none(cx, expr, recv, call_span, false),
diff --git a/tests/ui/const_is_empty.rs b/tests/ui/const_is_empty.rs
new file mode 100644
index 00000000000..d61cdbb0424
--- /dev/null
+++ b/tests/ui/const_is_empty.rs
@@ -0,0 +1,52 @@
+#![warn(clippy::const_is_empty)]
+
+fn test_literal() {
+    if "".is_empty() {
+        //~^ERROR: this expression always evaluates to true
+    }
+    if "foobar".is_empty() {
+        //~^ERROR: this expression always evaluates to false
+    }
+}
+
+fn test_byte_literal() {
+    if b"".is_empty() {
+        //~^ERROR: this expression always evaluates to true
+    }
+    if b"foobar".is_empty() {
+        //~^ERROR: this expression always evaluates to false
+    }
+}
+
+fn test_no_mut() {
+    let mut empty = "";
+    if empty.is_empty() {
+        // No lint because it is mutable
+    }
+}
+
+fn test_propagated() {
+    let empty = "";
+    let non_empty = "foobar";
+    let empty2 = empty;
+    let non_empty2 = non_empty;
+    if empty2.is_empty() {
+        //~^ERROR: this expression always evaluates to true
+    }
+    if non_empty2.is_empty() {
+        //~^ERROR: this expression always evaluates to false
+    }
+}
+
+fn main() {
+    let value = "foobar";
+    let _ = value.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let x = value;
+    let _ = x.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let _ = "".is_empty();
+    //~^ ERROR: this expression always evaluates to true
+    let _ = b"".is_empty();
+    //~^ ERROR: this expression always evaluates to true
+}
diff --git a/tests/ui/const_is_empty.stderr b/tests/ui/const_is_empty.stderr
new file mode 100644
index 00000000000..5a89e686242
--- /dev/null
+++ b/tests/ui/const_is_empty.stderr
@@ -0,0 +1,124 @@
+error: this expression always evaluates to true
+  --> tests/ui/const_is_empty.rs:4:8
+   |
+LL |     if "".is_empty() {
+   |        ^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:4:8
+   |
+LL |     if "".is_empty() {
+   |        ^^
+   = note: `-D clippy::const-is-empty` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::const_is_empty)]`
+
+error: this expression always evaluates to false
+  --> tests/ui/const_is_empty.rs:7:8
+   |
+LL |     if "foobar".is_empty() {
+   |        ^^^^^^^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:7:8
+   |
+LL |     if "foobar".is_empty() {
+   |        ^^^^^^^^
+
+error: this expression always evaluates to true
+  --> tests/ui/const_is_empty.rs:13:8
+   |
+LL |     if b"".is_empty() {
+   |        ^^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:13:8
+   |
+LL |     if b"".is_empty() {
+   |        ^^^
+
+error: this expression always evaluates to false
+  --> tests/ui/const_is_empty.rs:16:8
+   |
+LL |     if b"foobar".is_empty() {
+   |        ^^^^^^^^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:16:8
+   |
+LL |     if b"foobar".is_empty() {
+   |        ^^^^^^^^^
+
+error: this expression always evaluates to true
+  --> tests/ui/const_is_empty.rs:33:8
+   |
+LL |     if empty2.is_empty() {
+   |        ^^^^^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:29:17
+   |
+LL |     let empty = "";
+   |                 ^^
+
+error: this expression always evaluates to false
+  --> tests/ui/const_is_empty.rs:36:8
+   |
+LL |     if non_empty2.is_empty() {
+   |        ^^^^^^^^^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:30:21
+   |
+LL |     let non_empty = "foobar";
+   |                     ^^^^^^^^
+
+error: this expression always evaluates to false
+  --> tests/ui/const_is_empty.rs:43:13
+   |
+LL |     let _ = value.is_empty();
+   |             ^^^^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:42:17
+   |
+LL |     let value = "foobar";
+   |                 ^^^^^^^^
+
+error: this expression always evaluates to false
+  --> tests/ui/const_is_empty.rs:46:13
+   |
+LL |     let _ = x.is_empty();
+   |             ^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:42:17
+   |
+LL |     let value = "foobar";
+   |                 ^^^^^^^^
+
+error: this expression always evaluates to true
+  --> tests/ui/const_is_empty.rs:48:13
+   |
+LL |     let _ = "".is_empty();
+   |             ^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:48:13
+   |
+LL |     let _ = "".is_empty();
+   |             ^^
+
+error: this expression always evaluates to true
+  --> tests/ui/const_is_empty.rs:50:13
+   |
+LL |     let _ = b"".is_empty();
+   |             ^^^^^^^^^^^^^^
+   |
+note: because its initialization value is constant
+  --> tests/ui/const_is_empty.rs:50:13
+   |
+LL |     let _ = b"".is_empty();
+   |             ^^^
+
+error: aborting due to 10 previous errors
+