Extend implicit_clone to handle to_string calls

Author: smoelius

clippy_lints/src/methods/implicit_clone.rs

@@ -40,14 +40,12 @@ pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv
 }
 
 /// Returns true if the named method can be used to clone the receiver.
-/// Note that `to_string` is not flagged by `implicit_clone`. So other lints that call
-/// `is_clone_like` and that do flag `to_string` must handle it separately. See, e.g.,
-/// `is_to_owned_like` in `unnecessary_to_owned.rs`.
 pub fn is_clone_like(cx: &LateContext<'_>, method_name: &str, method_def_id: hir::def_id::DefId) -> bool {
     match method_name {
         "to_os_string" => is_diag_item_method(cx, method_def_id, sym::OsStr),
         "to_owned" => is_diag_trait_item(cx, method_def_id, sym::ToOwned),
         "to_path_buf" => is_diag_item_method(cx, method_def_id, sym::Path),
+        "to_string" => is_diag_trait_item(cx, method_def_id, sym::ToString),
         "to_vec" => cx
             .tcx
             .impl_of_method(method_def_id)

clippy_lints/src/methods/mod.rs

@@ -5279,7 +5279,7 @@ impl Methods {
                         implicit_clone::check(cx, name, expr, recv);
                     }
                 },
-                ("to_os_string" | "to_path_buf" | "to_vec", []) => {
+                ("to_os_string" | "to_path_buf" | "to_string" | "to_vec", []) => {
                     implicit_clone::check(cx, name, expr, recv);
                 },
                 ("type_id", []) => {

clippy_lints/src/methods/unnecessary_to_owned.rs

@@ -618,8 +618,8 @@ fn is_cloned_or_copied(cx: &LateContext<'_>, method_name: Symbol, method_def_id:
 /// Returns true if the named method can be used to convert the receiver to its "owned"
 /// representation.
 fn is_to_owned_like<'a>(cx: &LateContext<'a>, call_expr: &Expr<'a>, method_name: Symbol, method_def_id: DefId) -> bool {
-    is_clone_like(cx, method_name.as_str(), method_def_id)
-        || is_cow_into_owned(cx, method_name, method_def_id)
+    is_cow_into_owned(cx, method_name, method_def_id)
+        || (method_name != sym::to_string && is_clone_like(cx, method_name.as_str(), method_def_id))
         || is_to_string_on_string_like(cx, call_expr, method_name, method_def_id)
 }
 

tests/ui/implicit_clone.fixed

@@ -124,4 +124,8 @@ fn main() {
     }
     let no_clone = &NoClone;
     let _ = no_clone.to_owned();
+
+    let s = String::from("foo");
+    let _ = s.clone();
+    let _ = s.clone();
 }

tests/ui/implicit_clone.rs

@@ -124,4 +124,8 @@ fn main() {
     }
     let no_clone = &NoClone;
     let _ = no_clone.to_owned();
+
+    let s = String::from("foo");
+    let _ = s.to_owned();
+    let _ = s.to_string();
 }

tests/ui/implicit_clone.stderr

@@ -67,5 +67,17 @@ error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenc
 LL |     let _ = pathbuf_ref.to_path_buf();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(**pathbuf_ref).clone()`
 
-error: aborting due to 11 previous errors
+error: implicitly cloning a `String` by calling `to_owned` on its dereferenced type
+  --> tests/ui/implicit_clone.rs:129:13
+   |
+LL |     let _ = s.to_owned();
+   |             ^^^^^^^^^^^^ help: consider using: `s.clone()`
+
+error: implicitly cloning a `String` by calling `to_string` on its dereferenced type
+  --> tests/ui/implicit_clone.rs:130:13
+   |
+LL |     let _ = s.to_string();
+   |             ^^^^^^^^^^^^^ help: consider using: `s.clone()`
+
+error: aborting due to 13 previous errors
 

tests/ui/string_to_string.fixed

@@ -0,0 +1,8 @@
+#![warn(clippy::implicit_clone)]
+#![allow(clippy::redundant_clone)]
+
+fn main() {
+    let mut message = String::from("Hello");
+    let mut v = message.clone();
+    //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type
+}

tests/ui/string_to_string.rs

@@ -1,8 +1,8 @@
-#![warn(clippy::string_to_string)]
+#![warn(clippy::implicit_clone)]
 #![allow(clippy::redundant_clone)]
 
 fn main() {
     let mut message = String::from("Hello");
     let mut v = message.to_string();
-    //~^ ERROR: `to_string()` called on a `String`
+    //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type
 }

tests/ui/string_to_string.stderr

@@ -1,12 +1,11 @@
-error: `to_string()` called on a `String`
+error: implicitly cloning a `String` by calling `to_string` on its dereferenced type
   --> tests/ui/string_to_string.rs:6:17
    |
 LL |     let mut v = message.to_string();
-   |                 ^^^^^^^^^^^^^^^^^^^
+   |                 ^^^^^^^^^^^^^^^^^^^ help: consider using: `message.clone()`
    |
-   = help: consider using `.clone()`
-   = note: `-D clippy::string-to-string` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::string_to_string)]`
+   = note: `-D clippy::implicit-clone` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::implicit_clone)]`
 
 error: aborting due to 1 previous error