Extend implicit_clone to handle to_string calls (#14177)

y21 · web-flow · commit e8185ec091d7 · 2025-08-02T11:41:00.000Z
Put another way, merge `string_to_string` into `implicit_clone`, as suggested here: #14173 (comment) Note: [I wrote](b891389) this comment: https://github.com/rust-lang/rust-clippy/blob/6cdb7f68c39a2458c6b8f6dc63da4123a6a5af89/clippy_lints/src/methods/implicit_clone.rs#L43-L45 Here is the context for why I wrote it: #7978 (comment) Regardless, it's probably time for the comment to go away. Extending `implicit_clone` to handle `to_string` calls yields many hits within Clippy's codebase. changelog: extend `implicit_clone` to handle `to_string` calls
diff --git a/.github/driver.sh b/.github/driver.sh
@@ -47,9 +47,9 @@ unset CARGO_MANIFEST_DIR
 
 # Run a lint and make sure it produces the expected output. It's also expected to exit with code 1
 # FIXME: How to match the clippy invocation in compile-test.rs?
-./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/string_to_string.rs 2>string_to_string.stderr && exit 1
-sed -e "/= help: for/d" string_to_string.stderr > normalized.stderr
-diff -u normalized.stderr tests/ui/string_to_string.stderr
+./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/char_lit_as_u8.rs 2>char_lit_as_u8.stderr && exit 1
+sed -e "/= help: for/d" char_lit_as_u8.stderr > normalized.stderr
+diff -u normalized.stderr tests/ui/char_lit_as_u8.stderr
 
 # make sure "clippy-driver --rustc --arg" and "rustc --arg" behave the same
 SYSROOT=$(rustc --print sysroot)
diff --git a/clippy_lints/src/blocks_in_conditions.rs b/clippy_lints/src/blocks_in_conditions.rs
@@ -100,8 +100,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInConditions {
                             cond.span,
                             BRACED_EXPR_MESSAGE,
                             "try",
-                            snippet_block_with_applicability(cx, ex.span, "..", Some(expr.span), &mut applicability)
-                                .to_string(),
+                            snippet_block_with_applicability(cx, ex.span, "..", Some(expr.span), &mut applicability),
                             applicability,
                         );
                     }
diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
@@ -235,9 +235,9 @@ fn lint_branches_sharing_code<'tcx>(
         let cond_snippet = reindent_multiline(&snippet(cx, cond_span, "_"), false, None);
         let cond_indent = indent_of(cx, cond_span);
         let moved_snippet = reindent_multiline(&snippet(cx, span, "_"), true, None);
-        let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
+        let suggestion = moved_snippet + "\n" + &cond_snippet + "{";
         let suggestion = reindent_multiline(&suggestion, true, cond_indent);
-        (replace_span, suggestion.to_string())
+        (replace_span, suggestion)
     });
     let end_suggestion = res.end_span(last_block, sm).map(|span| {
         let moved_snipped = reindent_multiline(&snippet(cx, span, "_"), true, None);
@@ -253,7 +253,7 @@ fn lint_branches_sharing_code<'tcx>(
                     .then_some(range.start - 4..range.end)
             })
             .map_or(span, |range| range.with_ctxt(span.ctxt()));
