Improve `string_to_string` lint in case it is in a map call (#14396)

Fixes https://github.com/rust-lang/rust-clippy/issues/14175.

There are two "big" cases to handle: `.map(|x| x.to_string())` and
`.map(String::to_string)`. If the closure has more than one expression,
we should not suggest `.cloned()`.

changelog: Improve `string_to_string` lint in case it is in a map call

r? @samueltardieu

6 files changed, 193 insertions, 17 deletions

diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs
index 4a5f143a2d3..27c548bed9f 100644
--- a/clippy_lints/src/strings.rs
+++ b/clippy_lints/src/strings.rs
@@ -14,6 +14,8 @@ use rustc_session::declare_lint_pass;
 use rustc_span::source_map::Spanned;
 use rustc_span::sym;
 
+use std::ops::ControlFlow;
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for string appends of the form `x = x + y` (without
@@ -438,27 +440,94 @@ declare_clippy_lint! {
 
 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;
         }
 
-        if let ExprKind::MethodCall(path, self_arg, [], _) = &expr.kind
-            && path.ident.name == sym::to_string
-            && let ty = cx.typeck_results().expr_ty(self_arg)
-            && is_type_lang_item(cx, ty, LangItem::String)
-        {
-            #[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()`");
-                },
-            );
+        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);
+                }
+            },
+            _ => {},
         }
     }
 }
diff --git a/tests/ui/string_to_string.rs b/tests/ui/string_to_string.rs
index 94174e1253b..7c5bd8a897b 100644
--- a/tests/ui/string_to_string.rs
+++ b/tests/ui/string_to_string.rs
@@ -1,8 +1,21 @@
 #![warn(clippy::string_to_string)]
-#![allow(clippy::redundant_clone)]
+#![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
+
+    let variable1 = String::new();
+    let v = &variable1;
+    let variable2 = Some(v);
+    let _ = variable2.map(|x| {
+        println!();
+        x.to_string()
+    });
+    //~^^ string_to_string
+
+    let x = Some(String::new());
+    let _ = x.unwrap_or_else(|| v.to_string());
+    //~^ string_to_string
 }
diff --git a/tests/ui/string_to_string.stderr b/tests/ui/string_to_string.stderr
index ae80597d1f8..99eea06f18e 100644
--- a/tests/ui/string_to_string.stderr
+++ b/tests/ui/string_to_string.stderr
@@ -8,5 +8,21 @@ LL |     let mut v = message.to_string();
    = note: `-D clippy::string-to-string` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::string_to_string)]`
 
-error: aborting due to 1 previous error
+error: `to_string()` called on a `String`
+  --> tests/ui/string_to_string.rs:14:9
+   |
+LL |         x.to_string()
+   |         ^^^^^^^^^^^^^
+   |
+   = help: consider using `.clone()`
+
+error: `to_string()` called on a `String`
+  --> tests/ui/string_to_string.rs:19:33
+   |
+LL |     let _ = x.unwrap_or_else(|| v.to_string());
+   |                                 ^^^^^^^^^^^^^
+   |
+   = help: consider using `.clone()`
+
+error: aborting due to 3 previous errors
 
diff --git a/tests/ui/string_to_string_in_map.fixed b/tests/ui/string_to_string_in_map.fixed
new file mode 100644
index 00000000000..efc085539f1
--- /dev/null
+++ b/tests/ui/string_to_string_in_map.fixed
@@ -0,0 +1,20 @@
+#![deny(clippy::string_to_string)]
+#![allow(clippy::unnecessary_literal_unwrap, clippy::useless_vec, clippy::iter_cloned_collect)]
+
+fn main() {
+    let variable1 = String::new();
+    let v = &variable1;
+    let variable2 = Some(v);
+    let _ = variable2.cloned();
+    //~^ string_to_string
+    let _ = variable2.cloned();
+    //~^ string_to_string
+    #[rustfmt::skip]
+    let _ = variable2.cloned();
+    //~^ string_to_string
+
+    let _ = vec![String::new()].iter().cloned().collect::<Vec<_>>();
+    //~^ string_to_string
+    let _ = vec![String::new()].iter().cloned().collect::<Vec<_>>();
+    //~^ string_to_string
+}
diff --git a/tests/ui/string_to_string_in_map.rs b/tests/ui/string_to_string_in_map.rs
new file mode 100644
index 00000000000..5bf1d7ba5a2
--- /dev/null
+++ b/tests/ui/string_to_string_in_map.rs
@@ -0,0 +1,20 @@
+#![deny(clippy::string_to_string)]
+#![allow(clippy::unnecessary_literal_unwrap, clippy::useless_vec, clippy::iter_cloned_collect)]
+
+fn main() {
+    let variable1 = String::new();
+    let v = &variable1;
+    let variable2 = Some(v);
+    let _ = variable2.map(String::to_string);
+    //~^ string_to_string
+    let _ = variable2.map(|x| x.to_string());
+    //~^ string_to_string
+    #[rustfmt::skip]
+    let _ = variable2.map(|x| { x.to_string() });
+    //~^ string_to_string
+
+    let _ = vec![String::new()].iter().map(String::to_string).collect::<Vec<_>>();
+    //~^ string_to_string
+    let _ = vec![String::new()].iter().map(|x| x.to_string()).collect::<Vec<_>>();
+    //~^ string_to_string
+}
diff --git a/tests/ui/string_to_string_in_map.stderr b/tests/ui/string_to_string_in_map.stderr
new file mode 100644
index 00000000000..35aeed656ee
--- /dev/null
+++ b/tests/ui/string_to_string_in_map.stderr
@@ -0,0 +1,38 @@
+error: `to_string()` called on a `String`
+  --> tests/ui/string_to_string_in_map.rs:8:23
+   |
+LL |     let _ = variable2.map(String::to_string);
+   |                       ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
+   |
+note: the lint level is defined here
+  --> tests/ui/string_to_string_in_map.rs:1:9
+   |
+LL | #![deny(clippy::string_to_string)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: `to_string()` called on a `String`
+  --> tests/ui/string_to_string_in_map.rs:10:23
+   |
+LL |     let _ = variable2.map(|x| x.to_string());
+   |                       ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
+
+error: `to_string()` called on a `String`
+  --> tests/ui/string_to_string_in_map.rs:13:23
+   |
+LL |     let _ = variable2.map(|x| { x.to_string() });
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
+
+error: `to_string()` called on a `String`
+  --> tests/ui/string_to_string_in_map.rs:16:40
+   |
+LL |     let _ = vec![String::new()].iter().map(String::to_string).collect::<Vec<_>>();
+   |                                        ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
+
+error: `to_string()` called on a `String`
+  --> tests/ui/string_to_string_in_map.rs:18:40
+   |
+LL |     let _ = vec![String::new()].iter().map(|x| x.to_string()).collect::<Vec<_>>();
+   |                                        ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
+
+error: aborting due to 5 previous errors
+