Make string_to_string emit a suggestion when cloned can be used

Author: GuillaumeGomez

clippy_lints/src/strings.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
+use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::ty::is_type_lang_item;
 use clippy_utils::{
@@ -440,14 +440,14 @@ 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_expr.kind
+        && 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_expr.span)
+        Some(parent_span)
     } else {
         None
     }
@@ -464,13 +464,14 @@ fn is_called_from_map_like(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<rust
 }
 
 fn suggest_cloned_string_to_string(cx: &LateContext<'_>, span: rustc_span::Span) {
-    span_lint_and_help(
+    span_lint_and_sugg(
         cx,
         STRING_TO_STRING,
         span,
         "`to_string()` called on a `String`",
-        None,
-        "consider using `.cloned()`",
+        "try",
+        "cloned()".to_string(),
+        Applicability::MachineApplicable,
     );
 }
 

tests/ui/string_to_string.rs

@@ -1,8 +1,22 @@
 #![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
+
+    // Should not lint.
+    let x = Some(String::new());
+    let _ = x.unwrap_or_else(|| v.to_string());
+    //~^ string_to_string
 }

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:20:33
+   |
+LL |     let _ = x.unwrap_or_else(|| v.to_string());
+   |                                 ^^^^^^^^^^^^^
+   |
+   = help: consider using `.clone()`
+
+error: aborting due to 3 previous errors
 

tests/ui/string_to_string_in_map.fixed

@@ -0,0 +1,17 @@
+#![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
+
+    let _ = vec![String::new()].iter().cloned().collect::<Vec<_>>();
+    //~^ string_to_string
+    let _ = vec![String::new()].iter().cloned().collect::<Vec<_>>();
+    //~^ string_to_string
+}

tests/ui/string_to_string_in_map.rs

@@ -1,22 +1,14 @@
 #![deny(clippy::string_to_string)]
-#![allow(clippy::unnecessary_literal_unwrap, clippy::useless_vec)]
+#![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| {
-        println!();
-        x.to_string()
-    });
-    //~^^ string_to_string
     let _ = variable2.map(|x| x.to_string());
     //~^ string_to_string
-    let x = Some(String::new());
-    let _ = x.unwrap_or_else(|| v.to_string());
-    //~^ string_to_string
 
     let _ = vec![String::new()].iter().map(String::to_string).collect::<Vec<_>>();
     //~^ string_to_string

tests/ui/string_to_string_in_map.stderr

@@ -1,55 +1,32 @@
 error: `to_string()` called on a `String`
-  --> tests/ui/string_to_string_in_map.rs:8:13
+  --> tests/ui/string_to_string_in_map.rs:8:23
    |
 LL |     let _ = variable2.map(String::to_string);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                       ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
    |
-   = help: consider using `.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:12:9
-   |
-LL |         x.to_string()
-   |         ^^^^^^^^^^^^^
-   |
-   = help: consider using `.clone()`
-
-error: `to_string()` called on a `String`
-  --> tests/ui/string_to_string_in_map.rs:15:13
+  --> tests/ui/string_to_string_in_map.rs:10:23
    |
 LL |     let _ = variable2.map(|x| x.to_string());
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = help: consider using `.cloned()`
+   |                       ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
 
 error: `to_string()` called on a `String`
-  --> tests/ui/string_to_string_in_map.rs:18:33
-   |
-LL |     let _ = x.unwrap_or_else(|| v.to_string());
-   |                                 ^^^^^^^^^^^^^
-   |
-   = help: consider using `.clone()`
-
-error: `to_string()` called on a `String`
-  --> tests/ui/string_to_string_in_map.rs:21:13
+  --> tests/ui/string_to_string_in_map.rs:13:40
    |
 LL |     let _ = vec![String::new()].iter().map(String::to_string).collect::<Vec<_>>();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = help: consider using `.cloned()`
+   |                                        ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
 
 error: `to_string()` called on a `String`
-  --> tests/ui/string_to_string_in_map.rs:23:13
+  --> tests/ui/string_to_string_in_map.rs:15:40
    |
 LL |     let _ = vec![String::new()].iter().map(|x| x.to_string()).collect::<Vec<_>>();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = help: consider using `.cloned()`
+   |                                        ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
 
-error: aborting due to 6 previous errors
+error: aborting due to 4 previous errors