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 @@ 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 { + 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 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 @@ "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 @@ pub fn new( 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 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { ("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 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { ("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 +