From 9eb4d7c56ebf768576183e5c5fd3e8159520dd22 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 10 Jan 2026 13:58:47 +0100 Subject: [PATCH 1/2] remove `no-rustfix` the suggestions are no longer overlapping?.. --- tests/ui/suspicious_to_owned.1.fixed | 73 ++++++++++++++++++++++++++++ tests/ui/suspicious_to_owned.2.fixed | 73 ++++++++++++++++++++++++++++ tests/ui/suspicious_to_owned.rs | 1 - tests/ui/suspicious_to_owned.stderr | 12 ++--- 4 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 tests/ui/suspicious_to_owned.1.fixed create mode 100644 tests/ui/suspicious_to_owned.2.fixed diff --git a/tests/ui/suspicious_to_owned.1.fixed b/tests/ui/suspicious_to_owned.1.fixed new file mode 100644 index 000000000000..6fd536a38ed1 --- /dev/null +++ b/tests/ui/suspicious_to_owned.1.fixed @@ -0,0 +1,73 @@ +#![warn(clippy::suspicious_to_owned)] +#![warn(clippy::implicit_clone)] +#![allow(clippy::redundant_clone)] +use std::borrow::Cow; +use std::ffi::{CStr, c_char}; + +fn main() { + let moo = "Moooo"; + let c_moo = b"Moooo\0"; + let c_moo_ptr = c_moo.as_ptr() as *const c_char; + let moos = ['M', 'o', 'o']; + let moos_vec = moos.to_vec(); + + // we expect this to be linted + let cow = Cow::Borrowed(moo); + let _ = cow.into_owned(); + //~^ suspicious_to_owned + + // we expect no lints for this + let cow = Cow::Borrowed(moo); + let _ = cow.into_owned(); + // we expect no lints for this + let cow = Cow::Borrowed(moo); + let _ = cow.clone(); + + // we expect this to be linted + let cow = Cow::Borrowed(&moos); + let _ = cow.into_owned(); + //~^ suspicious_to_owned + + // we expect no lints for this + let cow = Cow::Borrowed(&moos); + let _ = cow.into_owned(); + // we expect no lints for this + let cow = Cow::Borrowed(&moos); + let _ = cow.clone(); + + // we expect this to be linted + let cow = Cow::Borrowed(&moos_vec); + let _ = cow.into_owned(); + //~^ suspicious_to_owned + + // we expect no lints for this + let cow = Cow::Borrowed(&moos_vec); + let _ = cow.into_owned(); + // we expect no lints for this + let cow = Cow::Borrowed(&moos_vec); + let _ = cow.clone(); + + // we expect this to be linted + let cow = unsafe { CStr::from_ptr(c_moo_ptr) }.to_string_lossy(); + let _ = cow.into_owned(); + //~^ suspicious_to_owned + + // we expect no lints for this + let cow = unsafe { CStr::from_ptr(c_moo_ptr) }.to_string_lossy(); + let _ = cow.into_owned(); + // we expect no lints for this + let cow = unsafe { CStr::from_ptr(c_moo_ptr) }.to_string_lossy(); + let _ = cow.clone(); + + // we expect no lints for these + let _ = moo.to_owned(); + let _ = c_moo.to_owned(); + let _ = moos.to_owned(); + + // we expect implicit_clone lints for these + let _ = String::from(moo).clone(); + //~^ implicit_clone + + let _ = moos_vec.clone(); + //~^ implicit_clone +} diff --git a/tests/ui/suspicious_to_owned.2.fixed b/tests/ui/suspicious_to_owned.2.fixed new file mode 100644 index 000000000000..841adf8ea274 --- /dev/null +++ b/tests/ui/suspicious_to_owned.2.fixed @@ -0,0 +1,73 @@ +#![warn(clippy::suspicious_to_owned)] +#![warn(clippy::implicit_clone)] +#![allow(clippy::redundant_clone)] +use std::borrow::Cow; +use std::ffi::{CStr, c_char}; + +fn main() { + let moo = "Moooo"; + let c_moo = b"Moooo\0"; + let c_moo_ptr = c_moo.as_ptr() as *const c_char; + let moos = ['M', 'o', 'o']; + let moos_vec = moos.to_vec(); + + // we expect this to be linted + let cow = Cow::Borrowed(moo); + let _ = cow.clone(); + //~^ suspicious_to_owned + + // we expect no lints for this + let cow = Cow::Borrowed(moo); + let _ = cow.into_owned(); + // we expect no lints for this + let cow = Cow::Borrowed(moo); + let _ = cow.clone(); + + // we expect this to be linted + let cow = Cow::Borrowed(&moos); + let _ = cow.clone(); + //~^ suspicious_to_owned + + // we expect no lints for this + let cow = Cow::Borrowed(&moos); + let _ = cow.into_owned(); + // we expect no lints for this + let cow = Cow::Borrowed(&moos); + let _ = cow.clone(); + + // we expect this to be linted + let cow = Cow::Borrowed(&moos_vec); + let _ = cow.clone(); + //~^ suspicious_to_owned + + // we expect no lints for this + let cow = Cow::Borrowed(&moos_vec); + let _ = cow.into_owned(); + // we expect no lints for this + let cow = Cow::Borrowed(&moos_vec); + let _ = cow.clone(); + + // we expect this to be linted + let cow = unsafe { CStr::from_ptr(c_moo_ptr) }.to_string_lossy(); + let _ = cow.clone(); + //~^ suspicious_to_owned + + // we expect no lints for this + let cow = unsafe { CStr::from_ptr(c_moo_ptr) }.to_string_lossy(); + let _ = cow.into_owned(); + // we expect no lints for this + let cow = unsafe { CStr::from_ptr(c_moo_ptr) }.to_string_lossy(); + let _ = cow.clone(); + + // we expect no lints for these + let _ = moo.to_owned(); + let _ = c_moo.to_owned(); + let _ = moos.to_owned(); + + // we expect implicit_clone lints for these + let _ = String::from(moo).clone(); + //~^ implicit_clone + + let _ = moos_vec.clone(); + //~^ implicit_clone +} diff --git a/tests/ui/suspicious_to_owned.rs b/tests/ui/suspicious_to_owned.rs index 2eec05ccaf4c..f59b3fd6ed0c 100644 --- a/tests/ui/suspicious_to_owned.rs +++ b/tests/ui/suspicious_to_owned.rs @@ -1,4 +1,3 @@ -//@no-rustfix: overlapping suggestions #![warn(clippy::suspicious_to_owned)] #![warn(clippy::implicit_clone)] #![allow(clippy::redundant_clone)] diff --git a/tests/ui/suspicious_to_owned.stderr b/tests/ui/suspicious_to_owned.stderr index f90bea5fb8ff..1e631d4d4a14 100644 --- a/tests/ui/suspicious_to_owned.stderr +++ b/tests/ui/suspicious_to_owned.stderr @@ -1,5 +1,5 @@ error: this `to_owned` call clones the Cow<'_, str> itself and does not cause the Cow<'_, str> contents to become owned - --> tests/ui/suspicious_to_owned.rs:17:13 + --> tests/ui/suspicious_to_owned.rs:16:13 | LL | let _ = cow.to_owned(); | ^^^^^^^^^^^^^^ @@ -17,7 +17,7 @@ LL + let _ = cow.clone(); | error: this `to_owned` call clones the Cow<'_, [char; 3]> itself and does not cause the Cow<'_, [char; 3]> contents to become owned - --> tests/ui/suspicious_to_owned.rs:29:13 + --> tests/ui/suspicious_to_owned.rs:28:13 | LL | let _ = cow.to_owned(); | ^^^^^^^^^^^^^^ @@ -33,7 +33,7 @@ LL + let _ = cow.clone(); | error: this `to_owned` call clones the Cow<'_, Vec> itself and does not cause the Cow<'_, Vec> contents to become owned - --> tests/ui/suspicious_to_owned.rs:41:13 + --> tests/ui/suspicious_to_owned.rs:40:13 | LL | let _ = cow.to_owned(); | ^^^^^^^^^^^^^^ @@ -49,7 +49,7 @@ LL + let _ = cow.clone(); | error: this `to_owned` call clones the Cow<'_, str> itself and does not cause the Cow<'_, str> contents to become owned - --> tests/ui/suspicious_to_owned.rs:53:13 + --> tests/ui/suspicious_to_owned.rs:52:13 | LL | let _ = cow.to_owned(); | ^^^^^^^^^^^^^^ @@ -65,7 +65,7 @@ LL + let _ = cow.clone(); | error: implicitly cloning a `String` by calling `to_owned` on its dereferenced type - --> tests/ui/suspicious_to_owned.rs:69:13 + --> tests/ui/suspicious_to_owned.rs:68:13 | LL | let _ = String::from(moo).to_owned(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `String::from(moo).clone()` @@ -74,7 +74,7 @@ LL | let _ = String::from(moo).to_owned(); = help: to override `-D warnings` add `#[allow(clippy::implicit_clone)]` error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type - --> tests/ui/suspicious_to_owned.rs:72:13 + --> tests/ui/suspicious_to_owned.rs:71:13 | LL | let _ = moos_vec.to_owned(); | ^^^^^^^^^^^^^^^^^^^ help: consider using: `moos_vec.clone()` From f228056c644969488958f01cb88a89f0a93f642d Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 10 Jan 2026 13:51:44 +0100 Subject: [PATCH 2/2] improve lint messages - only mention the type once - put types in backticks - only highlight the method name in the suggestion - removes the need for a snippet - makes for finer diffs (seen through `cargo dev lint`) --- clippy_lints/src/methods/mod.rs | 2 +- .../src/methods/suspicious_to_owned.rs | 23 +++++++----------- tests/ui/suspicious_to_owned.stderr | 24 +++++++++---------- 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 248a147cfd77..d483496549fd 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -5576,7 +5576,7 @@ impl Methods { unnecessary_fallible_conversions::check_method(cx, expr); }, (sym::to_owned, []) => { - if !suspicious_to_owned::check(cx, expr, recv) { + if !suspicious_to_owned::check(cx, expr, span) { implicit_clone::check(cx, name, expr, recv); } }, diff --git a/clippy_lints/src/methods/suspicious_to_owned.rs b/clippy_lints/src/methods/suspicious_to_owned.rs index bcd1f11931fc..e9a5104eb3b4 100644 --- a/clippy_lints/src/methods/suspicious_to_owned.rs +++ b/clippy_lints/src/methods/suspicious_to_owned.rs @@ -1,15 +1,14 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::res::MaybeDef; -use clippy_utils::source::snippet_with_context; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_middle::ty::print::with_forced_trimmed_paths; -use rustc_span::sym; +use rustc_span::{Span, sym}; use super::SUSPICIOUS_TO_OWNED; -pub fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) -> bool { +pub fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_span: Span) -> bool { if cx .typeck_results() .type_dependent_def_id(expr.hir_id) @@ -18,28 +17,22 @@ pub fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) - && let input_type = cx.typeck_results().expr_ty(expr) && input_type.is_diag_item(cx, sym::Cow) { - let mut app = Applicability::MaybeIncorrect; - let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "..", &mut app).0; + let app = Applicability::MaybeIncorrect; span_lint_and_then( cx, SUSPICIOUS_TO_OWNED, expr.span, with_forced_trimmed_paths!(format!( - "this `to_owned` call clones the {input_type} itself and does not cause the {input_type} contents to become owned" + "this `to_owned` call clones the `{input_type}` itself and does not cause its contents to become owned" )), |diag| { diag.span_suggestion( - expr.span, - "depending on intent, either make the Cow an Owned variant", - format!("{recv_snip}.into_owned()"), - app, - ); - diag.span_suggestion( - expr.span, - "or clone the Cow itself", - format!("{recv_snip}.clone()"), + method_span, + "depending on intent, either make the `Cow` an `Owned` variant", + "into_owned".to_string(), app, ); + diag.span_suggestion(method_span, "or clone the `Cow` itself", "clone".to_string(), app); }, ); return true; diff --git a/tests/ui/suspicious_to_owned.stderr b/tests/ui/suspicious_to_owned.stderr index 1e631d4d4a14..5fda1ed1cc9c 100644 --- a/tests/ui/suspicious_to_owned.stderr +++ b/tests/ui/suspicious_to_owned.stderr @@ -1,4 +1,4 @@ -error: this `to_owned` call clones the Cow<'_, str> itself and does not cause the Cow<'_, str> contents to become owned +error: this `to_owned` call clones the `Cow<'_, str>` itself and does not cause its contents to become owned --> tests/ui/suspicious_to_owned.rs:16:13 | LL | let _ = cow.to_owned(); @@ -6,59 +6,59 @@ LL | let _ = cow.to_owned(); | = note: `-D clippy::suspicious-to-owned` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::suspicious_to_owned)]` -help: depending on intent, either make the Cow an Owned variant +help: depending on intent, either make the `Cow` an `Owned` variant | LL | let _ = cow.into_owned(); | ++ -help: or clone the Cow itself +help: or clone the `Cow` itself | LL - let _ = cow.to_owned(); LL + let _ = cow.clone(); | -error: this `to_owned` call clones the Cow<'_, [char; 3]> itself and does not cause the Cow<'_, [char; 3]> contents to become owned +error: this `to_owned` call clones the `Cow<'_, [char; 3]>` itself and does not cause its contents to become owned --> tests/ui/suspicious_to_owned.rs:28:13 | LL | let _ = cow.to_owned(); | ^^^^^^^^^^^^^^ | -help: depending on intent, either make the Cow an Owned variant +help: depending on intent, either make the `Cow` an `Owned` variant | LL | let _ = cow.into_owned(); | ++ -help: or clone the Cow itself +help: or clone the `Cow` itself | LL - let _ = cow.to_owned(); LL + let _ = cow.clone(); | -error: this `to_owned` call clones the Cow<'_, Vec> itself and does not cause the Cow<'_, Vec> contents to become owned +error: this `to_owned` call clones the `Cow<'_, Vec>` itself and does not cause its contents to become owned --> tests/ui/suspicious_to_owned.rs:40:13 | LL | let _ = cow.to_owned(); | ^^^^^^^^^^^^^^ | -help: depending on intent, either make the Cow an Owned variant +help: depending on intent, either make the `Cow` an `Owned` variant | LL | let _ = cow.into_owned(); | ++ -help: or clone the Cow itself +help: or clone the `Cow` itself | LL - let _ = cow.to_owned(); LL + let _ = cow.clone(); | -error: this `to_owned` call clones the Cow<'_, str> itself and does not cause the Cow<'_, str> contents to become owned +error: this `to_owned` call clones the `Cow<'_, str>` itself and does not cause its contents to become owned --> tests/ui/suspicious_to_owned.rs:52:13 | LL | let _ = cow.to_owned(); | ^^^^^^^^^^^^^^ | -help: depending on intent, either make the Cow an Owned variant +help: depending on intent, either make the `Cow` an `Owned` variant | LL | let _ = cow.into_owned(); | ++ -help: or clone the Cow itself +help: or clone the `Cow` itself | LL - let _ = cow.to_owned(); LL + let _ = cow.clone();