Implement clippy::manual_empty_string_creation lint

Author: Guilherme-Vasconcelos

CHANGELOG.md

@@ -3828,6 +3828,7 @@ Released 2018-09-13
 [`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
 [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
+[`manual_empty_string_creation`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_empty_string_creation
 [`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
 [`manual_find`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find
 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map

clippy_lints/src/lib.register_all.rs

@@ -124,6 +124,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(main_recursion::MAIN_RECURSION),
     LintId::of(manual_async_fn::MANUAL_ASYNC_FN),
     LintId::of(manual_bits::MANUAL_BITS),
+    LintId::of(manual_empty_string_creation::MANUAL_EMPTY_STRING_CREATION),
     LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
     LintId::of(manual_rem_euclid::MANUAL_REM_EUCLID),
     LintId::of(manual_retain::MANUAL_RETAIN),

clippy_lints/src/lib.register_lints.rs

@@ -244,6 +244,7 @@ store.register_lints(&[
     manual_assert::MANUAL_ASSERT,
     manual_async_fn::MANUAL_ASYNC_FN,
     manual_bits::MANUAL_BITS,
+    manual_empty_string_creation::MANUAL_EMPTY_STRING_CREATION,
     manual_instant_elapsed::MANUAL_INSTANT_ELAPSED,
     manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
     manual_ok_or::MANUAL_OK_OR,

clippy_lints/src/lib.register_style.rs

@@ -44,6 +44,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(main_recursion::MAIN_RECURSION),
     LintId::of(manual_async_fn::MANUAL_ASYNC_FN),
     LintId::of(manual_bits::MANUAL_BITS),
+    LintId::of(manual_empty_string_creation::MANUAL_EMPTY_STRING_CREATION),
     LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
     LintId::of(map_clone::MAP_CLONE),
     LintId::of(match_result_ok::MATCH_RESULT_OK),

clippy_lints/src/lib.rs

@@ -273,6 +273,7 @@ mod main_recursion;
 mod manual_assert;
 mod manual_async_fn;
 mod manual_bits;
+mod manual_empty_string_creation;
 mod manual_instant_elapsed;
 mod manual_non_exhaustive;
 mod manual_ok_or;
@@ -933,6 +934,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
     store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
     store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone));
+    store.register_late_pass(|| Box::new(manual_empty_string_creation::ManualEmptyStringCreation));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/manual_empty_string_creation.rs

@@ -0,0 +1,141 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use rustc_ast::LitKind;
+use rustc_errors::Applicability::MachineApplicable;
+use rustc_hir::{Expr, ExprKind, PathSegment, QPath, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{sym, symbol, Span};
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for usage of `""` to create a `String`, such as `"".to_string()`, `"".to_owned()`,
+    /// `String::from("")` and others.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Different ways of creating an empty string makes your code less standardized, which can
+    /// be confusing.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let a = "".to_string();
+    /// let b: String = "".into();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let a = String::new();
+    /// let b = String::new();
+    /// ```
+    #[clippy::version = "1.65.0"]
+    pub MANUAL_EMPTY_STRING_CREATION,
+    style,
+    "empty String is being created manually"
+}
+declare_lint_pass!(ManualEmptyStringCreation => [MANUAL_EMPTY_STRING_CREATION]);
+
+impl LateLintPass<'_> for ManualEmptyStringCreation {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        if expr.span.from_expansion() {
+            return;
+        }
+
+        let ty = cx.typeck_results().expr_ty(expr);
+        match ty.kind() {
+            ty::Adt(adt_def, _) if adt_def.is_struct() => {
+                if !cx.tcx.is_diagnostic_item(sym::String, adt_def.did()) {
+                    return;
+                }
+            },
+            _ => return,
+        }
+
+        match expr.kind {
+            ExprKind::Call(func, args) => {
+                parse_call(cx, expr.span, func, args);
+            },
+            ExprKind::MethodCall(path_segment, args, _) => {
+                parse_method_call(cx, expr.span, path_segment, args);
+            },
+            _ => (),
+        }
+    }
+}
+
+/// Checks if an expression's kind corresponds to an empty &str.
+fn is_expr_kind_empty_str(expr_kind: &ExprKind<'_>) -> bool {
+    if  let ExprKind::Lit(lit) = expr_kind &&
+        let LitKind::Str(value, _) = lit.node &&
+        value == symbol::kw::Empty
+    {
+        return true;
+    }
+
+    false
+}
+
+/// Emits the `MANUAL_EMPTY_STRING_CREATION` warning and suggests the appropriate fix.
+fn warn_then_suggest(cx: &LateContext<'_>, span: Span) {
+    span_lint_and_sugg(
+        cx,
+        MANUAL_EMPTY_STRING_CREATION,
+        span,
+        "empty String is being created manually",
+        "consider using",
+        "String::new()".into(),
+        MachineApplicable,
+    );
+}
+
+/// Tries to parse an expression as a method call, emiting the warning if necessary.
+fn parse_method_call(cx: &LateContext<'_>, span: Span, path_segment: &PathSegment<'_>, args: &[Expr<'_>]) {
+    if args.is_empty() {
+        // When parsing TryFrom::try_from(...).expect(...), we will have more than 1 arg.
+        return;
+    }
+
+    let ident = path_segment.ident.as_str();
+    let method_arg_kind = &args[0].kind;
+    if ["to_string", "to_owned", "into"].contains(&ident) && is_expr_kind_empty_str(method_arg_kind) {
+        warn_then_suggest(cx, span);
+    } else if let ExprKind::Call(func, args) = method_arg_kind {
+        // If our first argument is a function call itself, it could be an `unwrap`-like function.
+        // E.g. String::try_from("hello").unwrap(), TryFrom::try_from("").expect("hello"), etc.
+        parse_call(cx, span, func, args);
+    }
+}
+
+/// Tries to parse an expression as a function call, emiting the warning if necessary.
+fn parse_call(cx: &LateContext<'_>, span: Span, func: &Expr<'_>, args: &[Expr<'_>]) {
+    if args.len() != 1 {
+        return;
+    }
+
+    let arg_kind = &args[0].kind;
+    if let ExprKind::Path(qpath) = &func.kind {
+        if let QPath::TypeRelative(_, _) = qpath {
+            // String::from(...) or String::try_from(...)
+            if  let QPath::TypeRelative(ty, path_seg) = qpath &&
+                [sym::from, sym::try_from].contains(&path_seg.ident.name) &&
+                let TyKind::Path(qpath) = &ty.kind &&
+                let QPath::Resolved(_, path) = qpath &&
+                let [path_seg] = path.segments &&
+                path_seg.ident.name == sym::String &&
+                is_expr_kind_empty_str(arg_kind)
+            {
+                warn_then_suggest(cx, span);
+            }
+        } else if let QPath::Resolved(_, path) = qpath {
+            // From::from(...) or TryFrom::try_from(...)
+            if  let [path_seg1, path_seg2] = path.segments &&
+                is_expr_kind_empty_str(arg_kind) && (
+                    (path_seg1.ident.name == sym::From && path_seg2.ident.name == sym::from) ||
+                    (path_seg1.ident.name == sym::TryFrom && path_seg2.ident.name == sym::try_from)
+                )
+            {
+                warn_then_suggest(cx, span);
+            }
+        }
+    }
+}

clippy_utils/src/msrvs.rs

@@ -32,8 +32,8 @@ msrv_aliases! {
     1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
     1,28,0 { FROM_BOOL }
     1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN }
+    1,24,0 { IS_ASCII_DIGIT }
     1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN }
     1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR }
     1,16,0 { STR_REPEAT }
