From 8c6eea85a171bdd54811f5562884e8e706da34ce Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Sat, 8 Feb 2025 19:47:27 -0500 Subject: [PATCH] Extend `implicit_clone` to handle `to_string` calls --- clippy_lints/src/methods/implicit_clone.rs | 4 +--- clippy_lints/src/methods/mod.rs | 2 +- clippy_lints/src/methods/unnecessary_to_owned.rs | 4 ++-- tests/ui/implicit_clone.fixed | 6 ++++++ tests/ui/implicit_clone.rs | 6 ++++++ tests/ui/implicit_clone.stderr | 14 +++++++++++++- tests/ui/string_to_string.fixed | 8 ++++++++ tests/ui/string_to_string.rs | 4 ++-- tests/ui/string_to_string.stderr | 9 ++++----- 9 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 tests/ui/string_to_string.fixed diff --git a/clippy_lints/src/methods/implicit_clone.rs b/clippy_lints/src/methods/implicit_clone.rs index 9724463f0c08..adb2002feea9 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 d2d59f0013c0..a79ccba19e15 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 29a0d2950bc6..74cb95359c59 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 d60d1cb0ec04..267514c5f3d3 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 b96828f28c82..fba954026e7f 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 1eb6ff1fe429..4cca9b0d0c07 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 000000000000..354b6c7c9fb6 --- /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 7c5bd8a897ba..43a1cc18cd06 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 99eea06f18eb..0e33c6c1bf35 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