From 0e4b6c8124b1be1b0c353fc9a68566e106c72953 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 13 Mar 2025 11:46:02 +0100 Subject: [PATCH] Make `string_to_string` emit a suggestion when `cloned` can be used --- clippy_lints/src/strings.rs | 13 ++++---- tests/ui/string_to_string.rs | 16 +++++++++- tests/ui/string_to_string.stderr | 18 ++++++++++- tests/ui/string_to_string_in_map.fixed | 17 ++++++++++ tests/ui/string_to_string_in_map.rs | 10 +----- tests/ui/string_to_string_in_map.stderr | 41 ++++++------------------- 6 files changed, 66 insertions(+), 49 deletions(-) create mode 100644 tests/ui/string_to_string_in_map.fixed diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 6762fc71d453..247b5e270f82 100644 --- a/clippy_lints/src/strings.rs +++ b/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 { 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, 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, ); } diff --git a/tests/ui/string_to_string.rs b/tests/ui/string_to_string.rs index 94174e1253b8..8c97d8c16f93 100644 --- a/tests/ui/string_to_string.rs +++ b/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 } diff --git a/tests/ui/string_to_string.stderr b/tests/ui/string_to_string.stderr index ae80597d1f84..c0e2e75f51a7 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:20: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 000000000000..b004fd736adf --- /dev/null +++ b/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::>(); + //~^ string_to_string + let _ = vec![String::new()].iter().cloned().collect::>(); + //~^ string_to_string +} diff --git a/tests/ui/string_to_string_in_map.rs b/tests/ui/string_to_string_in_map.rs index 0afefa230730..387abe84205d 100644 --- a/tests/ui/string_to_string_in_map.rs +++ b/tests/ui/string_to_string_in_map.rs @@ -1,5 +1,5 @@ #![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(); @@ -7,16 +7,8 @@ fn main() { 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::>(); //~^ string_to_string diff --git a/tests/ui/string_to_string_in_map.stderr b/tests/ui/string_to_string_in_map.stderr index f77606bbf1ae..44e2119c1002 100644 --- a/tests/ui/string_to_string_in_map.stderr +++ b/tests/ui/string_to_string_in_map.stderr @@ -1,10 +1,9 @@ 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 | @@ -12,44 +11,22 @@ 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::>(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = 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::>(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `.cloned()` + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()` -error: aborting due to 6 previous errors +error: aborting due to 4 previous errors