-    1,24,0 { IS_ASCII_DIGIT }
 }

tests/ui/manual_empty_string_creation.fixed

@@ -0,0 +1,51 @@
+// run-rustfix
+
+#![warn(clippy::manual_empty_string_creation)]
+
+fn main() {
+    // Method calls
+    let _ = String::new();
+    let _ = "no warning".to_string();
+
+    let _ = String::new();
+    let _ = "no warning".to_owned();
+
+    let _: String = String::new();
+    let _: String = "no warning".into();
+
+    let _: SomeOtherStruct = "no warning".into();
+    let _: SomeOtherStruct = "".into(); // No warning too. We are not converting into String.
+
+    // Calls
+    let _ = String::new();
+    let _ = String::new();
+    let _ = String::from("no warning");
+    let _ = SomeOtherStruct::from("no warning");
+    let _ = SomeOtherStruct::from(""); // Again: no warning.
+
+    let _ = String::new();
+    let _ = String::try_from("no warning").unwrap();
+    let _ = String::try_from("no warning").expect("this should not warn");
+    let _ = SomeOtherStruct::try_from("no warning").unwrap();
+    let _ = SomeOtherStruct::try_from("").unwrap(); // Again: no warning.
+
+    let _: String = String::new();
+    let _: String = From::from("no warning");
+    let _: SomeOtherStruct = From::from("no warning");
+    let _: SomeOtherStruct = From::from(""); // Again: no warning.
+
+    let _: String = String::new();
+    let _: String = TryFrom::try_from("no warning").unwrap();
+    let _: String = TryFrom::try_from("no warning").expect("this should not warn");
+    let _: String = String::new();
+    let _: SomeOtherStruct = TryFrom::try_from("no_warning").unwrap();
+    let _: SomeOtherStruct = TryFrom::try_from("").unwrap(); // Again: no warning.
+}
+
+struct SomeOtherStruct {}
+
+impl From<&str> for SomeOtherStruct {
+    fn from(_value: &str) -> Self {
+        Self {}
+    }
+}

