diff options
| -rw-r--r-- | clippy_lints/src/methods/unnecessary_to_owned.rs | 46 | ||||
| -rw-r--r-- | tests/ui/unnecessary_to_owned.fixed | 48 | ||||
| -rw-r--r-- | tests/ui/unnecessary_to_owned.rs | 48 | ||||
| -rw-r--r-- | tests/ui/unnecessary_to_owned.stderr | 8 |
4 files changed, 126 insertions, 24 deletions
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs index 7916fb8e3b4..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}; @@ -114,7 +116,12 @@ fn check_addr_of_expr( parent.span, &format!("unnecessary use of `{}`", method_name), "use", - format!("{:&>width$}{}", "", receiver_snippet, width = n_target_refs - n_receiver_refs), + format!( + "{:&>width$}{}", + "", + receiver_snippet, + width = n_target_refs - n_receiver_refs + ), Applicability::MachineApplicable, ); return true; @@ -182,20 +189,10 @@ fn check_into_iter_call_arg(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: if let Some(item_ty) = get_iterator_item_ty(cx, parent_ty); if let Some(receiver_snippet) = snippet_opt(cx, receiver.span); then { - if unnecessary_iter_cloned::check_for_loop_iter( - cx, - parent, - method_name, - receiver, - true, - ) { + if unnecessary_iter_cloned::check_for_loop_iter(cx, parent, method_name, receiver, true) { return true; } - let cloned_or_copied = if is_copy(cx, item_ty) { - "copied" - } else { - "cloned" - }; + let cloned_or_copied = if is_copy(cx, item_ty) { "copied" } else { "cloned" }; // The next suggestion may be incorrect because the removal of the `to_owned`-like // function could cause the iterator to hold a reference to a resource that is used // mutably. See https://github.com/rust-lang/rust-clippy/issues/8148. @@ -243,10 +240,11 @@ fn check_other_call_arg<'tcx>( if if trait_predicate.def_id() == deref_trait_id { if let [projection_predicate] = projection_predicates[..] { let normalized_ty = - cx.tcx.subst_and_normalize_erasing_regions(call_substs, cx.param_env, projection_predicate.term); + cx.tcx + .subst_and_normalize_erasing_regions(call_substs, cx.param_env, projection_predicate.term); implements_trait(cx, receiver_ty, deref_trait_id, &[]) - && get_associated_type(cx, receiver_ty, deref_trait_id, - "Target").map_or(false, |ty| ty::Term::Ty(ty) == normalized_ty) + && get_associated_type(cx, receiver_ty, deref_trait_id, "Target") + .map_or(false, |ty| ty::Term::Ty(ty) == normalized_ty) } else { false } @@ -254,7 +252,7 @@ fn check_other_call_arg<'tcx>( let composed_substs = compose_substs( cx, &trait_predicate.trait_ref.substs.iter().skip(1).collect::<Vec<_>>()[..], - call_substs + call_substs, ); implements_trait(cx, receiver_ty, as_ref_trait_id, &composed_substs) } else { @@ -264,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( @@ -339,11 +343,7 @@ fn get_input_traits_and_projections<'tcx>( if let Some(arg) = substs.iter().next(); if let GenericArgKind::Type(arg_ty) = arg.unpack(); if arg_ty == input; - then { - true - } else { - false - } + then { true } else { false } } }; match predicate.kind().skip_binder() { 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 |