Extend implicit_clone to handle to_string calls

.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)

clippy_dev/src/update_lints.rs

@@ -546,7 +546,7 @@ impl Lint {
     /// Returns the lints in a `HashMap`, grouped by the different lint groups
     #[must_use]
     fn by_lint_group(lints: impl Iterator<Item = Self>) -> HashMap<String, Vec<Self>> {
-        lints.map(|lint| (lint.group.to_string(), lint)).into_group_map()
+        lints.map(|lint| (lint.group.clone(), lint)).into_group_map()
     }
 }
 

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,
                         );
                     }

clippy_lints/src/copies.rs

@@ -245,9 +245,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);
@@ -263,7 +263,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) {

clippy_lints/src/declared_lints.rs

@@ -708,7 +708,6 @@ pub static LINTS: &[&crate::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,

clippy_lints/src/deprecated_lints.rs

@@ -42,6 +42,8 @@ declare_with_version! { DEPRECATED(DEPRECATED_VERSION): &[(&str, &str)] = &[
     ("clippy::wrong_pub_self_convention", "`clippy::wrong_self_convention` now covers this case via the `avoid-breaking-exported-api` config"),
     #[clippy::version = "1.86.0"]
     ("clippy::option_map_or_err_ok", "`clippy::manual_ok_or` covers this case"),
+    #[clippy::version = "1.87.0"]
+    ("clippy::string_to_string", "`clippy:implicit_clone` and `clippy:map_clone` cover those cases"),
     // end deprecated lints. used by `cargo dev deprecate_lint`
 ]}
 

clippy_lints/src/if_not_else.rs

@@ -89,7 +89,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),

clippy_lints/src/lib.rs

@@ -822,7 +822,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     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));

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",

clippy_lints/src/matches/match_single_binding.rs

@@ -32,8 +32,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
         Some(expr.span),
         &mut app,
     )
-    .0
-    .to_string();
+    .0;
 
     // Do we need to add ';' to suggestion ?
     if let Node::Stmt(stmt) = cx.tcx.parent_hir_node(expr.hir_id)

clippy_lints/src/matches/single_match.rs

@@ -111,9 +111,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 {
@@ -126,7 +124,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;
@@ -182,7 +180,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);
     });
 }

clippy_lints/src/methods/implicit_clone.rs

@@ -1,11 +1,12 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::implements_trait;
-use clippy_utils::{is_diag_item_method, is_diag_trait_item, peel_middle_ty_refs};
+use clippy_utils::{get_parent_expr, is_diag_item_method, is_diag_trait_item, peel_middle_ty_refs};
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
 use rustc_span::sym;
+use std::ops::ControlFlow;
 
 use super::IMPLICIT_CLONE;
 
@@ -20,6 +21,7 @@ pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv
         && return_type == input_type
         && let Some(clone_trait) = cx.tcx.lang_items().clone_trait()
         && implements_trait(cx, return_type, clone_trait, &[])
+        && is_called_from_map_like(cx, expr).is_none()
     {
         let mut app = Applicability::MachineApplicable;
         let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "..", &mut app).0;
@@ -40,14 +42,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)
@@ -58,3 +58,38 @@ pub fn is_clone_like(cx: &LateContext<'_>, method_name: &str, method_def_id: hir
         _ => false,
     }
 }
+
+fn is_called_from_map_like(cx: &LateContext<'_>, expr: &hir::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
+            hir::Node::Block(block) if block.stmts.is_empty() && block.expr.map(|e| e.hir_id) == Some(child_hir_id) => {
+                ControlFlow::Continue(block.hir_id)
+            },
+            hir::Node::Expr(expr) if matches!(expr.kind, hir::ExprKind::Block(..)) => {
+                ControlFlow::Continue(expr.hir_id)
+            },
+            hir::Node::Expr(expr) if matches!(expr.kind, hir::ExprKind::Closure(_)) => ControlFlow::Break(Some(expr)),
+            _ => ControlFlow::Break(None),
+        })
+        .break_value()?;
+    is_parent_map_like(cx, parent_closure?)
+}
+
+fn is_parent_map_like(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<rustc_span::Span> {
+    if let Some(parent_expr) = get_parent_expr(cx, expr)
+        && let hir::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)
+        && (is_diag_item_method(cx, caller_def_id, sym::Result)
+            || is_diag_item_method(cx, caller_def_id, sym::Option)
+            || is_diag_trait_item(cx, caller_def_id, sym::Iterator))
+    {
+        Some(parent_span)
+    } else {
+        None
+    }
+}

clippy_lints/src/methods/map_clone.rs

@@ -1,3 +1,4 @@
+use super::implicit_clone::is_clone_like;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::source::snippet_with_applicability;
@@ -8,8 +9,8 @@ use rustc_hir::def_id::DefId;
 use rustc_hir::{self as hir, LangItem};
 use rustc_lint::LateContext;
 use rustc_middle::mir::Mutability;
-use rustc_middle::ty;
 use rustc_middle::ty::adjustment::Adjust;
+use rustc_middle::ty::{self, Ty};
 use rustc_span::symbol::Ident;
 use rustc_span::{Span, sym};
 
