From a4d1837f07f65e4c440646799a0730f1d9a5f798 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Thu, 7 Apr 2022 20:21:47 +0200 Subject: [PATCH] unnecessary_string_new --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + clippy_lints/src/lib.rs | 2 + .../src/unnecessary_owned_empty_string.rs | 81 +++++++++++++++++++ clippy_utils/src/paths.rs | 1 + tests/ui/unnecessary_owned_empty_string.fixed | 22 +++++ tests/ui/unnecessary_owned_empty_string.rs | 22 +++++ .../ui/unnecessary_owned_empty_string.stderr | 16 ++++ 10 files changed, 148 insertions(+) create mode 100644 clippy_lints/src/unnecessary_owned_empty_string.rs create mode 100644 tests/ui/unnecessary_owned_empty_string.fixed create mode 100644 tests/ui/unnecessary_owned_empty_string.rs create mode 100644 tests/ui/unnecessary_owned_empty_string.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b4097ea86a5..e48413dc30c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3650,6 +3650,7 @@ Released 2018-09-13 [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation +[`unnecessary_owned_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_string [`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by [`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 14ca93b5f3c..b601e37a4c8 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -310,6 +310,7 @@ LintId::of(unit_types::UNIT_CMP), LintId::of(unnamed_address::FN_ADDRESS_COMPARISONS), LintId::of(unnamed_address::VTABLE_ADDRESS_COMPARISONS), + LintId::of(unnecessary_owned_empty_string::UNNECESSARY_OWNED_EMPTY_STRING), LintId::of(unnecessary_sort_by::UNNECESSARY_SORT_BY), LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(unused_io_amount::UNUSED_IO_AMOUNT), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 532590aaa5a..0a23e60fd2c 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -523,6 +523,7 @@ unit_types::UNIT_CMP, unnamed_address::FN_ADDRESS_COMPARISONS, unnamed_address::VTABLE_ADDRESS_COMPARISONS, + unnecessary_owned_empty_string::UNNECESSARY_OWNED_EMPTY_STRING, unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS, unnecessary_sort_by::UNNECESSARY_SORT_BY, unnecessary_wraps::UNNECESSARY_WRAPS, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index 3114afac886..b014c11f0f4 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -106,6 +106,7 @@ LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS), LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS), LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME), + LintId::of(unnecessary_owned_empty_string::UNNECESSARY_OWNED_EMPTY_STRING), LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(unused_unit::UNUSED_UNIT), LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c9b836f9580..576a6611267 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -383,6 +383,7 @@ macro_rules! declare_clippy_lint { mod unit_return_expecting_ord; mod unit_types; mod unnamed_address; +mod unnecessary_owned_empty_string; mod unnecessary_self_imports; mod unnecessary_sort_by; mod unnecessary_wraps; @@ -868,6 +869,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: }); store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef)); store.register_early_pass(|| Box::new(empty_structs_with_brackets::EmptyStructsWithBrackets)); + store.register_late_pass(|| Box::new(unnecessary_owned_empty_string::UnnecessaryOwnedEmptyString)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/unnecessary_owned_empty_string.rs b/clippy_lints/src/unnecessary_owned_empty_string.rs new file mode 100644 index 00000000000..bb42c7816e7 --- /dev/null +++ b/clippy_lints/src/unnecessary_owned_empty_string.rs @@ -0,0 +1,81 @@ +use clippy_utils::{diagnostics::span_lint_and_sugg, ty::is_type_diagnostic_item}; +use clippy_utils::{match_def_path, paths}; +use if_chain::if_chain; +use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// + /// Detects cases of owned empty strings being passed as an argument to a function expecting `&str` + /// + /// ### Why is this bad? + /// + /// This results in longer and less readable code + /// + /// ### Example + /// ```rust + /// vec!["1", "2", "3"].join(&String::new()); + /// ``` + /// Use instead: + /// ```rust + /// vec!["1", "2", "3"].join(""); + /// ``` + #[clippy::version = "1.62.0"] + pub UNNECESSARY_OWNED_EMPTY_STRING, + style, + "detects cases of references to owned empty strings being passed as an argument to a function expecting `&str`" +} +declare_lint_pass!(UnnecessaryOwnedEmptyString => [UNNECESSARY_OWNED_EMPTY_STRING]); + +impl<'tcx> LateLintPass<'tcx> for UnnecessaryOwnedEmptyString { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if_chain! { + if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner_expr) = expr.kind; + if let ExprKind::Call(fun, args) = inner_expr.kind; + if let ExprKind::Path(ref qpath) = fun.kind; + if let Some(fun_def_id) = cx.qpath_res(qpath, fun.hir_id).opt_def_id(); + if let ty::Ref(_, inner_str, _) = cx.typeck_results().expr_ty_adjusted(expr).kind(); + if inner_str.is_str(); + then { + if match_def_path(cx, fun_def_id, &paths::STRING_NEW) { + span_lint_and_sugg( + cx, + UNNECESSARY_OWNED_EMPTY_STRING, + expr.span, + "usage of `&String::new()` for a function expecting a `&str` argument", + "try", + "\"\"".to_owned(), + Applicability::MachineApplicable, + ); + } else { + if_chain! { + if match_def_path(cx, fun_def_id, &paths::FROM_FROM); + if let [.., last_arg] = args; + if let ExprKind::Lit(spanned) = &last_arg.kind; + if let LitKind::Str(symbol, _) = spanned.node; + if symbol.is_empty(); + let inner_expr_type = cx.typeck_results().expr_ty(inner_expr); + if is_type_diagnostic_item(cx, inner_expr_type, sym::String); + then { + span_lint_and_sugg( + cx, + UNNECESSARY_OWNED_EMPTY_STRING, + expr.span, + "usage of `&String::from(\"\")` for a function expecting a `&str` argument", + "try", + "\"\"".to_owned(), + Applicability::MachineApplicable, + ); + } + } + } + } + } + } +} diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 79e6e92dc0a..deb548daf2d 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -148,6 +148,7 @@ pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"]; pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"]; pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"]; +pub const STRING_NEW: [&str; 4] = ["alloc", "string", "String", "new"]; pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "", "ends_with"]; pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"]; pub const STR_LEN: [&str; 4] = ["core", "str", "", "len"]; diff --git a/tests/ui/unnecessary_owned_empty_string.fixed b/tests/ui/unnecessary_owned_empty_string.fixed new file mode 100644 index 00000000000..cd69e956fd4 --- /dev/null +++ b/tests/ui/unnecessary_owned_empty_string.fixed @@ -0,0 +1,22 @@ +// run-rustfix + +#![warn(clippy::unnecessary_owned_empty_string)] + +fn ref_str_argument(_value: &str) {} + +#[allow(clippy::ptr_arg)] +fn ref_string_argument(_value: &String) {} + +fn main() { + // should be linted + ref_str_argument(""); + + // should be linted + ref_str_argument(""); + + // should not be linted + ref_str_argument(""); + + // should not be linted + ref_string_argument(&String::new()); +} diff --git a/tests/ui/unnecessary_owned_empty_string.rs b/tests/ui/unnecessary_owned_empty_string.rs new file mode 100644 index 00000000000..3fbba156240 --- /dev/null +++ b/tests/ui/unnecessary_owned_empty_string.rs @@ -0,0 +1,22 @@ +// run-rustfix + +#![warn(clippy::unnecessary_owned_empty_string)] + +fn ref_str_argument(_value: &str) {} + +#[allow(clippy::ptr_arg)] +fn ref_string_argument(_value: &String) {} + +fn main() { + // should be linted + ref_str_argument(&String::new()); + + // should be linted + ref_str_argument(&String::from("")); + + // should not be linted + ref_str_argument(""); + + // should not be linted + ref_string_argument(&String::new()); +} diff --git a/tests/ui/unnecessary_owned_empty_string.stderr b/tests/ui/unnecessary_owned_empty_string.stderr new file mode 100644 index 00000000000..c32372290d1 --- /dev/null +++ b/tests/ui/unnecessary_owned_empty_string.stderr @@ -0,0 +1,16 @@ +error: usage of `&String::new()` for a function expecting a `&str` argument + --> $DIR/unnecessary_owned_empty_string.rs:12:22 + | +LL | ref_str_argument(&String::new()); + | ^^^^^^^^^^^^^^ help: try: `""` + | + = note: `-D clippy::unnecessary-owned-empty-string` implied by `-D warnings` + +error: usage of `&String::from("")` for a function expecting a `&str` argument + --> $DIR/unnecessary_owned_empty_string.rs:15:22 + | +LL | ref_str_argument(&String::from("")); + | ^^^^^^^^^^^^^^^^^ help: try: `""` + +error: aborting due to 2 previous errors + -- 2.44.0