-        (span, suggestion.to_string())
+        (span, suggestion.clone())
     });
 
     let (span, msg, end_span) = match (&start_suggestion, &end_suggestion) {
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -690,7 +690,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::strings::STRING_FROM_UTF8_AS_BYTES_INFO,
     crate::strings::STRING_LIT_AS_BYTES_INFO,
     crate::strings::STRING_SLICE_INFO,
-    crate::strings::STRING_TO_STRING_INFO,
     crate::strings::STR_TO_STRING_INFO,
     crate::strings::TRIM_SPLIT_WHITESPACE_INFO,
     crate::strlen_on_c_strings::STRLEN_ON_C_STRINGS_INFO,
diff --git a/clippy_lints/src/deprecated_lints.rs b/clippy_lints/src/deprecated_lints.rs
@@ -34,6 +34,8 @@ declare_with_version! { DEPRECATED(DEPRECATED_VERSION) = [
     ("clippy::replace_consts", "`min_value` and `max_value` are now deprecated"),
     #[clippy::version = "pre 1.29.0"]
     ("clippy::should_assert_eq", "`assert!(a == b)` can now print the values the same way `assert_eq!(a, b) can"),
+    #[clippy::version = "1.90.0"]
+    ("clippy::string_to_string", "`clippy:implicit_clone` covers those cases"),
     #[clippy::version = "pre 1.29.0"]
     ("clippy::unsafe_vector_initialization", "the suggested alternative could be substantially slower"),
     #[clippy::version = "pre 1.29.0"]
diff --git a/clippy_lints/src/if_not_else.rs b/clippy_lints/src/if_not_else.rs
@@ -81,7 +81,7 @@ impl LateLintPass<'_> for IfNotElse {
                         e.span,
                         msg,
                         "try",
-                        make_sugg(cx, &cond.kind, cond_inner.span, els.span, "..", Some(e.span)).to_string(),
+                        make_sugg(cx, &cond.kind, cond_inner.span, els.span, "..", Some(e.span)),
                         Applicability::MachineApplicable,
                     ),
                     _ => span_lint_and_help(cx, IF_NOT_ELSE, e.span, msg, None, help),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -665,7 +665,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86IntelSyntax));
     store.register_late_pass(|_| Box::new(empty_drop::EmptyDrop));
     store.register_late_pass(|_| Box::new(strings::StrToString));
-    store.register_late_pass(|_| Box::new(strings::StringToString));
     store.register_late_pass(|_| Box::new(zero_sized_map_values::ZeroSizedMapValues));
     store.register_late_pass(|_| Box::<vec_init_then_push::VecInitThenPush>::default());
     store.register_late_pass(|_| Box::new(redundant_slicing::RedundantSlicing));
diff --git a/clippy_lints/src/manual_async_fn.rs b/clippy_lints/src/manual_async_fn.rs
@@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAsyncFn {
                             format!("{} async {}", vis_snip, &header_snip[vis_snip.len() + 1..ret_pos])
                         };
 
