Auto merge of #9796 - smoelius:issue-9771, r=flip1995

Fix #9771 (`unnecessary_to_owned` false positive)

Fixes #9771

In that issue's example(s), the lint tried to add a `&` to a value, which implicitly changed the type of a field to a reference. The fix is to add the reference to `receiver_ty` (the type of the receiver of the `to_owned`-like method), before passing `receiver_ty` to `can_change_type`. `can_change_type` properly rejects the modified `receiver_ty`.

cc: `@mikerite` just because I think he was the author of `can_change_type`.

changelog: fix `unnecessary_to_owned` false positive which implicitly tried to change the type of a field to a reference
This commit is contained in:
bors 2022-11-22 12:50:08 +00:00
commit 94ce4465e5
3 changed files with 72 additions and 4 deletions

View File

@ -19,7 +19,6 @@
use rustc_semver::RustcVersion; use rustc_semver::RustcVersion;
use rustc_span::{sym, Symbol}; use rustc_span::{sym, Symbol};
use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause}; use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause};
use std::cmp::max;
use super::UNNECESSARY_TO_OWNED; use super::UNNECESSARY_TO_OWNED;
@ -263,11 +262,22 @@ fn check_other_call_arg<'tcx>(
if let Some(as_ref_trait_id) = cx.tcx.get_diagnostic_item(sym::AsRef); if let Some(as_ref_trait_id) = cx.tcx.get_diagnostic_item(sym::AsRef);
if trait_predicate.def_id() == deref_trait_id || trait_predicate.def_id() == as_ref_trait_id; if trait_predicate.def_id() == deref_trait_id || trait_predicate.def_id() == as_ref_trait_id;
let receiver_ty = cx.typeck_results().expr_ty(receiver); let receiver_ty = cx.typeck_results().expr_ty(receiver);
if can_change_type(cx, maybe_arg, receiver_ty);
// We can't add an `&` when the trait is `Deref` because `Target = &T` won't match // We can't add an `&` when the trait is `Deref` because `Target = &T` won't match
// `Target = T`. // `Target = T`.
if n_refs > 0 || is_copy(cx, receiver_ty) || trait_predicate.def_id() != deref_trait_id; if let Some((n_refs, receiver_ty)) = if n_refs > 0 || is_copy(cx, receiver_ty) {
let n_refs = max(n_refs, usize::from(!is_copy(cx, receiver_ty))); Some((n_refs, receiver_ty))
} else if trait_predicate.def_id() != deref_trait_id {
Some((1, cx.tcx.mk_ref(
cx.tcx.lifetimes.re_erased,
ty::TypeAndMut {
ty: receiver_ty,
mutbl: Mutability::Not,
},
)))
} else {
None
};
if can_change_type(cx, maybe_arg, receiver_ty);
if let Some(receiver_snippet) = snippet_opt(cx, receiver.span); if let Some(receiver_snippet) = snippet_opt(cx, receiver.span);
then { then {
span_lint_and_sugg( span_lint_and_sugg(

View File

@ -426,3 +426,32 @@ mod issue_9504 {
foo(std::path::PathBuf::new().to_string_lossy().to_string()).await; foo(std::path::PathBuf::new().to_string_lossy().to_string()).await;
} }
} }
mod issue_9771a {
#![allow(dead_code)]
use std::marker::PhantomData;
pub struct Key<K: AsRef<[u8]>, V: ?Sized>(K, PhantomData<V>);
impl<K: AsRef<[u8]>, V: ?Sized> Key<K, V> {
pub fn new(key: K) -> Key<K, V> {
Key(key, PhantomData)
}
}
pub fn pkh(pkh: &[u8]) -> Key<Vec<u8>, String> {
Key::new([b"pkh-", pkh].concat().to_vec())
}
}
mod issue_9771b {
#![allow(dead_code)]
pub struct Key<K: AsRef<[u8]>>(K);
pub fn from(c: &[u8]) -> Key<Vec<u8>> {
let v = [c].concat();
Key(v.to_vec())
}
}

View File

@ -426,3 +426,32 @@ async fn bar() {
foo(std::path::PathBuf::new().to_string_lossy().to_string()).await; foo(std::path::PathBuf::new().to_string_lossy().to_string()).await;
} }
} }
mod issue_9771a {
#![allow(dead_code)]
use std::marker::PhantomData;
pub struct Key<K: AsRef<[u8]>, V: ?Sized>(K, PhantomData<V>);
impl<K: AsRef<[u8]>, V: ?Sized> Key<K, V> {
pub fn new(key: K) -> Key<K, V> {
Key(key, PhantomData)
}
}
pub fn pkh(pkh: &[u8]) -> Key<Vec<u8>, String> {
Key::new([b"pkh-", pkh].concat().to_vec())
}
}
mod issue_9771b {
#![allow(dead_code)]
pub struct Key<K: AsRef<[u8]>>(K);
pub fn from(c: &[u8]) -> Key<Vec<u8>> {
let v = [c].concat();
Key(v.to_vec())
}
}