feat: add const_is_empty lint

This commit is contained in:
Samuel Tardieu 2024-02-19 23:30:17 +01:00
parent aa2c94e416
commit dbfbd0e77f
6 changed files with 252 additions and 1 deletions

View file

@ -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 [`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_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 [`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 [`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 [`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 [`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def

View file

@ -350,6 +350,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::CLONE_ON_COPY_INFO, crate::methods::CLONE_ON_COPY_INFO,
crate::methods::CLONE_ON_REF_PTR_INFO, crate::methods::CLONE_ON_REF_PTR_INFO,
crate::methods::COLLAPSIBLE_STR_REPLACE_INFO, crate::methods::COLLAPSIBLE_STR_REPLACE_INFO,
crate::methods::CONST_IS_EMPTY_INFO,
crate::methods::DRAIN_COLLECT_INFO, crate::methods::DRAIN_COLLECT_INFO,
crate::methods::ERR_EXPECT_INFO, crate::methods::ERR_EXPECT_INFO,
crate::methods::EXPECT_FUN_CALL_INFO, crate::methods::EXPECT_FUN_CALL_INFO,

View file

@ -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
}
}

View file

@ -36,6 +36,7 @@ mod inefficient_to_string;
mod inspect_for_each; mod inspect_for_each;
mod into_iter_on_ref; mod into_iter_on_ref;
mod is_digit_ascii_radix; mod is_digit_ascii_radix;
mod is_empty;
mod iter_cloned_collect; mod iter_cloned_collect;
mod iter_count; mod iter_count;
mod iter_filter; mod iter_filter;
@ -4041,6 +4042,31 @@ declare_clippy_lint! {
"calling `.get().is_some()` or `.get().is_none()` instead of `.contains()` or `.contains_key()`" "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 { pub struct Methods {
avoid_breaking_exported_api: bool, avoid_breaking_exported_api: bool,
msrv: Msrv, msrv: Msrv,
@ -4089,6 +4115,7 @@ impl_lint_pass!(Methods => [
CLONE_ON_COPY, CLONE_ON_COPY,
CLONE_ON_REF_PTR, CLONE_ON_REF_PTR,
COLLAPSIBLE_STR_REPLACE, COLLAPSIBLE_STR_REPLACE,
CONST_IS_EMPTY,
ITER_OVEREAGER_CLONED, ITER_OVEREAGER_CLONED,
CLONED_INSTEAD_OF_COPIED, CLONED_INSTEAD_OF_COPIED,
FLAT_MAP_OPTION, FLAT_MAP_OPTION,
@ -4442,7 +4469,7 @@ impl Methods {
("as_deref" | "as_deref_mut", []) => { ("as_deref" | "as_deref_mut", []) => {
needless_option_as_deref::check(cx, expr, recv, name); 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) { if let Some(("as_str", recv, [], as_str_span, _)) = method_call(recv) {
redundant_as_str::check(cx, expr, recv, as_str_span, span); redundant_as_str::check(cx, expr, recv, as_str_span, span);
} }
@ -4616,6 +4643,12 @@ impl Methods {
("hash", [arg]) => { ("hash", [arg]) => {
unit_hash::check(cx, expr, recv, 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_file", []) => filetype_is_file::check(cx, expr, recv),
("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, &self.msrv), ("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), ("is_none", []) => check_is_some_is_none(cx, expr, recv, call_span, false),

View file

@ -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
}

View file

@ -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