-                        let body_snip = snippet_block(cx, closure_body.value.span, "..", Some(block.span)).to_string();
+                        let body_snip = snippet_block(cx, closure_body.value.span, "..", Some(block.span));
 
                         diag.multipart_suggestion(
                             "make the function `async` and return the output of the future directly",
diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs
@@ -31,9 +31,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
     let match_body = peel_blocks(arms[0].body);
     let mut app = Applicability::MaybeIncorrect;
     let ctxt = expr.span.ctxt();
-    let mut snippet_body = snippet_block_with_context(cx, match_body.span, ctxt, "..", Some(expr.span), &mut app)
-        .0
-        .to_string();
+    let mut snippet_body = snippet_block_with_context(cx, match_body.span, ctxt, "..", Some(expr.span), &mut app).0;
 
     // Do we need to add ';' to suggestion ?
     if let Node::Stmt(stmt) = cx.tcx.parent_hir_node(expr.hir_id)
diff --git a/clippy_lints/src/matches/single_match.rs b/clippy_lints/src/matches/single_match.rs
@@ -112,9 +112,7 @@ fn report_single_pattern(
         let (sugg, help) = if is_unit_expr(arm.body) {
             (String::new(), "`match` expression can be removed")
         } else {
-            let mut sugg = snippet_block_with_context(cx, arm.body.span, ctxt, "..", Some(expr.span), &mut app)
-                .0
-                .to_string();
+            let mut sugg = snippet_block_with_context(cx, arm.body.span, ctxt, "..", Some(expr.span), &mut app).0;
             if let Node::Stmt(stmt) = cx.tcx.parent_hir_node(expr.hir_id)
                 && let StmtKind::Expr(_) = stmt.kind
                 && match arm.body.kind {
@@ -127,7 +125,7 @@ fn report_single_pattern(
             (sugg, "try")
         };
         span_lint_and_then(cx, lint, expr.span, msg, |diag| {
-            diag.span_suggestion(expr.span, help, sugg.to_string(), app);
+            diag.span_suggestion(expr.span, help, sugg, app);
             note(diag);
         });
         return;
@@ -188,7 +186,7 @@ fn report_single_pattern(
     };
 
     span_lint_and_then(cx, lint, expr.span, msg, |diag| {
-        diag.span_suggestion(expr.span, "try", sugg.to_string(), app);
+        diag.span_suggestion(expr.span, "try", sugg, app);
         note(diag);
     });
 }
diff --git 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
@@ -5450,7 +5450,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_sort_by.rs b/clippy_lints/src/methods/unnecessary_sort_by.rs
@@ -216,7 +216,7 @@ pub(super) fn check<'tcx>(
                 {
                     format!("{}::cmp::Reverse({})", std_or_core, trigger.closure_body)
                 } else {
-                    trigger.closure_body.to_string()
+                    trigger.closure_body
                 },
             ),
             if trigger.reverse {
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs
@@ -621,8 +621,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/clippy_lints/src/redundant_else.rs b/clippy_lints/src/redundant_else.rs
@@ -97,7 +97,7 @@ impl EarlyLintPass for RedundantElse {
             els.span.with_lo(then.span.hi()),
             "redundant else block",
             "remove the `else` block and move the contents out",
-            make_sugg(cx, els.span, "..", Some(expr.span)).to_string(),
+            make_sugg(cx, els.span, "..", Some(expr.span)),
             app,
         );
     }
diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs
@@ -13,8 +13,6 @@ use rustc_middle::ty;
 use rustc_session::declare_lint_pass;
 use rustc_span::source_map::Spanned;
 
-use std::ops::ControlFlow;
-
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for string appends of the form `x = x + y` (without
@@ -411,125 +409,6 @@ impl<'tcx> LateLintPass<'tcx> for StrToString {
     }
 }
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// This lint checks for `.to_string()` method calls on values of type `String`.
-    ///
-    /// ### Why restrict this?
-    /// The `to_string` method is also used on other types to convert them to a string.
-    /// When called on a `String` it only clones the `String`, which can be more specifically
-    /// expressed with `.clone()`.
-    ///
-    /// ### Example
-    /// ```no_run
-    /// let msg = String::from("Hello World");
-    /// let _ = msg.to_string();
-    /// ```
-    /// Use instead:
-    /// ```no_run
-    /// let msg = String::from("Hello World");
-    /// let _ = msg.clone();
-    /// ```
-    #[clippy::version = "pre 1.29.0"]
-    pub STRING_TO_STRING,
-    restriction,
-    "using `to_string()` on a `String`, which should be `clone()`"
-}
-
-declare_lint_pass!(StringToString => [STRING_TO_STRING]);
-
-fn is_parent_map_like(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<rustc_span::Span> {
-    if let Some(parent_expr) = get_parent_expr(cx, expr)
-        && let ExprKind::MethodCall(name, _, _, parent_span) = parent_expr.kind
-        && name.ident.name == sym::map
-        && let Some(caller_def_id) = cx.typeck_results().type_dependent_def_id(parent_expr.hir_id)
-        && (clippy_utils::is_diag_item_method(cx, caller_def_id, sym::Result)
-            || clippy_utils::is_diag_item_method(cx, caller_def_id, sym::Option)
-            || clippy_utils::is_diag_trait_item(cx, caller_def_id, sym::Iterator))
-    {
-        Some(parent_span)
-    } else {
-        None
-    }
-}
-
-fn is_called_from_map_like(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<rustc_span::Span> {
-    // Look for a closure as parent of `expr`, discarding simple blocks
-    let parent_closure = cx
-        .tcx
-        .hir_parent_iter(expr.hir_id)
-        .try_fold(expr.hir_id, |child_hir_id, (_, node)| match node {
-            // Check that the child expression is the only expression in the block
-            Node::Block(block) if block.stmts.is_empty() && block.expr.map(|e| e.hir_id) == Some(child_hir_id) => {
-                ControlFlow::Continue(block.hir_id)
-            },
-            Node::Expr(expr) if matches!(expr.kind, ExprKind::Block(..)) => ControlFlow::Continue(expr.hir_id),
-            Node::Expr(expr) if matches!(expr.kind, ExprKind::Closure(_)) => ControlFlow::Break(Some(expr)),
-            _ => ControlFlow::Break(None),
-        })
-        .break_value()?;
-    is_parent_map_like(cx, parent_closure?)
-}
-
-fn suggest_cloned_string_to_string(cx: &LateContext<'_>, span: rustc_span::Span) {
-    span_lint_and_sugg(
-        cx,
-        STRING_TO_STRING,
-        span,
-        "`to_string()` called on a `String`",
-        "try",
-        "cloned()".to_string(),
-        Applicability::MachineApplicable,
-    );
-}
-
-impl<'tcx> LateLintPass<'tcx> for StringToString {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
-        if expr.span.from_expansion() {
-            return;
-        }
-
-        match &expr.kind {
-            ExprKind::MethodCall(path, self_arg, [], _) => {
-                if path.ident.name == sym::to_string
-                    && let ty = cx.typeck_results().expr_ty(self_arg)
-                    && is_type_lang_item(cx, ty.peel_refs(), LangItem::String)
-                {
-                    if let Some(parent_span) = is_called_from_map_like(cx, expr) {
-                        suggest_cloned_string_to_string(cx, parent_span);
-                    } else {
-                        #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
-                        span_lint_and_then(
-                            cx,
-                            STRING_TO_STRING,
-                            expr.span,
-                            "`to_string()` called on a `String`",
-                            |diag| {
-                                diag.help("consider using `.clone()`");
-                            },
-                        );
-                    }
-                }
-            },
-            ExprKind::Path(QPath::TypeRelative(ty, segment)) => {
-                if segment.ident.name == sym::to_string
-                    && let rustc_hir::TyKind::Path(QPath::Resolved(_, path)) = ty.peel_refs().kind
-                    && let rustc_hir::def::Res::Def(_, def_id) = path.res
-                    && cx
-                        .tcx
-                        .lang_items()
-                        .get(LangItem::String)
-                        .is_some_and(|lang_id| lang_id == def_id)
-                    && let Some(parent_span) = is_parent_map_like(cx, expr)
-                {
-                    suggest_cloned_string_to_string(cx, parent_span);
-                }
-            },
-            _ => {},
-        }
-    }
-}
-
 declare_clippy_lint! {
     /// ### What it does
     /// Warns about calling `str::trim` (or variants) before `str::split_whitespace`.
diff --git a/lintcheck/src/input.rs b/lintcheck/src/input.rs
@@ -117,7 +117,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec<CrateWithSource>, RecursiveOptions)
             crate_sources.push(CrateWithSource {
                 name: tk.name.clone(),
                 source: CrateSource::CratesIo {
-                    version: version.to_string(),
+                    version: version.clone(),
                 },
                 file_link: tk.file_link(DEFAULT_DOCS_LINK),
                 options: tk.options.clone(),
diff --git a/lintcheck/src/output.rs b/lintcheck/src/output.rs
@@ -220,7 +220,7 @@ fn print_stats(old_stats: HashMap<String, usize>, new_stats: HashMap<&String, us
     let same_in_both_hashmaps = old_stats
         .iter()
         .filter(|(old_key, old_val)| new_stats.get::<&String>(old_key) == Some(old_val))
-        .map(|(k, v)| (k.to_string(), *v))
+        .map(|(k, v)| (k.clone(), *v))
         .collect::<Vec<(String, usize)>>();
 
     let mut old_stats_deduped = old_stats;
diff --git a/tests/ui/deprecated.rs b/tests/ui/deprecated.rs
@@ -12,6 +12,7 @@
 #![warn(clippy::regex_macro)] //~ ERROR: lint `clippy::regex_macro`
 #![warn(clippy::replace_consts)] //~ ERROR: lint `clippy::replace_consts`
 #![warn(clippy::should_assert_eq)] //~ ERROR: lint `clippy::should_assert_eq`
+#![warn(clippy::string_to_string)] //~ ERROR: lint `clippy::string_to_string`
 #![warn(clippy::unsafe_vector_initialization)] //~ ERROR: lint `clippy::unsafe_vector_initialization`
 #![warn(clippy::unstable_as_mut_slice)] //~ ERROR: lint `clippy::unstable_as_mut_slice`
 #![warn(clippy::unstable_as_slice)] //~ ERROR: lint `clippy::unstable_as_slice`
diff --git a/tests/ui/deprecated.stderr b/tests/ui/deprecated.stderr
@@ -61,35 +61,41 @@ error: lint `clippy::should_assert_eq` has been removed: `assert!(a == b)` can n
 LL | #![warn(clippy::should_assert_eq)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: lint `clippy::unsafe_vector_initialization` has been removed: the suggested alternative could be substantially slower
+error: lint `clippy::string_to_string` has been removed: `clippy:implicit_clone` covers those cases
   --> tests/ui/deprecated.rs:15:9
    |
+LL | #![warn(clippy::string_to_string)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: lint `clippy::unsafe_vector_initialization` has been removed: the suggested alternative could be substantially slower
+  --> tests/ui/deprecated.rs:16:9
+   |
 LL | #![warn(clippy::unsafe_vector_initialization)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: lint `clippy::unstable_as_mut_slice` has been removed: `Vec::as_mut_slice` is now stable
-  --> tests/ui/deprecated.rs:16:9
+  --> tests/ui/deprecated.rs:17:9
    |
 LL | #![warn(clippy::unstable_as_mut_slice)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: lint `clippy::unstable_as_slice` has been removed: `Vec::as_slice` is now stable
-  --> tests/ui/deprecated.rs:17:9
+  --> tests/ui/deprecated.rs:18:9
    |
 LL | #![warn(clippy::unstable_as_slice)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: lint `clippy::unused_collect` has been removed: `Iterator::collect` is now marked as `#[must_use]`
-  --> tests/ui/deprecated.rs:18:9
+  --> tests/ui/deprecated.rs:19:9
    |
 LL | #![warn(clippy::unused_collect)]
    |         ^^^^^^^^^^^^^^^^^^^^^^
 
 error: lint `clippy::wrong_pub_self_convention` has been removed: `clippy::wrong_self_convention` now covers this case via the `avoid-breaking-exported-api` config
-  --> tests/ui/deprecated.rs:19:9
+  --> tests/ui/deprecated.rs:20:9
    |
 LL | #![warn(clippy::wrong_pub_self_convention)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 15 previous errors
+error: aborting due to 16 previous errors
 
diff --git a/tests/ui/implicit_clone.fixed b/tests/ui/implicit_clone.fixed
diff --git a/tests/ui/implicit_clone.rs b/tests/ui/implicit_clone.rs
diff --git a/tests/ui/implicit_clone.stderr b/tests/ui/implicit_clone.stderr
diff --git a/tests/ui/string_to_string.rs b/tests/ui/string_to_string.rs
diff --git a/tests/ui/string_to_string.stderr b/tests/ui/string_to_string.stderr
diff --git a/tests/ui/string_to_string_in_map.fixed b/tests/ui/string_to_string_in_map.fixed
diff --git a/tests/ui/string_to_string_in_map.rs b/tests/ui/string_to_string_in_map.rs
diff --git a/tests/ui/string_to_string_in_map.stderr b/tests/ui/string_to_string_in_map.stderr
