diff --git a/CHANGELOG.md b/CHANGELOG.md index b4097ea86a5..44a36870108 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_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings [`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..02ba7835639 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_strings::UNNECESSARY_OWNED_EMPTY_STRINGS), 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..704e79885cf 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_strings::UNNECESSARY_OWNED_EMPTY_STRINGS, 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..f52fe97ed23 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_strings::UNNECESSARY_OWNED_EMPTY_STRINGS), 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..74ade422dc8 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_strings; 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_strings::UnnecessaryOwnedEmptyStrings)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/unnecessary_owned_empty_strings.rs b/clippy_lints/src/unnecessary_owned_empty_strings.rs new file mode 100644 index 00000000000..8a4f4c0ad97 --- /dev/null +++ b/clippy_lints/src/unnecessary_owned_empty_strings.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_STRINGS, + style, + "detects cases of references to owned empty strings being passed as an argument to a function expecting `&str`" +} +declare_lint_pass!(UnnecessaryOwnedEmptyStrings => [UNNECESSARY_OWNED_EMPTY_STRINGS]); + +impl<'tcx> LateLintPass<'tcx> for UnnecessaryOwnedEmptyStrings { + 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_STRINGS, + 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_STRINGS, + 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_strings.fixed b/tests/ui/unnecessary_owned_empty_strings.fixed new file mode 100644 index 00000000000..f95f91329a2 --- /dev/null +++ b/tests/ui/unnecessary_owned_empty_strings.fixed @@ -0,0 +1,22 @@ +// run-rustfix + +#![warn(clippy::unnecessary_owned_empty_strings)] + +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_strings.rs b/tests/ui/unnecessary_owned_empty_strings.rs new file mode 100644 index 00000000000..0cbdc151ed9 --- /dev/null +++ b/tests/ui/unnecessary_owned_empty_strings.rs @@ -0,0 +1,22 @@ +// run-rustfix + +#![warn(clippy::unnecessary_owned_empty_strings)] + +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_strings.stderr b/tests/ui/unnecessary_owned_empty_strings.stderr new file mode 100644 index 00000000000..46bc4597b33 --- /dev/null +++ b/tests/ui/unnecessary_owned_empty_strings.stderr @@ -0,0 +1,16 @@ +error: usage of `&String::new()` for a function expecting a `&str` argument + --> $DIR/unnecessary_owned_empty_strings.rs:12:22 + | +LL | ref_str_argument(&String::new()); + | ^^^^^^^^^^^^^^ help: try: `""` + | + = note: `-D clippy::unnecessary-owned-empty-strings` implied by `-D warnings` + +error: usage of `&String::from("")` for a function expecting a `&str` argument + --> $DIR/unnecessary_owned_empty_strings.rs:15:22 + | +LL | ref_str_argument(&String::from("")); + | ^^^^^^^^^^^^^^^^^ help: try: `""` + +error: aborting due to 2 previous errors +