tests/ui/manual_empty_string_creation.rs

@@ -0,0 +1,51 @@
+// run-rustfix
+
+#![warn(clippy::manual_empty_string_creation)]
+
+fn main() {
+    // Method calls
+    let _ = "".to_string();
+    let _ = "no warning".to_string();
+
+    let _ = "".to_owned();
+    let _ = "no warning".to_owned();
+
+    let _: String = "".into();
+    let _: String = "no warning".into();
+
+    let _: SomeOtherStruct = "no warning".into();
+    let _: SomeOtherStruct = "".into(); // No warning too. We are not converting into String.
+
+    // Calls
+    let _ = String::from("");
+    let _ = <String>::from("");
+    let _ = String::from("no warning");
+    let _ = SomeOtherStruct::from("no warning");
+    let _ = SomeOtherStruct::from(""); // Again: no warning.
+
+    let _ = String::try_from("").unwrap();
+    let _ = String::try_from("no warning").unwrap();
+    let _ = String::try_from("no warning").expect("this should not warn");
+    let _ = SomeOtherStruct::try_from("no warning").unwrap();
+    let _ = SomeOtherStruct::try_from("").unwrap(); // Again: no warning.
+
+    let _: String = From::from("");
+    let _: String = From::from("no warning");
+    let _: SomeOtherStruct = From::from("no warning");
+    let _: SomeOtherStruct = From::from(""); // Again: no warning.
+
+    let _: String = TryFrom::try_from("").unwrap();
+    let _: String = TryFrom::try_from("no warning").unwrap();
+    let _: String = TryFrom::try_from("no warning").expect("this should not warn");
+    let _: String = TryFrom::try_from("").expect("this should warn");
+    let _: SomeOtherStruct = TryFrom::try_from("no_warning").unwrap();
+    let _: SomeOtherStruct = TryFrom::try_from("").unwrap(); // Again: no warning.
+}
+
+struct SomeOtherStruct {}
+
+impl From<&str> for SomeOtherStruct {
+    fn from(_value: &str) -> Self {
+        Self {}
+    }
+}

tests/ui/manual_empty_string_creation.stderr

@@ -0,0 +1,58 @@
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:7:13
+   |
+LL |     let _ = "".to_string();
+   |             ^^^^^^^^^^^^^^ help: consider using: `String::new()`
+   |
+   = note: `-D clippy::manual-empty-string-creation` implied by `-D warnings`
+
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:10:13
+   |
+LL |     let _ = "".to_owned();
+   |             ^^^^^^^^^^^^^ help: consider using: `String::new()`
+
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:13:21
+   |
+LL |     let _: String = "".into();
+   |                     ^^^^^^^^^ help: consider using: `String::new()`
+
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:20:13
+   |
+LL |     let _ = String::from("");
+   |             ^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
+
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:21:13
+   |
+LL |     let _ = <String>::from("");
+   |             ^^^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
+
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:26:13
+   |
+LL |     let _ = String::try_from("").unwrap();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
+
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:32:21
+   |
+LL |     let _: String = From::from("");
+   |                     ^^^^^^^^^^^^^^ help: consider using: `String::new()`
+
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:37:21
+   |
+LL |     let _: String = TryFrom::try_from("").unwrap();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
+
+error: empty String is being created manually
+  --> $DIR/manual_empty_string_creation.rs:40:21
+   |
+LL |     let _: String = TryFrom::try_from("").expect("this should warn");
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
+
+error: aborting due to 9 previous errors
+