Auto merge of #8660 - yoav-lavi:squashed-master, r=flip1995

`unnecessary_owned_empty_strings`

[`unnecessary_owned_empty_strings`]

Fixes #8650

- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints

changelog: Adds `unnecessary_owned_empty_strings`, a lint that detects passing owned empty strings to a function expecting `&str`

Author: bors

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

clippy_lints/src/lib.register_all.rs

@@ -310,6 +310,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     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),

clippy_lints/src/lib.register_lints.rs

@@ -523,6 +523,7 @@ store.register_lints(&[
     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,

clippy_lints/src/lib.register_style.rs

@@ -106,6 +106,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     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),

clippy_lints/src/lib.rs

@@ -383,6 +383,7 @@ mod unit_hash;
 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`
 }
 

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,
+                            );
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

clippy_utils/src/paths.rs

@@ -148,6 +148,7 @@ pub const CONVERT_IDENTITY: [&str; 3] = ["core", "convert", "identity"];
 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", "<impl str>", "ends_with"];
 pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"];
 pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"];

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());
+}

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());
+}

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
+