Extend `implicit_clone` to handle `to_string` calls

9 files changed, 43 insertions, 14 deletions

diff --git a/clippy_lints/src/methods/implicit_clone.rs b/clippy_lints/src/methods/implicit_clone.rs
index 9724463f0c0..adb2002feea 100644
--- a/clippy_lints/src/methods/implicit_clone.rs
+++ b/clippy_lints/src/methods/implicit_clone.rs
@@ -40,14 +40,12 @@ pub fn check(cx: &LateContext<'_>, method_name: Symbol, expr: &hir::Expr<'_>, re
 }
 
 /// 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: Symbol, method_def_id: hir::def_id::DefId) -> bool {
     match method_name {
         sym::to_os_string => is_diag_item_method(cx, method_def_id, sym::OsStr),
         sym::to_owned => is_diag_trait_item(cx, method_def_id, sym::ToOwned),
         sym::to_path_buf => is_diag_item_method(cx, method_def_id, sym::Path),
+        sym::to_string => is_diag_trait_item(cx, method_def_id, sym::ToString),
         sym::to_vec => cx
             .tcx
             .impl_of_method(method_def_id)
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index d2d59f0013c..a79ccba19e1 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -5403,7 +5403,7 @@ impl Methods {
                         implicit_clone::check(cx, name, expr, recv);
                     }
                 },
-                (sym::to_os_string | sym::to_path_buf | sym::to_vec, []) => {
+                (sym::to_os_string | sym::to_path_buf | sym::to_string | sym::to_vec, []) => {
                     implicit_clone::check(cx, name, expr, recv);
                 },
                 (sym::type_id, []) => {
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs
index 29a0d2950bc..74cb95359c5 100644
--- a/clippy_lints/src/methods/unnecessary_to_owned.rs
+++ b/clippy_lints/src/methods/unnecessary_to_owned.rs
@@ -619,8 +619,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, 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, method_def_id))
         || is_to_string_on_string_like(cx, call_expr, method_name, method_def_id)
 }
 
diff --git a/tests/ui/implicit_clone.fixed b/tests/ui/implicit_clone.fixed
index d60d1cb0ec0..267514c5f3d 100644
--- a/tests/ui/implicit_clone.fixed
+++ b/tests/ui/implicit_clone.fixed
@@ -135,4 +135,10 @@ fn main() {
     }
     let no_clone = &NoClone;
     let _ = no_clone.to_owned();
+
+    let s = String::from("foo");
+    let _ = s.clone();
+    //~^ implicit_clone
+    let _ = s.clone();
+    //~^ implicit_clone
 }
diff --git a/tests/ui/implicit_clone.rs b/tests/ui/implicit_clone.rs
index b96828f28c8..fba954026e7 100644
--- a/tests/ui/implicit_clone.rs
+++ b/tests/ui/implicit_clone.rs
@@ -135,4 +135,10 @@ fn main() {
     }
     let no_clone = &NoClone;
     let _ = no_clone.to_owned();
+
+    let s = String::from("foo");
+    let _ = s.to_owned();
+    //~^ implicit_clone
+    let _ = s.to_string();
+    //~^ implicit_clone
 }
diff --git a/tests/ui/implicit_clone.stderr b/tests/ui/implicit_clone.stderr
index 1eb6ff1fe42..4cca9b0d0c0 100644
--- a/tests/ui/implicit_clone.stderr
+++ b/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:140: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:142:13
+   |
+LL |     let _ = s.to_string();
+   |             ^^^^^^^^^^^^^ help: consider using: `s.clone()`
+
+error: aborting due to 13 previous errors
 
diff --git a/tests/ui/string_to_string.fixed b/tests/ui/string_to_string.fixed
new file mode 100644
index 00000000000..354b6c7c9fb
--- /dev/null
+++ b/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
+}
diff --git a/tests/ui/string_to_string.rs b/tests/ui/string_to_string.rs
index 7c5bd8a897b..43a1cc18cd0 100644
--- a/tests/ui/string_to_string.rs
+++ b/tests/ui/string_to_string.rs
@@ -1,10 +1,10 @@
-#![warn(clippy::string_to_string)]
+#![warn(clippy::implicit_clone, clippy::string_to_string)]
 #![allow(clippy::redundant_clone, clippy::unnecessary_literal_unwrap)]
 
 fn main() {
     let mut message = String::from("Hello");
     let mut v = message.to_string();
-    //~^ string_to_string
+    //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type
 
     let variable1 = String::new();
     let v = &variable1;
diff --git a/tests/ui/string_to_string.stderr b/tests/ui/string_to_string.stderr
index 99eea06f18e..0e33c6c1bf3 100644
--- a/tests/ui/string_to_string.stderr
+++ b/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: `to_string()` called on a `String`
   --> tests/ui/string_to_string.rs:14:9