refactor(suspicious_to_owned): improve lint messages (#16376)

changelog: [`suspicious_to_owned`]: improve lint messages

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);
                     }
                 },

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;

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
+}

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
+}

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)]

tests/ui/suspicious_to_owned.stderr

@@ -1,71 +1,71 @@
-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
+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();
    |             ^^^^^^^^^^^^^^
    |
    = 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
-  --> tests/ui/suspicious_to_owned.rs:29:13
+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<char>> itself and does not cause the Cow<'_, Vec<char>> contents to become owned
-  --> tests/ui/suspicious_to_owned.rs:41:13
+error: this `to_owned` call clones the `Cow<'_, Vec<char>>` 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
-  --> tests/ui/suspicious_to_owned.rs:53:13
+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();
    |
 
 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()`