@@ -67,22 +68,29 @@ pub(super) fn check(cx: &LateContext<'_>, e: &hir::Expr<'_>, recv: &hir::Expr<'_
                                 }
                             },
                             hir::ExprKind::MethodCall(method, obj, [], _) => {
-                                if ident_eq(name, obj) && method.ident.name == sym::clone
+                                if ident_eq(name, obj)
                                 && let Some(fn_id) = cx.typeck_results().type_dependent_def_id(closure_expr.hir_id)
-                                && let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
-                                && cx.tcx.lang_items().clone_trait() == Some(trait_id)
+                                && (is_clone(cx, method.ident.name, fn_id)
+                                    || is_clone_like(cx, method.ident.as_str(), fn_id))
                                 // no autoderefs
                                 && !cx.typeck_results().expr_adjustments(obj).iter()
                                     .any(|a| matches!(a.kind, Adjust::Deref(Some(..))))
+                                && let obj_ty = cx.typeck_results().expr_ty_adjusted(obj)
+                                && let ty::Ref(_, ty, Mutability::Not) = obj_ty.kind()
+                                // Verify that the method call's output type is the same as its input type. This is to
+                                // avoid cases like `to_string` being called on a `&str`, or `to_vec` being called on a
+                                // slice.
+                                && *ty == cx.typeck_results().expr_ty_adjusted(closure_expr)
                                 {
-                                    let obj_ty = cx.typeck_results().expr_ty(obj);
-                                    if let ty::Ref(_, ty, mutability) = obj_ty.kind() {
-                                        if matches!(mutability, Mutability::Not) {
-                                            let copy = is_copy(cx, *ty);
-                                            lint_explicit_closure(cx, e.span, recv.span, copy, msrv);
-                                        }
+                                    let obj_ty_unadjusted = cx.typeck_results().expr_ty(obj);
+                                    if obj_ty == obj_ty_unadjusted {
+                                        let copy = is_copy(cx, *ty);
+                                        lint_explicit_closure(cx, e.span, recv.span, copy, msrv);
                                     } else {
-                                        lint_needless_cloning(cx, e.span, recv.span);
+                                        // To avoid issue #6299, ensure `obj` is not a mutable reference.
+                                        if !matches!(obj_ty_unadjusted.kind(), ty::Ref(_, _, Mutability::Mut)) {
+                                            lint_needless_cloning(cx, e.span, recv.span);
+                                        }
                                     }
                                 }
                             },
@@ -105,29 +113,65 @@ pub(super) fn check(cx: &LateContext<'_>, e: &hir::Expr<'_>, recv: &hir::Expr<'_
     }
 }
 
+fn is_clone(cx: &LateContext<'_>, method: rustc_span::Symbol, fn_id: DefId) -> bool {
+    if method == sym::clone
+        && let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
+        && cx.tcx.lang_items().clone_trait() == Some(trait_id)
+    {
+        true
+    } else {
+        false
+    }
+}
+
 fn handle_path(
     cx: &LateContext<'_>,
     arg: &hir::Expr<'_>,
     qpath: &hir::QPath<'_>,
     e: &hir::Expr<'_>,
     recv: &hir::Expr<'_>,
 ) {
+    let method = || match qpath {
+        hir::QPath::TypeRelative(_, method) => Some(method),
+        _ => None,
+    };
     if let Some(path_def_id) = cx.qpath_res(qpath, arg.hir_id).opt_def_id()
-        && cx.tcx.lang_items().get(LangItem::CloneFn) == Some(path_def_id)
+        && (cx.tcx.lang_items().get(LangItem::CloneFn) == Some(path_def_id)
+            || method().is_some_and(|method| is_clone_like(cx, method.ident.name.as_str(), path_def_id)))
         // The `copied` and `cloned` methods are only available on `&T` and `&mut T` in `Option`
         // and `Result`.
-        && let ty::Adt(_, args) = cx.typeck_results().expr_ty(recv).kind()
-        && let args = args.as_slice()
-        && let Some(ty) = args.iter().find_map(|generic_arg| generic_arg.as_type())
-        && let ty::Ref(_, ty, Mutability::Not) = ty.kind()
-        && let ty::FnDef(_, lst) = cx.typeck_results().expr_ty(arg).kind()
-        && lst.iter().all(|l| l.as_type() == Some(*ty))
-        && !should_call_clone_as_function(cx, *ty)
+        // Previously, `handle_path` would make this determination by checking whether `recv`'s type
+        // was instantiated with a reference. However, that approach can produce false negatives.
+        // Consider `std::slice::Iter`, for example. Even if it is instantiated with a non-reference
+        // type, its `map` method will expect a function that operates on references.
+        && let Some(ty) = clone_like_referent(cx, arg)
+        && !should_call_clone_as_function(cx, ty)
     {
         lint_path(cx, e.span, recv.span, is_copy(cx, ty.peel_refs()));
     }
 }
 
+/// Determine whether `expr` is function whose input type is `&T` and whose output type is `T`. If
+/// so, return `T`.
+fn clone_like_referent<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>) -> Option<Ty<'tcx>> {
+    let ty = cx.typeck_results().expr_ty_adjusted(expr);
+    if let ty::FnDef(def_id, generic_args) = ty.kind()
+        && let tys = cx
+            .tcx
+            .fn_sig(def_id)
+            .instantiate(cx.tcx, generic_args)
+            .skip_binder()
+            .inputs_and_output
+        && let [input_ty, output_ty] = tys.as_slice()
+        && let ty::Ref(_, referent_ty, Mutability::Not) = input_ty.kind()
+        && referent_ty == output_ty
+    {
+        Some(*referent_ty)
+    } else {
+        None
+    }
+}
+
 fn ident_eq(name: Ident, path: &hir::Expr<'_>) -> bool {
     if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = path.kind {
         path.segments.len() == 1 && path.segments[0].ident == name

clippy_lints/src/methods/mod.rs

@@ -5368,7 +5368,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_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 {

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.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)
 }
 

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,
         );
     }