From dd084940de3947b7d095fcbd6d3381f98470c58b Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Fri, 9 Jun 2023 04:13:00 +0200 Subject: [PATCH] check that the adjusted receiver type matches target --- .../src/methods/unnecessary_to_owned.rs | 5 +++ tests/ui/unnecessary_to_owned.fixed | 33 +++++++++++++++++++ tests/ui/unnecessary_to_owned.rs | 33 +++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs index 309d2157b76e..da7196ed0a81 100644 --- a/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -144,6 +144,11 @@ fn check_addr_of_expr( if let Some(deref_trait_id) = cx.tcx.get_diagnostic_item(sym::Deref); if implements_trait(cx, receiver_ty, deref_trait_id, &[]); if cx.get_associated_type(receiver_ty, deref_trait_id, "Target") == Some(target_ty); + // Make sure that it's actually calling the right `.to_string()`, (#10033) + // *or* this is a `Cow::into_owned()` call (which would be the wrong into_owned receiver (str != Cow) + // but that's ok for Cow::into_owned specifically) + if cx.typeck_results().expr_ty_adjusted(receiver).peel_refs() == target_ty + || is_cow_into_owned(cx, method_name, method_def_id); then { if n_receiver_refs > 0 { span_lint_and_sugg( diff --git a/tests/ui/unnecessary_to_owned.fixed b/tests/ui/unnecessary_to_owned.fixed index 08733906b0ed..592a53f3a819 100644 --- a/tests/ui/unnecessary_to_owned.fixed +++ b/tests/ui/unnecessary_to_owned.fixed @@ -474,3 +474,36 @@ mod issue_10021 { Ok(()) } } + +mod issue_10033 { + #![allow(dead_code)] + use std::{fmt::Display, ops::Deref}; + + fn _main() { + let f = Foo; + + // Not actually unnecessary - this calls `Foo`'s `Display` impl, not `str`'s (even though `Foo` does + // deref to `str`) + foo(&f.to_string()); + } + + fn foo(s: &str) { + println!("{}", s); + } + + struct Foo; + + impl Deref for Foo { + type Target = str; + + fn deref(&self) -> &Self::Target { + "str" + } + } + + impl Display for Foo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Foo") + } + } +} diff --git a/tests/ui/unnecessary_to_owned.rs b/tests/ui/unnecessary_to_owned.rs index e3589ea0d65e..f2e48b1c4a6a 100644 --- a/tests/ui/unnecessary_to_owned.rs +++ b/tests/ui/unnecessary_to_owned.rs @@ -474,3 +474,36 @@ mod issue_10021 { Ok(()) } } + +mod issue_10033 { + #![allow(dead_code)] + use std::{fmt::Display, ops::Deref}; + + fn _main() { + let f = Foo; + + // Not actually unnecessary - this calls `Foo`'s `Display` impl, not `str`'s (even though `Foo` does + // deref to `str`) + foo(&f.to_string()); + } + + fn foo(s: &str) { + println!("{}", s); + } + + struct Foo; + + impl Deref for Foo { + type Target = str; + + fn deref(&self) -> &Self::Target { + "str" + } + } + + impl Display for Foo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Foo") + } + } +}