From 8c6eea85a171bdd54811f5562884e8e706da34ce Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Sat, 8 Feb 2025 19:47:27 -0500 Subject: [PATCH 1/6] Extend `implicit_clone` to handle `to_string` calls --- clippy_lints/src/methods/implicit_clone.rs | 4 +--- clippy_lints/src/methods/mod.rs | 2 +- clippy_lints/src/methods/unnecessary_to_owned.rs | 4 ++-- tests/ui/implicit_clone.fixed | 6 ++++++ tests/ui/implicit_clone.rs | 6 ++++++ tests/ui/implicit_clone.stderr | 14 +++++++++++++- tests/ui/string_to_string.fixed | 8 ++++++++ tests/ui/string_to_string.rs | 4 ++-- tests/ui/string_to_string.stderr | 9 ++++----- 9 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 tests/ui/string_to_string.fixed diff --git a/clippy_lints/src/methods/implicit_clone.rs b/clippy_lints/src/methods/implicit_clone.rs index 9724463f0c08..adb2002feea9 100644 --- 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 index d2d59f0013c0..a79ccba19e15 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -5403,7 +5403,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_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs index 29a0d2950bc6..74cb95359c59 100644 --- a/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -619,8 +619,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/tests/ui/implicit_clone.fixed b/tests/ui/implicit_clone.fixed index d60d1cb0ec04..267514c5f3d3 100644 --- a/tests/ui/implicit_clone.fixed +++ b/tests/ui/implicit_clone.fixed @@ -135,4 +135,10 @@ fn main() { } let no_clone = &NoClone; let _ = no_clone.to_owned(); + + let s = String::from("foo"); + let _ = s.clone(); + //~^ implicit_clone + let _ = s.clone(); + //~^ implicit_clone } diff --git a/tests/ui/implicit_clone.rs b/tests/ui/implicit_clone.rs index b96828f28c82..fba954026e7f 100644 --- a/tests/ui/implicit_clone.rs +++ b/tests/ui/implicit_clone.rs @@ -135,4 +135,10 @@ fn main() { } let no_clone = &NoClone; let _ = no_clone.to_owned(); + + let s = String::from("foo"); + let _ = s.to_owned(); + //~^ implicit_clone + let _ = s.to_string(); + //~^ implicit_clone } diff --git a/tests/ui/implicit_clone.stderr b/tests/ui/implicit_clone.stderr index 1eb6ff1fe429..4cca9b0d0c07 100644 --- a/tests/ui/implicit_clone.stderr +++ b/tests/ui/implicit_clone.stderr @@ -67,5 +67,17 @@ error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenc LL | let _ = pathbuf_ref.to_path_buf(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(**pathbuf_ref).clone()` -error: aborting due to 11 previous errors +error: implicitly cloning a `String` by calling `to_owned` on its dereferenced type + --> tests/ui/implicit_clone.rs:140:13 + | +LL | let _ = s.to_owned(); + | ^^^^^^^^^^^^ help: consider using: `s.clone()` + +error: implicitly cloning a `String` by calling `to_string` on its dereferenced type + --> tests/ui/implicit_clone.rs:142:13 + | +LL | let _ = s.to_string(); + | ^^^^^^^^^^^^^ help: consider using: `s.clone()` + +error: aborting due to 13 previous errors diff --git a/tests/ui/string_to_string.fixed b/tests/ui/string_to_string.fixed new file mode 100644 index 000000000000..354b6c7c9fb6 --- /dev/null +++ b/tests/ui/string_to_string.fixed @@ -0,0 +1,8 @@ +#![warn(clippy::implicit_clone)] +#![allow(clippy::redundant_clone)] + +fn main() { + let mut message = String::from("Hello"); + let mut v = message.clone(); + //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type +} diff --git a/tests/ui/string_to_string.rs b/tests/ui/string_to_string.rs index 7c5bd8a897ba..43a1cc18cd06 100644 --- a/tests/ui/string_to_string.rs +++ b/tests/ui/string_to_string.rs @@ -1,10 +1,10 @@ -#![warn(clippy::string_to_string)] +#![warn(clippy::implicit_clone, clippy::string_to_string)] #![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 + //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type let variable1 = String::new(); let v = &variable1; diff --git a/tests/ui/string_to_string.stderr b/tests/ui/string_to_string.stderr index 99eea06f18eb..0e33c6c1bf35 100644 --- a/tests/ui/string_to_string.stderr +++ b/tests/ui/string_to_string.stderr @@ -1,12 +1,11 @@ -error: `to_string()` called on a `String` +error: implicitly cloning a `String` by calling `to_string` on its dereferenced type --> tests/ui/string_to_string.rs:6:17 | LL | let mut v = message.to_string(); - | ^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `message.clone()` | - = help: consider using `.clone()` - = note: `-D clippy::string-to-string` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::string_to_string)]` + = note: `-D clippy::implicit-clone` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::implicit_clone)]` error: `to_string()` called on a `String` --> tests/ui/string_to_string.rs:14:9 From 8a9adba8525e7bf40ca5f3b08d359742173f6fa7 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Sat, 8 Feb 2025 19:29:02 -0500 Subject: [PATCH 2/6] Fix adjacent code --- clippy_lints/src/blocks_in_conditions.rs | 3 +-- clippy_lints/src/copies.rs | 6 +++--- clippy_lints/src/if_not_else.rs | 2 +- clippy_lints/src/manual_async_fn.rs | 2 +- clippy_lints/src/matches/match_single_binding.rs | 4 +--- clippy_lints/src/matches/single_match.rs | 8 +++----- clippy_lints/src/methods/unnecessary_sort_by.rs | 2 +- clippy_lints/src/redundant_else.rs | 2 +- lintcheck/src/input.rs | 2 +- lintcheck/src/output.rs | 2 +- 10 files changed, 14 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/blocks_in_conditions.rs b/clippy_lints/src/blocks_in_conditions.rs index 011962846cb3..10fe1a4174f8 100644 --- 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 index 2467fc95fd05..6cffb508fe64 100644 --- a/clippy_lints/src/copies.rs +++ b/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) { diff --git a/clippy_lints/src/if_not_else.rs b/clippy_lints/src/if_not_else.rs index 45f9aa0a53e4..5e7eb26a4f90 100644 --- a/clippy_lints/src/if_not_else.rs +++ b/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), diff --git a/clippy_lints/src/manual_async_fn.rs b/clippy_lints/src/manual_async_fn.rs index abd1ac954cda..ba1ad599e116 100644 --- 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 index adda35869900..5c2837d6b1a0 100644 --- a/clippy_lints/src/matches/match_single_binding.rs +++ b/clippy_lints/src/matches/match_single_binding.rs @@ -25,9 +25,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 index 08c0caa4266c..7c0cd6ee0dea 100644 --- 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; @@ -183,7 +181,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/unnecessary_sort_by.rs b/clippy_lints/src/methods/unnecessary_sort_by.rs index 235fdac29433..437f353746f5 100644 --- 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/redundant_else.rs b/clippy_lints/src/redundant_else.rs index a3be16ed858e..79353dc9247e 100644 --- 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/lintcheck/src/input.rs b/lintcheck/src/input.rs index 83eb0a577d6b..60182fc2293a 100644 --- a/lintcheck/src/input.rs +++ b/lintcheck/src/input.rs @@ -117,7 +117,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec, 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 index dcc1ec339ef9..e38e21015702 100644 --- a/lintcheck/src/output.rs +++ b/lintcheck/src/output.rs @@ -220,7 +220,7 @@ fn print_stats(old_stats: HashMap, 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::>(); let mut old_stats_deduped = old_stats; From 8e5b0cdae1aaffcee5455852cb0eec0c6b56b17f Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 20 Mar 2025 14:01:04 -0400 Subject: [PATCH 3/6] Account for changes from issue 14396 --- clippy_lints/src/strings.rs | 16 ++-------------- tests/ui/string_to_string.fixed | 17 +++++++++++++++-- tests/ui/string_to_string.rs | 4 ++-- tests/ui/string_to_string.stderr | 12 ++++-------- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 73a9fe71e001..9fcef84eeb34 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -494,21 +494,9 @@ impl<'tcx> LateLintPass<'tcx> for StringToString { 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) + && let Some(parent_span) = is_called_from_map_like(cx, expr) { - 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()`"); - }, - ); - } + suggest_cloned_string_to_string(cx, parent_span); } }, ExprKind::Path(QPath::TypeRelative(ty, segment)) => { diff --git a/tests/ui/string_to_string.fixed b/tests/ui/string_to_string.fixed index 354b6c7c9fb6..751f451cb685 100644 --- a/tests/ui/string_to_string.fixed +++ b/tests/ui/string_to_string.fixed @@ -1,8 +1,21 @@ -#![warn(clippy::implicit_clone)] -#![allow(clippy::redundant_clone)] +#![warn(clippy::implicit_clone, clippy::string_to_string)] +#![allow(clippy::redundant_clone, clippy::unnecessary_literal_unwrap)] fn main() { let mut message = String::from("Hello"); let mut v = message.clone(); //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type + + let variable1 = String::new(); + let v = &variable1; + let variable2 = Some(v); + let _ = variable2.map(|x| { + println!(); + x.clone() + //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type + }); + + let x = Some(String::new()); + let _ = x.unwrap_or_else(|| v.clone()); + //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type } diff --git a/tests/ui/string_to_string.rs b/tests/ui/string_to_string.rs index 43a1cc18cd06..d519677d59ec 100644 --- a/tests/ui/string_to_string.rs +++ b/tests/ui/string_to_string.rs @@ -12,10 +12,10 @@ fn main() { let _ = variable2.map(|x| { println!(); x.to_string() + //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type }); - //~^^ string_to_string let x = Some(String::new()); let _ = x.unwrap_or_else(|| v.to_string()); - //~^ string_to_string + //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type } diff --git a/tests/ui/string_to_string.stderr b/tests/ui/string_to_string.stderr index 0e33c6c1bf35..220fe3170c6e 100644 --- a/tests/ui/string_to_string.stderr +++ b/tests/ui/string_to_string.stderr @@ -7,21 +7,17 @@ LL | let mut v = message.to_string(); = note: `-D clippy::implicit-clone` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::implicit_clone)]` -error: `to_string()` called on a `String` +error: implicitly cloning a `String` by calling `to_string` on its dereferenced type --> tests/ui/string_to_string.rs:14:9 | LL | x.to_string() - | ^^^^^^^^^^^^^ - | - = help: consider using `.clone()` + | ^^^^^^^^^^^^^ help: consider using: `x.clone()` -error: `to_string()` called on a `String` +error: implicitly cloning a `String` by calling `to_string` on its dereferenced type --> tests/ui/string_to_string.rs:19:33 | LL | let _ = x.unwrap_or_else(|| v.to_string()); - | ^^^^^^^^^^^^^ - | - = help: consider using `.clone()` + | ^^^^^^^^^^^^^ help: consider using: `v.clone()` error: aborting due to 3 previous errors From 6cea9ccf0f256e00936baf1db0cfbb223a5e8812 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Mon, 10 Mar 2025 22:55:21 +0000 Subject: [PATCH 4/6] Replace `string_to_string` with `char_lit_as_u8` in driver.sh --- .github/driver.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/driver.sh b/.github/driver.sh index 5a81b4112918..2874aaf2110c 100755 --- 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) From 94ca0544c11bd3b8742332f6899d313ec26d7b34 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Fri, 28 Mar 2025 12:23:02 -0400 Subject: [PATCH 5/6] Handle additional cases from issue 14396 --- clippy_lints/src/methods/implicit_clone.rs | 39 +++++++++- clippy_lints/src/methods/map_clone.rs | 82 +++++++++++++++++----- tests/ui/map_clone.fixed | 16 +++++ tests/ui/map_clone.rs | 16 +++++ tests/ui/map_clone.stderr | 32 ++++++++- tests/ui/string_to_string_in_map.fixed | 20 ------ tests/ui/string_to_string_in_map.rs | 20 ------ tests/ui/string_to_string_in_map.stderr | 38 ---------- 8 files changed, 164 insertions(+), 99 deletions(-) delete mode 100644 tests/ui/string_to_string_in_map.fixed delete mode 100644 tests/ui/string_to_string_in_map.rs delete mode 100644 tests/ui/string_to_string_in_map.stderr diff --git a/clippy_lints/src/methods/implicit_clone.rs b/clippy_lints/src/methods/implicit_clone.rs index adb2002feea9..174a695d43ba 100644 --- a/clippy_lints/src/methods/implicit_clone.rs +++ b/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, sym}; +use clippy_utils::{get_parent_expr, is_diag_item_method, is_diag_trait_item, peel_middle_ty_refs, sym}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_span::Symbol; +use std::ops::ControlFlow; use super::IMPLICIT_CLONE; @@ -20,6 +21,7 @@ pub fn check(cx: &LateContext<'_>, method_name: Symbol, expr: &hir::Expr<'_>, re && 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; @@ -56,3 +58,38 @@ pub fn is_clone_like(cx: &LateContext<'_>, method_name: Symbol, method_def_id: h _ => false, } } + +fn is_called_from_map_like(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option { + // 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 { + 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 + } +} diff --git a/clippy_lints/src/methods/map_clone.rs b/clippy_lints/src/methods/map_clone.rs index 333a33f7527d..22699b6b4658 100644 --- a/clippy_lints/src/methods/map_clone.rs +++ b/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.name, 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,6 +113,17 @@ 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<'_>, @@ -112,22 +131,47 @@ fn handle_path( 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, 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> { + 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 diff --git a/tests/ui/map_clone.fixed b/tests/ui/map_clone.fixed index 8db4b21be684..3862177fd057 100644 --- a/tests/ui/map_clone.fixed +++ b/tests/ui/map_clone.fixed @@ -158,4 +158,20 @@ fn main() { let y = Some(&x); let _z = y.map(RcWeak::clone); } + + let variable1 = String::new(); + let v = &variable1; + let variable2 = Some(v); + let _ = variable2.cloned(); + //~^ map_clone + let _ = variable2.cloned(); + //~^ map_clone + #[rustfmt::skip] + let _ = variable2.cloned(); + //~^ map_clone + + let _ = vec![String::new()].iter().cloned().collect::>(); + //~^ map_clone + let _ = vec![String::new()].iter().cloned().collect::>(); + //~^ map_clone } diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs index a726e3561634..4e10d4cda1d3 100644 --- a/tests/ui/map_clone.rs +++ b/tests/ui/map_clone.rs @@ -158,4 +158,20 @@ fn main() { let y = Some(&x); let _z = y.map(RcWeak::clone); } + + let variable1 = String::new(); + let v = &variable1; + let variable2 = Some(v); + let _ = variable2.map(String::to_string); + //~^ map_clone + let _ = variable2.map(|x| x.to_string()); + //~^ map_clone + #[rustfmt::skip] + let _ = variable2.map(|x| { x.to_string() }); + //~^ map_clone + + let _ = vec![String::new()].iter().map(String::to_string).collect::>(); + //~^ map_clone + let _ = vec![String::new()].iter().map(|x| x.to_string()).collect::>(); + //~^ map_clone } diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr index 8bf54e4e8aee..e44388ba78d0 100644 --- a/tests/ui/map_clone.stderr +++ b/tests/ui/map_clone.stderr @@ -91,5 +91,35 @@ error: you are explicitly cloning with `.map()` LL | let y = x.map(|x| Clone::clone(x)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()` -error: aborting due to 15 previous errors +error: you are explicitly cloning with `.map()` + --> tests/ui/map_clone.rs:165:13 + | +LL | let _ = variable2.map(String::to_string); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `variable2.cloned()` + +error: you are using an explicit closure for cloning elements + --> tests/ui/map_clone.rs:167:13 + | +LL | let _ = variable2.map(|x| x.to_string()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `variable2.cloned()` + +error: you are using an explicit closure for cloning elements + --> tests/ui/map_clone.rs:170:13 + | +LL | let _ = variable2.map(|x| { x.to_string() }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `variable2.cloned()` + +error: you are explicitly cloning with `.map()` + --> tests/ui/map_clone.rs:173:13 + | +LL | let _ = vec![String::new()].iter().map(String::to_string).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `vec![String::new()].iter().cloned()` + +error: you are using an explicit closure for cloning elements + --> tests/ui/map_clone.rs:175:13 + | +LL | let _ = vec![String::new()].iter().map(|x| x.to_string()).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `vec![String::new()].iter().cloned()` + +error: aborting due to 20 previous errors diff --git a/tests/ui/string_to_string_in_map.fixed b/tests/ui/string_to_string_in_map.fixed deleted file mode 100644 index efc085539f15..000000000000 --- a/tests/ui/string_to_string_in_map.fixed +++ /dev/null @@ -1,20 +0,0 @@ -#![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::>(); - //~^ 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 deleted file mode 100644 index 5bf1d7ba5a2e..000000000000 --- a/tests/ui/string_to_string_in_map.rs +++ /dev/null @@ -1,20 +0,0 @@ -#![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::>(); - //~^ string_to_string - let _ = vec![String::new()].iter().map(|x| x.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 deleted file mode 100644 index 35aeed656eed..000000000000 --- a/tests/ui/string_to_string_in_map.stderr +++ /dev/null @@ -1,38 +0,0 @@ -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::>(); - | ^^^^^^^^^^^^^^^^^^^^^^ 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::>(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()` - -error: aborting due to 5 previous errors - From b09d9d69e04e59a71d31dbeb73840b7e6bec56bd Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Fri, 28 Mar 2025 12:27:04 -0400 Subject: [PATCH 6/6] Deprecate `string_to_string` --- clippy_lints/src/declared_lints.rs | 1 - clippy_lints/src/deprecated_lints.rs | 2 + clippy_lints/src/lib.rs | 1 - clippy_lints/src/strings.rs | 109 --------------------------- tests/ui/deprecated.rs | 1 + tests/ui/deprecated.stderr | 18 +++-- tests/ui/string_to_string.fixed | 21 ------ tests/ui/string_to_string.rs | 21 ------ tests/ui/string_to_string.stderr | 23 ------ 9 files changed, 15 insertions(+), 182 deletions(-) delete mode 100644 tests/ui/string_to_string.fixed delete mode 100644 tests/ui/string_to_string.rs delete mode 100644 tests/ui/string_to_string.stderr diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5fcb851dfebc..1b0e87ffa710 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -683,7 +683,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, diff --git a/clippy_lints/src/deprecated_lints.rs b/clippy_lints/src/deprecated_lints.rs index 5204f73ea0a9..38100ab5f546 100644 --- 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.87.0"] + ("clippy::string_to_string", "`clippy:implicit_clone` and `clippy:map_clone` cover 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/lib.rs b/clippy_lints/src/lib.rs index ca1a0b357108..cd9917e3ac88 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -782,7 +782,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::::default()); store.register_late_pass(|_| Box::new(redundant_slicing::RedundantSlicing)); diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 9fcef84eeb34..d520df95ae89 100644 --- 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,113 +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 { - 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 { - // 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) - && let Some(parent_span) = is_called_from_map_like(cx, expr) - { - suggest_cloned_string_to_string(cx, parent_span); - } - }, - 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/tests/ui/deprecated.rs b/tests/ui/deprecated.rs index 6b69bdd29cea..9743a83fb934 100644 --- 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 index 07e59d33d608..3571ca960c75 100644 --- 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` and `clippy:map_clone` cover 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/string_to_string.fixed b/tests/ui/string_to_string.fixed deleted file mode 100644 index 751f451cb685..000000000000 --- a/tests/ui/string_to_string.fixed +++ /dev/null @@ -1,21 +0,0 @@ -#![warn(clippy::implicit_clone, clippy::string_to_string)] -#![allow(clippy::redundant_clone, clippy::unnecessary_literal_unwrap)] - -fn main() { - let mut message = String::from("Hello"); - let mut v = message.clone(); - //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type - - let variable1 = String::new(); - let v = &variable1; - let variable2 = Some(v); - let _ = variable2.map(|x| { - println!(); - x.clone() - //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type - }); - - let x = Some(String::new()); - let _ = x.unwrap_or_else(|| v.clone()); - //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type -} diff --git a/tests/ui/string_to_string.rs b/tests/ui/string_to_string.rs deleted file mode 100644 index d519677d59ec..000000000000 --- a/tests/ui/string_to_string.rs +++ /dev/null @@ -1,21 +0,0 @@ -#![warn(clippy::implicit_clone, clippy::string_to_string)] -#![allow(clippy::redundant_clone, clippy::unnecessary_literal_unwrap)] - -fn main() { - let mut message = String::from("Hello"); - let mut v = message.to_string(); - //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type - - let variable1 = String::new(); - let v = &variable1; - let variable2 = Some(v); - let _ = variable2.map(|x| { - println!(); - x.to_string() - //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type - }); - - let x = Some(String::new()); - let _ = x.unwrap_or_else(|| v.to_string()); - //~^ ERROR: implicitly cloning a `String` by calling `to_string` on its dereferenced type -} diff --git a/tests/ui/string_to_string.stderr b/tests/ui/string_to_string.stderr deleted file mode 100644 index 220fe3170c6e..000000000000 --- a/tests/ui/string_to_string.stderr +++ /dev/null @@ -1,23 +0,0 @@ -error: implicitly cloning a `String` by calling `to_string` on its dereferenced type - --> tests/ui/string_to_string.rs:6:17 - | -LL | let mut v = message.to_string(); - | ^^^^^^^^^^^^^^^^^^^ help: consider using: `message.clone()` - | - = note: `-D clippy::implicit-clone` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::implicit_clone)]` - -error: implicitly cloning a `String` by calling `to_string` on its dereferenced type - --> tests/ui/string_to_string.rs:14:9 - | -LL | x.to_string() - | ^^^^^^^^^^^^^ help: consider using: `x.clone()` - -error: implicitly cloning a `String` by calling `to_string` on its dereferenced type - --> tests/ui/string_to_string.rs:19:33 - | -LL | let _ = x.unwrap_or_else(|| v.to_string()); - | ^^^^^^^^^^^^^ help: consider using: `v.clone()` - -error: aborting due to 3 previous errors -