about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel E. Moelius III <sam@moeli.us>2022-03-05 21:18:44 -0500
committerSamuel E. Moelius III <sam@moeli.us>2022-03-05 21:18:44 -0500
commit1a95590faf2a81e7b7bae697c2209ba7607f0336 (patch)
tree812a4365e70a0b8b8c49278f9b3bd740d509150b
parentce841fe73b1476f6d4ac1495f88a567c501d01a8 (diff)
downloadrust-1a95590faf2a81e7b7bae697c2209ba7607f0336.tar.gz
rust-1a95590faf2a81e7b7bae697c2209ba7607f0336.zip
Fix #8507
-rw-r--r--clippy_lints/src/methods/unnecessary_to_owned.rs10
-rw-r--r--tests/ui/unnecessary_to_owned.fixed48
-rw-r--r--tests/ui/unnecessary_to_owned.rs48
-rw-r--r--tests/ui/unnecessary_to_owned.stderr8
4 files changed, 112 insertions, 2 deletions
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs
index cc9949b153f..1555758fc4a 100644
--- a/clippy_lints/src/methods/unnecessary_to_owned.rs
+++ b/clippy_lints/src/methods/unnecessary_to_owned.rs
@@ -2,7 +2,9 @@ use super::implicit_clone::is_clone_like;
 use super::unnecessary_iter_cloned::{self, is_into_iter};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_opt;
-use clippy_utils::ty::{get_associated_type, get_iterator_item_ty, implements_trait, is_copy, peel_mid_ty_refs};
+use clippy_utils::ty::{
+    contains_ty, get_associated_type, get_iterator_item_ty, implements_trait, is_copy, peel_mid_ty_refs,
+};
 use clippy_utils::{fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item};
 use rustc_errors::Applicability;
 use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind};
@@ -260,6 +262,12 @@ fn check_other_call_arg<'tcx>(
         // `Target = T`.
         if n_refs > 0 || is_copy(cx, receiver_ty) || trait_predicate.def_id() != deref_trait_id;
         let n_refs = max(n_refs, if is_copy(cx, receiver_ty) { 0 } else { 1 });
+        // If the trait is `AsRef` and the input type variable `T` occurs in the output type, then
+        // `T` must not be instantiated with a reference
+        // (https://github.com/rust-lang/rust-clippy/issues/8507).
+        if (n_refs == 0 && !receiver_ty.is_ref())
+            || trait_predicate.def_id() != as_ref_trait_id
+            || !contains_ty(fn_sig.output(), input);
         if let Some(receiver_snippet) = snippet_opt(cx, receiver.span);
         then {
             span_lint_and_sugg(
diff --git a/tests/ui/unnecessary_to_owned.fixed b/tests/ui/unnecessary_to_owned.fixed
index 720138db137..38ba41ac54e 100644
--- a/tests/ui/unnecessary_to_owned.fixed
+++ b/tests/ui/unnecessary_to_owned.fixed
@@ -212,3 +212,51 @@ fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::E
 }
 
 fn require_string(_: &String) {}
+
+// https://github.com/rust-lang/rust-clippy/issues/8507
+mod issue_8507 {
+    #![allow(dead_code)]
+
+    struct Opaque<P>(P);
+
+    pub trait Abstracted {}
+
+    impl<P> Abstracted for Opaque<P> {}
+
+    fn build<P>(p: P) -> Opaque<P>
+    where
+        P: AsRef<str>,
+    {
+        Opaque(p)
+    }
+
+    // Should not lint.
+    fn test_str(s: &str) -> Box<dyn Abstracted> {
+        Box::new(build(s.to_string()))
+    }
+
+    // Should not lint.
+    fn test_x(x: super::X) -> Box<dyn Abstracted> {
+        Box::new(build(x))
+    }
+
+    #[derive(Clone, Copy)]
+    struct Y(&'static str);
+
+    impl AsRef<str> for Y {
+        fn as_ref(&self) -> &str {
+            self.0
+        }
+    }
+
+    impl ToString for Y {
+        fn to_string(&self) -> String {
+            self.0.to_string()
+        }
+    }
+
+    // Should lint because Y is copy.
+    fn test_y(y: Y) -> Box<dyn Abstracted> {
+        Box::new(build(y))
+    }
+}
diff --git a/tests/ui/unnecessary_to_owned.rs b/tests/ui/unnecessary_to_owned.rs
index 60b2e718f5d..15fb7ee83e3 100644
--- a/tests/ui/unnecessary_to_owned.rs
+++ b/tests/ui/unnecessary_to_owned.rs
@@ -212,3 +212,51 @@ fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::E
 }
 
 fn require_string(_: &String) {}
+
+// https://github.com/rust-lang/rust-clippy/issues/8507
+mod issue_8507 {
+    #![allow(dead_code)]
+
+    struct Opaque<P>(P);
+
+    pub trait Abstracted {}
+
+    impl<P> Abstracted for Opaque<P> {}
+
+    fn build<P>(p: P) -> Opaque<P>
+    where
+        P: AsRef<str>,
+    {
+        Opaque(p)
+    }
+
+    // Should not lint.
+    fn test_str(s: &str) -> Box<dyn Abstracted> {
+        Box::new(build(s.to_string()))
+    }
+
+    // Should not lint.
+    fn test_x(x: super::X) -> Box<dyn Abstracted> {
+        Box::new(build(x))
+    }
+
+    #[derive(Clone, Copy)]
+    struct Y(&'static str);
+
+    impl AsRef<str> for Y {
+        fn as_ref(&self) -> &str {
+            self.0
+        }
+    }
+
+    impl ToString for Y {
+        fn to_string(&self) -> String {
+            self.0.to_string()
+        }
+    }
+
+    // Should lint because Y is copy.
+    fn test_y(y: Y) -> Box<dyn Abstracted> {
+        Box::new(build(y.to_string()))
+    }
+}
diff --git a/tests/ui/unnecessary_to_owned.stderr b/tests/ui/unnecessary_to_owned.stderr
index 1dfc65e22e2..c53ce32be77 100644
--- a/tests/ui/unnecessary_to_owned.stderr
+++ b/tests/ui/unnecessary_to_owned.stderr
@@ -491,5 +491,11 @@ LL -         let path = match get_file_path(&t) {
 LL +         let path = match get_file_path(t) {
    | 
 
-error: aborting due to 76 previous errors
+error: unnecessary use of `to_string`
+  --> $DIR/unnecessary_to_owned.rs:260:24
+   |
+LL |         Box::new(build(y.to_string()))
+   |                        ^^^^^^^^^^^^^ help: use: `y`
+
+error: aborting due to 77 previous errors