From 468c86e4a3fa31b8c670422ea63875428e899b7f Mon Sep 17 00:00:00 2001 From: "Samuel E. Moelius III" Date: Mon, 15 Nov 2021 19:25:08 -0500 Subject: [PATCH] Add `unnecessary_to_owned` lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_perf.rs | 1 + clippy_lints/src/methods/mod.rs | 31 +- .../src/methods/unnecessary_to_owned.rs | 328 +++++++++++++++ clippy_lints/src/unit_return_expecting_ord.rs | 2 +- clippy_utils/src/paths.rs | 2 + tests/ui/unnecessary_to_owned.fixed | 170 ++++++++ tests/ui/unnecessary_to_owned.rs | 170 ++++++++ tests/ui/unnecessary_to_owned.stderr | 382 ++++++++++++++++++ 11 files changed, 1087 insertions(+), 2 deletions(-) create mode 100644 clippy_lints/src/methods/unnecessary_to_owned.rs create mode 100644 tests/ui/unnecessary_to_owned.fixed create mode 100644 tests/ui/unnecessary_to_owned.rs create mode 100644 tests/ui/unnecessary_to_owned.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f64cd0e914..7b5279cda6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3210,6 +3210,7 @@ Released 2018-09-13 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation [`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by +[`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap [`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 9ac77d63879..3d3999d4cc0 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -181,6 +181,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::UNNECESSARY_FILTER_MAP), LintId::of(methods::UNNECESSARY_FOLD), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), + LintId::of(methods::UNNECESSARY_TO_OWNED), LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), LintId::of(methods::USELESS_ASREF), LintId::of(methods::WRONG_SELF_CONVENTION), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 68889f4f50a..766c5ba1bcb 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -315,6 +315,7 @@ store.register_lints(&[ methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FOLD, methods::UNNECESSARY_LAZY_EVALUATIONS, + methods::UNNECESSARY_TO_OWNED, methods::UNWRAP_OR_ELSE_DEFAULT, methods::UNWRAP_USED, methods::USELESS_ASREF, diff --git a/clippy_lints/src/lib.register_perf.rs b/clippy_lints/src/lib.register_perf.rs index a0d5cf9418e..2ea0b696f1f 100644 --- a/clippy_lints/src/lib.register_perf.rs +++ b/clippy_lints/src/lib.register_perf.rs @@ -17,6 +17,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(methods::MANUAL_STR_REPEAT), LintId::of(methods::OR_FUN_CALL), LintId::of(methods::SINGLE_CHAR_PATTERN), + LintId::of(methods::UNNECESSARY_TO_OWNED), LintId::of(misc::CMP_OWNED), LintId::of(mutex_atomic::MUTEX_ATOMIC), LintId::of(redundant_clone::REDUNDANT_CLONE), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 58ec2213535..a0de296fd07 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -57,6 +57,7 @@ mod uninit_assumed_init; mod unnecessary_filter_map; mod unnecessary_fold; mod unnecessary_lazy_eval; +mod unnecessary_to_owned; mod unwrap_or_else_default; mod unwrap_used; mod useless_asref; @@ -1885,6 +1886,32 @@ declare_clippy_lint! { "usages of `str::splitn` that can be replaced with `str::split`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for unnecessary calls to [`ToOwned::to_owned`](https://doc.rust-lang.org/std/borrow/trait.ToOwned.html#tymethod.to_owned) + /// and other `to_owned`-like functions. + /// + /// ### Why is this bad? + /// The unnecessary calls result in unnecessary allocations. + /// + /// ### Example + /// ```rust + /// let path = std::path::Path::new("x"); + /// foo(&path.to_string_lossy().to_string()); + /// fn foo(s: &str) {} + /// ``` + /// Use instead: + /// ```rust + /// let path = std::path::Path::new("x"); + /// foo(&path.to_string_lossy()); + /// fn foo(s: &str) {} + /// ``` + #[clippy::version = "1.58.0"] + pub UNNECESSARY_TO_OWNED, + perf, + "unnecessary calls to `to_owned`-like functions" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option, @@ -1964,7 +1991,8 @@ impl_lint_pass!(Methods => [ MANUAL_STR_REPEAT, EXTEND_WITH_DRAIN, MANUAL_SPLIT_ONCE, - NEEDLESS_SPLITN + NEEDLESS_SPLITN, + UNNECESSARY_TO_OWNED ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -2007,6 +2035,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { single_char_add_str::check(cx, expr, args); into_iter_on_ref::check(cx, expr, *method_span, method_call.ident.name, args); single_char_pattern::check(cx, expr, method_call.ident.name, args); + unnecessary_to_owned::check(cx, expr, method_call.ident.name, args); }, hir::ExprKind::Binary(op, lhs, rhs) if op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne => { let mut info = BinaryExprInfo { diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs new file mode 100644 index 00000000000..307de7bb65a --- /dev/null +++ b/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -0,0 +1,328 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_opt; +use clippy_utils::ty::{implements_trait, is_copy, peel_mid_ty_refs}; +use clippy_utils::{get_parent_expr, match_def_path, paths}; +use rustc_errors::Applicability; +use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::mir::Mutability; +use rustc_middle::ty::adjustment::{Adjust, Adjustment, OverloadedDeref}; +use rustc_middle::ty::subst::{GenericArg, GenericArgKind, SubstsRef}; +use rustc_middle::ty::{self, PredicateKind, ProjectionPredicate, TraitPredicate, Ty}; +use rustc_span::{sym, Symbol}; +use std::cmp::max; + +use super::UNNECESSARY_TO_OWNED; + +const TO_OWNED_LIKE_PATHS: &[&[&str]] = &[ + &paths::COW_INTO_OWNED, + &paths::OS_STR_TO_OS_STRING, + &paths::PATH_TO_PATH_BUF, + &paths::SLICE_TO_VEC, + &paths::TO_OWNED_METHOD, + &paths::TO_STRING_METHOD, +]; + +pub fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method_name: Symbol, args: &'tcx [Expr<'tcx>]) { + if_chain! { + if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); + if TO_OWNED_LIKE_PATHS + .iter() + .any(|path| match_def_path(cx, method_def_id, path)); + if let [receiver] = args; + then { + // At this point, we know the call is of a `to_owned`-like function. The functions + // `check_addr_of_expr` and `check_call_arg` determine whether the call is unnecessary + // based on its context, that is, whether it is a referent in an `AddrOf` expression or + // an argument in a function call. + if check_addr_of_expr(cx, expr, method_name, receiver) { + return; + } + check_call_arg(cx, expr, method_name, receiver); + } + } +} + +/// Checks whether `expr` is a referent in an `AddrOf` expression and, if so, determines whether its +/// call of a `to_owned`-like function is unnecessary. +fn check_addr_of_expr( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + method_name: Symbol, + receiver: &'tcx Expr<'tcx>, +) -> bool { + if_chain! { + if let Some(parent) = get_parent_expr(cx, expr); + if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) = parent.kind; + let adjustments = cx.typeck_results().expr_adjustments(parent).iter().collect::>(); + if let Some(target_ty) = match adjustments[..] + { + // For matching uses of `Cow::from` + [ + Adjustment { + kind: Adjust::Deref(None), + .. + }, + Adjustment { + kind: Adjust::Borrow(_), + target: target_ty, + }, + ] + // For matching uses of arrays + | [ + Adjustment { + kind: Adjust::Deref(None), + .. + }, + Adjustment { + kind: Adjust::Borrow(_), + .. + }, + Adjustment { + kind: Adjust::Pointer(_), + target: target_ty, + }, + ] + // For matching everything else + | [ + Adjustment { + kind: Adjust::Deref(None), + .. + }, + Adjustment { + kind: Adjust::Deref(Some(OverloadedDeref { .. })), + .. + }, + Adjustment { + kind: Adjust::Borrow(_), + target: target_ty, + }, + ] => Some(target_ty), + _ => None, + }; + then { + let (target_ty, n_target_refs) = peel_mid_ty_refs(target_ty); + let receiver_ty = cx.typeck_results().expr_ty(receiver); + let (receiver_ty, n_receiver_refs) = peel_mid_ty_refs(receiver_ty); + if_chain! { + if receiver_ty == target_ty; + if n_target_refs >= n_receiver_refs; + if let Some(receiver_snippet) = snippet_opt(cx, receiver.span); + then { + span_lint_and_sugg( + cx, + UNNECESSARY_TO_OWNED, + parent.span, + &format!("unnecessary use of `{}`", method_name), + "use", + format!("{:&>width$}{}", "", receiver_snippet, width = n_target_refs - n_receiver_refs), + Applicability::MachineApplicable, + ); + return true; + } + } + if implements_deref_trait(cx, receiver_ty, target_ty) { + span_lint_and_sugg( + cx, + UNNECESSARY_TO_OWNED, + expr.span.with_lo(receiver.span.hi()), + &format!("unnecessary use of `{}`", method_name), + "remove this", + String::new(), + Applicability::MachineApplicable, + ); + return true; + } + if_chain! { + if let Some(as_ref_trait_id) = cx.tcx.get_diagnostic_item(sym::AsRef); + if implements_trait(cx, receiver_ty, as_ref_trait_id, &[GenericArg::from(target_ty)]); + if let Some(receiver_snippet) = snippet_opt(cx, receiver.span); + then { + span_lint_and_sugg( + cx, + UNNECESSARY_TO_OWNED, + parent.span, + &format!("unnecessary use of `{}`", method_name), + "use", + format!("{}.as_ref()", receiver_snippet), + Applicability::MachineApplicable, + ); + return true; + } + } + } + } + false +} + +/// Checks whether `expr` is an argument in a function call and, if so, determines whether its call +/// of a `to_owned`-like function is unnecessary. +fn check_call_arg(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method_name: Symbol, receiver: &'tcx Expr<'tcx>) { + if_chain! { + if let Some((maybe_call, maybe_arg)) = skip_addr_of_ancestors(cx, expr); + if let Some((callee_def_id, call_substs, call_args)) = get_callee_substs_and_args(cx, maybe_call); + let fn_sig = cx.tcx.fn_sig(callee_def_id).skip_binder(); + if let Some(i) = call_args.iter().position(|arg| arg.hir_id == maybe_arg.hir_id); + if let Some(input) = fn_sig.inputs().get(i); + let (input, n_refs) = peel_mid_ty_refs(input); + if let (trait_predicates, projection_predicates) = get_input_traits_and_projections(cx, callee_def_id, input); + if let Some(sized_def_id) = cx.tcx.lang_items().sized_trait(); + if let [trait_predicate] = trait_predicates + .iter() + .filter(|trait_predicate| trait_predicate.def_id() != sized_def_id) + .collect::>()[..]; + if let Some(deref_trait_id) = cx.tcx.get_diagnostic_item(sym::Deref); + if let Some(as_ref_trait_id) = cx.tcx.get_diagnostic_item(sym::AsRef); + let receiver_ty = cx.typeck_results().expr_ty(receiver); + // If the callee has type parameters, they could appear in `projection_predicate.ty` or the + // types of `trait_predicate.trait_ref.substs`. + 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.ty); + implements_deref_trait(cx, receiver_ty, normalized_ty) + } else { + false + } + } else if trait_predicate.def_id() == as_ref_trait_id { + let composed_substs = compose_substs( + cx, + &trait_predicate.trait_ref.substs.iter().skip(1).collect::>()[..], + call_substs + ); + implements_trait(cx, receiver_ty, as_ref_trait_id, &composed_substs) + } else { + false + }; + // We can't add an `&` when the trait is `Deref` because `Target = &T` won't match + // `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 let Some(receiver_snippet) = snippet_opt(cx, receiver.span); + then { + span_lint_and_sugg( + cx, + UNNECESSARY_TO_OWNED, + maybe_arg.span, + &format!("unnecessary use of `{}`", method_name), + "use", + format!("{:&>width$}{}", "", receiver_snippet, width = n_refs), + Applicability::MachineApplicable, + ); + } + } +} + +/// Walks an expression's ancestors until it finds a non-`AddrOf` expression. Returns the first such +/// expression found (if any) along with the immediately prior expression. +fn skip_addr_of_ancestors( + cx: &LateContext<'tcx>, + mut expr: &'tcx Expr<'tcx>, +) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> { + while let Some(parent) = get_parent_expr(cx, expr) { + if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) = parent.kind { + expr = parent; + } else { + return Some((parent, expr)); + } + } + None +} + +/// Checks whether an expression is a function or method call and, if so, returns its `DefId`, +/// `Substs`, and arguments. +fn get_callee_substs_and_args( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, +) -> Option<(DefId, SubstsRef<'tcx>, &'tcx [Expr<'tcx>])> { + if_chain! { + if let ExprKind::Call(callee, args) = expr.kind; + let callee_ty = cx.typeck_results().expr_ty(callee); + if let ty::FnDef(callee_def_id, _) = callee_ty.kind(); + then { + let substs = cx.typeck_results().node_substs(callee.hir_id); + return Some((*callee_def_id, substs, args)); + } + } + if_chain! { + if let ExprKind::MethodCall(_, _, args, _) = expr.kind; + if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); + then { + let substs = cx.typeck_results().node_substs(expr.hir_id); + return Some((method_def_id, substs, args)); + } + } + None +} + +/// Returns the `TraitPredicate`s and `ProjectionPredicate`s for a function's input type. +fn get_input_traits_and_projections( + cx: &LateContext<'tcx>, + callee_def_id: DefId, + input: Ty<'tcx>, +) -> (Vec>, Vec>) { + let mut trait_predicates = Vec::new(); + let mut projection_predicates = Vec::new(); + for (predicate, _) in cx.tcx.predicates_of(callee_def_id).predicates.iter() { + // `substs` should have 1 + n elements. The first is the type on the left hand side of an + // `as`. The remaining n are trait parameters. + let is_input_substs = |substs: SubstsRef<'tcx>| { + if_chain! { + if let Some(arg) = substs.iter().next(); + if let GenericArgKind::Type(arg_ty) = arg.unpack(); + if arg_ty == input; + then { + true + } else { + false + } + } + }; + match predicate.kind().skip_binder() { + PredicateKind::Trait(trait_predicate) => { + if is_input_substs(trait_predicate.trait_ref.substs) { + trait_predicates.push(trait_predicate); + } + }, + PredicateKind::Projection(projection_predicate) => { + if is_input_substs(projection_predicate.projection_ty.substs) { + projection_predicates.push(projection_predicate); + } + }, + _ => {}, + } + } + (trait_predicates, projection_predicates) +} + +/// Composes two substitutions by applying the latter to the types of the former. +fn compose_substs(cx: &LateContext<'tcx>, left: &[GenericArg<'tcx>], right: SubstsRef<'tcx>) -> Vec> { + left.iter() + .map(|arg| { + if let GenericArgKind::Type(arg_ty) = arg.unpack() { + let normalized_ty = cx.tcx.subst_and_normalize_erasing_regions(right, cx.param_env, arg_ty); + GenericArg::from(normalized_ty) + } else { + *arg + } + }) + .collect() +} + +/// Helper function to check whether a type implements the `Deref` trait. +fn implements_deref_trait(cx: &LateContext<'tcx>, ty: Ty<'tcx>, deref_target_ty: Ty<'tcx>) -> bool { + if_chain! { + if let Some(deref_trait_id) = cx.tcx.get_diagnostic_item(sym::Deref); + if implements_trait(cx, ty, deref_trait_id, &[]); + if let Some(deref_target_id) = cx.tcx.lang_items().deref_target(); + let substs = cx.tcx.mk_substs_trait(ty, &[]); + let projection_ty = cx.tcx.mk_projection(deref_target_id, substs); + let normalized_ty = cx.tcx.normalize_erasing_regions(cx.param_env, projection_ty); + if normalized_ty == deref_target_ty; + then { + true + } else { + false + } + } +} diff --git a/clippy_lints/src/unit_return_expecting_ord.rs b/clippy_lints/src/unit_return_expecting_ord.rs index 9fb8f236899..fe35ff33d35 100644 --- a/clippy_lints/src/unit_return_expecting_ord.rs +++ b/clippy_lints/src/unit_return_expecting_ord.rs @@ -169,7 +169,7 @@ impl<'tcx> LateLintPass<'tcx> for UnitReturnExpectingOrd { trait_name ), Some(last_semi), - &"probably caused by this trailing semicolon".to_string(), + "probably caused by this trailing semicolon", ); }, None => {}, diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 6171823abbb..634a452d6f1 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -36,6 +36,7 @@ pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"]; pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"]; pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"]; pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"]; +pub const COW_INTO_OWNED: [&str; 4] = ["alloc", "borrow", "Cow", "into_owned"]; pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"]; pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"]; pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; @@ -170,6 +171,7 @@ pub const SLICE_FROM_RAW_PARTS: [&str; 4] = ["core", "slice", "raw", "from_raw_p pub const SLICE_FROM_RAW_PARTS_MUT: [&str; 4] = ["core", "slice", "raw", "from_raw_parts_mut"]; pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "", "into_vec"]; pub const SLICE_ITER: [&str; 4] = ["core", "slice", "iter", "Iter"]; +pub const SLICE_TO_VEC: [&str; 4] = ["alloc", "slice", "", "to_vec"]; pub const STDERR: [&str; 4] = ["std", "io", "stdio", "stderr"]; pub const STDOUT: [&str; 4] = ["std", "io", "stdio", "stdout"]; pub const CONVERT_IDENTITY: [&str; 3] = ["core", "convert", "identity"]; diff --git a/tests/ui/unnecessary_to_owned.fixed b/tests/ui/unnecessary_to_owned.fixed new file mode 100644 index 00000000000..f1a5dc91a7e --- /dev/null +++ b/tests/ui/unnecessary_to_owned.fixed @@ -0,0 +1,170 @@ +// run-rustfix + +#![allow(clippy::ptr_arg)] +// Some of the expressions that `redundant_clone` flags overlap with ours. Enabling it interferes +// with `rustfix`. +#![allow(clippy::redundant_clone)] +// `needless_borrow` is for checking the fixed code. +#![warn(clippy::needless_borrow)] +#![warn(clippy::unnecessary_to_owned)] + +use std::borrow::Cow; +use std::ffi::{CStr, OsStr}; +use std::ops::Deref; + +#[derive(Clone)] +struct X(String); + +impl Deref for X { + type Target = [u8]; + fn deref(&self) -> &[u8] { + self.0.as_bytes() + } +} + +impl AsRef for X { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + +impl ToString for X { + fn to_string(&self) -> String { + self.0.to_string() + } +} + +impl X { + fn join(&self, other: impl AsRef) -> Self { + let mut s = self.0.clone(); + s.push_str(other.as_ref()); + Self(s) + } +} + +fn main() { + let c_str = CStr::from_bytes_with_nul(&[0]).unwrap(); + let os_str = OsStr::new("x"); + let path = std::path::Path::new("x"); + let s = "x"; + let array = ["x"]; + let array_ref = &["x"]; + let slice = &["x"][..]; + let x = X(String::from("x")); + + require_c_str(&Cow::from(c_str)); + require_c_str(c_str); + + require_os_str(os_str); + require_os_str(&Cow::from(os_str)); + require_os_str(os_str); + + require_path(path); + require_path(&Cow::from(path)); + require_path(path); + + require_str(s); + require_str(&Cow::from(s)); + require_str(s); + require_str(x.as_ref()); + + require_slice(slice); + require_slice(&Cow::from(slice)); + require_slice(array.as_ref()); + require_slice(array_ref.as_ref()); + require_slice(slice); + require_slice(&x); + + require_x(&Cow::::Owned(x.clone())); + require_x(&x); + + require_deref_c_str(c_str); + require_deref_os_str(os_str); + require_deref_path(path); + require_deref_str(s); + require_deref_slice(slice); + + require_impl_deref_c_str(c_str); + require_impl_deref_os_str(os_str); + require_impl_deref_path(path); + require_impl_deref_str(s); + require_impl_deref_slice(slice); + + require_deref_str_slice(s, slice); + require_deref_slice_str(slice, s); + + require_as_ref_c_str(c_str); + require_as_ref_os_str(os_str); + require_as_ref_path(path); + require_as_ref_str(s); + require_as_ref_str(&x); + require_as_ref_slice(array); + require_as_ref_slice(array_ref); + require_as_ref_slice(slice); + + require_impl_as_ref_c_str(c_str); + require_impl_as_ref_os_str(os_str); + require_impl_as_ref_path(path); + require_impl_as_ref_str(s); + require_impl_as_ref_str(&x); + require_impl_as_ref_slice(array); + require_impl_as_ref_slice(array_ref); + require_impl_as_ref_slice(slice); + + require_as_ref_str_slice(s, array); + require_as_ref_str_slice(s, array_ref); + require_as_ref_str_slice(s, slice); + require_as_ref_slice_str(array, s); + require_as_ref_slice_str(array_ref, s); + require_as_ref_slice_str(slice, s); + + let _ = x.join(&x); + + // negative tests + require_string(&s.to_string()); + require_string(&Cow::from(s).into_owned()); + require_string(&s.to_owned()); + require_string(&x.to_string()); + + // `X` isn't copy. + require_deref_slice(x.to_owned()); +} + +fn require_c_str(_: &CStr) {} +fn require_os_str(_: &OsStr) {} +fn require_path(_: &std::path::Path) {} +fn require_str(_: &str) {} +fn require_slice(_: &[T]) {} +fn require_x(_: &X) {} + +fn require_deref_c_str>(_: T) {} +fn require_deref_os_str>(_: T) {} +fn require_deref_path>(_: T) {} +fn require_deref_str>(_: T) {} +fn require_deref_slice>(_: U) {} + +fn require_impl_deref_c_str(_: impl Deref) {} +fn require_impl_deref_os_str(_: impl Deref) {} +fn require_impl_deref_path(_: impl Deref) {} +fn require_impl_deref_str(_: impl Deref) {} +fn require_impl_deref_slice(_: impl Deref) {} + +fn require_deref_str_slice, U, V: Deref>(_: T, _: V) {} +fn require_deref_slice_str, V: Deref>(_: U, _: V) {} + +fn require_as_ref_c_str>(_: T) {} +fn require_as_ref_os_str>(_: T) {} +fn require_as_ref_path>(_: T) {} +fn require_as_ref_str>(_: T) {} +fn require_as_ref_slice>(_: U) {} + +fn require_impl_as_ref_c_str(_: impl AsRef) {} +fn require_impl_as_ref_os_str(_: impl AsRef) {} +fn require_impl_as_ref_path(_: impl AsRef) {} +fn require_impl_as_ref_str(_: impl AsRef) {} +fn require_impl_as_ref_slice(_: impl AsRef<[T]>) {} + +fn require_as_ref_str_slice, U, V: AsRef<[U]>>(_: T, _: V) {} +fn require_as_ref_slice_str, V: AsRef>(_: U, _: V) {} + +fn require_string(_: &String) {} diff --git a/tests/ui/unnecessary_to_owned.rs b/tests/ui/unnecessary_to_owned.rs new file mode 100644 index 00000000000..081ff635bbd --- /dev/null +++ b/tests/ui/unnecessary_to_owned.rs @@ -0,0 +1,170 @@ +// run-rustfix + +#![allow(clippy::ptr_arg)] +// Some of the expressions that `redundant_clone` flags overlap with ours. Enabling it interferes +// with `rustfix`. +#![allow(clippy::redundant_clone)] +// `needless_borrow` is for checking the fixed code. +#![warn(clippy::needless_borrow)] +#![warn(clippy::unnecessary_to_owned)] + +use std::borrow::Cow; +use std::ffi::{CStr, OsStr}; +use std::ops::Deref; + +#[derive(Clone)] +struct X(String); + +impl Deref for X { + type Target = [u8]; + fn deref(&self) -> &[u8] { + self.0.as_bytes() + } +} + +impl AsRef for X { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + +impl ToString for X { + fn to_string(&self) -> String { + self.0.to_string() + } +} + +impl X { + fn join(&self, other: impl AsRef) -> Self { + let mut s = self.0.clone(); + s.push_str(other.as_ref()); + Self(s) + } +} + +fn main() { + let c_str = CStr::from_bytes_with_nul(&[0]).unwrap(); + let os_str = OsStr::new("x"); + let path = std::path::Path::new("x"); + let s = "x"; + let array = ["x"]; + let array_ref = &["x"]; + let slice = &["x"][..]; + let x = X(String::from("x")); + + require_c_str(&Cow::from(c_str).into_owned()); + require_c_str(&c_str.to_owned()); + + require_os_str(&os_str.to_os_string()); + require_os_str(&Cow::from(os_str).into_owned()); + require_os_str(&os_str.to_owned()); + + require_path(&path.to_path_buf()); + require_path(&Cow::from(path).into_owned()); + require_path(&path.to_owned()); + + require_str(&s.to_string()); + require_str(&Cow::from(s).into_owned()); + require_str(&s.to_owned()); + require_str(&x.to_string()); + + require_slice(&slice.to_vec()); + require_slice(&Cow::from(slice).into_owned()); + require_slice(&array.to_owned()); + require_slice(&array_ref.to_owned()); + require_slice(&slice.to_owned()); + require_slice(&x.to_owned()); + + require_x(&Cow::::Owned(x.clone()).into_owned()); + require_x(&x.to_owned()); + + require_deref_c_str(c_str.to_owned()); + require_deref_os_str(os_str.to_owned()); + require_deref_path(path.to_owned()); + require_deref_str(s.to_owned()); + require_deref_slice(slice.to_owned()); + + require_impl_deref_c_str(c_str.to_owned()); + require_impl_deref_os_str(os_str.to_owned()); + require_impl_deref_path(path.to_owned()); + require_impl_deref_str(s.to_owned()); + require_impl_deref_slice(slice.to_owned()); + + require_deref_str_slice(s.to_owned(), slice.to_owned()); + require_deref_slice_str(slice.to_owned(), s.to_owned()); + + require_as_ref_c_str(c_str.to_owned()); + require_as_ref_os_str(os_str.to_owned()); + require_as_ref_path(path.to_owned()); + require_as_ref_str(s.to_owned()); + require_as_ref_str(x.to_owned()); + require_as_ref_slice(array.to_owned()); + require_as_ref_slice(array_ref.to_owned()); + require_as_ref_slice(slice.to_owned()); + + require_impl_as_ref_c_str(c_str.to_owned()); + require_impl_as_ref_os_str(os_str.to_owned()); + require_impl_as_ref_path(path.to_owned()); + require_impl_as_ref_str(s.to_owned()); + require_impl_as_ref_str(x.to_owned()); + require_impl_as_ref_slice(array.to_owned()); + require_impl_as_ref_slice(array_ref.to_owned()); + require_impl_as_ref_slice(slice.to_owned()); + + require_as_ref_str_slice(s.to_owned(), array.to_owned()); + require_as_ref_str_slice(s.to_owned(), array_ref.to_owned()); + require_as_ref_str_slice(s.to_owned(), slice.to_owned()); + require_as_ref_slice_str(array.to_owned(), s.to_owned()); + require_as_ref_slice_str(array_ref.to_owned(), s.to_owned()); + require_as_ref_slice_str(slice.to_owned(), s.to_owned()); + + let _ = x.join(&x.to_string()); + + // negative tests + require_string(&s.to_string()); + require_string(&Cow::from(s).into_owned()); + require_string(&s.to_owned()); + require_string(&x.to_string()); + + // `X` isn't copy. + require_deref_slice(x.to_owned()); +} + +fn require_c_str(_: &CStr) {} +fn require_os_str(_: &OsStr) {} +fn require_path(_: &std::path::Path) {} +fn require_str(_: &str) {} +fn require_slice(_: &[T]) {} +fn require_x(_: &X) {} + +fn require_deref_c_str>(_: T) {} +fn require_deref_os_str>(_: T) {} +fn require_deref_path>(_: T) {} +fn require_deref_str>(_: T) {} +fn require_deref_slice>(_: U) {} + +fn require_impl_deref_c_str(_: impl Deref) {} +fn require_impl_deref_os_str(_: impl Deref) {} +fn require_impl_deref_path(_: impl Deref) {} +fn require_impl_deref_str(_: impl Deref) {} +fn require_impl_deref_slice(_: impl Deref) {} + +fn require_deref_str_slice, U, V: Deref>(_: T, _: V) {} +fn require_deref_slice_str, V: Deref>(_: U, _: V) {} + +fn require_as_ref_c_str>(_: T) {} +fn require_as_ref_os_str>(_: T) {} +fn require_as_ref_path>(_: T) {} +fn require_as_ref_str>(_: T) {} +fn require_as_ref_slice>(_: U) {} + +fn require_impl_as_ref_c_str(_: impl AsRef) {} +fn require_impl_as_ref_os_str(_: impl AsRef) {} +fn require_impl_as_ref_path(_: impl AsRef) {} +fn require_impl_as_ref_str(_: impl AsRef) {} +fn require_impl_as_ref_slice(_: impl AsRef<[T]>) {} + +fn require_as_ref_str_slice, U, V: AsRef<[U]>>(_: T, _: V) {} +fn require_as_ref_slice_str, V: AsRef>(_: U, _: V) {} + +fn require_string(_: &String) {} diff --git a/tests/ui/unnecessary_to_owned.stderr b/tests/ui/unnecessary_to_owned.stderr new file mode 100644 index 00000000000..b15dd4c7ee7 --- /dev/null +++ b/tests/ui/unnecessary_to_owned.stderr @@ -0,0 +1,382 @@ +error: unnecessary use of `into_owned` + --> $DIR/unnecessary_to_owned.rs:55:36 + | +LL | require_c_str(&Cow::from(c_str).into_owned()); + | ^^^^^^^^^^^^^ help: remove this + | + = note: `-D clippy::unnecessary-to-owned` implied by `-D warnings` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:56:19 + | +LL | require_c_str(&c_str.to_owned()); + | ^^^^^^^^^^^^^^^^^ help: use: `c_str` + +error: unnecessary use of `to_os_string` + --> $DIR/unnecessary_to_owned.rs:58:20 + | +LL | require_os_str(&os_str.to_os_string()); + | ^^^^^^^^^^^^^^^^^^^^^^ help: use: `os_str` + +error: unnecessary use of `into_owned` + --> $DIR/unnecessary_to_owned.rs:59:38 + | +LL | require_os_str(&Cow::from(os_str).into_owned()); + | ^^^^^^^^^^^^^ help: remove this + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:60:20 + | +LL | require_os_str(&os_str.to_owned()); + | ^^^^^^^^^^^^^^^^^^ help: use: `os_str` + +error: unnecessary use of `to_path_buf` + --> $DIR/unnecessary_to_owned.rs:62:18 + | +LL | require_path(&path.to_path_buf()); + | ^^^^^^^^^^^^^^^^^^^ help: use: `path` + +error: unnecessary use of `into_owned` + --> $DIR/unnecessary_to_owned.rs:63:34 + | +LL | require_path(&Cow::from(path).into_owned()); + | ^^^^^^^^^^^^^ help: remove this + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:64:18 + | +LL | require_path(&path.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `path` + +error: unnecessary use of `to_string` + --> $DIR/unnecessary_to_owned.rs:66:17 + | +LL | require_str(&s.to_string()); + | ^^^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `into_owned` + --> $DIR/unnecessary_to_owned.rs:67:30 + | +LL | require_str(&Cow::from(s).into_owned()); + | ^^^^^^^^^^^^^ help: remove this + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:68:17 + | +LL | require_str(&s.to_owned()); + | ^^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_string` + --> $DIR/unnecessary_to_owned.rs:69:17 + | +LL | require_str(&x.to_string()); + | ^^^^^^^^^^^^^^ help: use: `x.as_ref()` + +error: unnecessary use of `to_vec` + --> $DIR/unnecessary_to_owned.rs:71:19 + | +LL | require_slice(&slice.to_vec()); + | ^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `into_owned` + --> $DIR/unnecessary_to_owned.rs:72:36 + | +LL | require_slice(&Cow::from(slice).into_owned()); + | ^^^^^^^^^^^^^ help: remove this + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:73:19 + | +LL | require_slice(&array.to_owned()); + | ^^^^^^^^^^^^^^^^^ help: use: `array.as_ref()` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:74:19 + | +LL | require_slice(&array_ref.to_owned()); + | ^^^^^^^^^^^^^^^^^^^^^ help: use: `array_ref.as_ref()` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:75:19 + | +LL | require_slice(&slice.to_owned()); + | ^^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:76:21 + | +LL | require_slice(&x.to_owned()); + | ^^^^^^^^^^^ help: remove this + +error: unnecessary use of `into_owned` + --> $DIR/unnecessary_to_owned.rs:78:42 + | +LL | require_x(&Cow::::Owned(x.clone()).into_owned()); + | ^^^^^^^^^^^^^ help: remove this + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:79:15 + | +LL | require_x(&x.to_owned()); + | ^^^^^^^^^^^^^ help: use: `&x` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:81:25 + | +LL | require_deref_c_str(c_str.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `c_str` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:82:26 + | +LL | require_deref_os_str(os_str.to_owned()); + | ^^^^^^^^^^^^^^^^^ help: use: `os_str` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:83:24 + | +LL | require_deref_path(path.to_owned()); + | ^^^^^^^^^^^^^^^ help: use: `path` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:84:23 + | +LL | require_deref_str(s.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:85:25 + | +LL | require_deref_slice(slice.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:87:30 + | +LL | require_impl_deref_c_str(c_str.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `c_str` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:88:31 + | +LL | require_impl_deref_os_str(os_str.to_owned()); + | ^^^^^^^^^^^^^^^^^ help: use: `os_str` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:89:29 + | +LL | require_impl_deref_path(path.to_owned()); + | ^^^^^^^^^^^^^^^ help: use: `path` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:90:28 + | +LL | require_impl_deref_str(s.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:91:30 + | +LL | require_impl_deref_slice(slice.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:93:29 + | +LL | require_deref_str_slice(s.to_owned(), slice.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:93:43 + | +LL | require_deref_str_slice(s.to_owned(), slice.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:94:29 + | +LL | require_deref_slice_str(slice.to_owned(), s.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:94:47 + | +LL | require_deref_slice_str(slice.to_owned(), s.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:96:26 + | +LL | require_as_ref_c_str(c_str.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `c_str` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:97:27 + | +LL | require_as_ref_os_str(os_str.to_owned()); + | ^^^^^^^^^^^^^^^^^ help: use: `os_str` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:98:25 + | +LL | require_as_ref_path(path.to_owned()); + | ^^^^^^^^^^^^^^^ help: use: `path` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:99:24 + | +LL | require_as_ref_str(s.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:100:24 + | +LL | require_as_ref_str(x.to_owned()); + | ^^^^^^^^^^^^ help: use: `&x` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:101:26 + | +LL | require_as_ref_slice(array.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `array` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:102:26 + | +LL | require_as_ref_slice(array_ref.to_owned()); + | ^^^^^^^^^^^^^^^^^^^^ help: use: `array_ref` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:103:26 + | +LL | require_as_ref_slice(slice.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:105:31 + | +LL | require_impl_as_ref_c_str(c_str.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `c_str` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:106:32 + | +LL | require_impl_as_ref_os_str(os_str.to_owned()); + | ^^^^^^^^^^^^^^^^^ help: use: `os_str` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:107:30 + | +LL | require_impl_as_ref_path(path.to_owned()); + | ^^^^^^^^^^^^^^^ help: use: `path` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:108:29 + | +LL | require_impl_as_ref_str(s.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:109:29 + | +LL | require_impl_as_ref_str(x.to_owned()); + | ^^^^^^^^^^^^ help: use: `&x` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:110:31 + | +LL | require_impl_as_ref_slice(array.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `array` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:111:31 + | +LL | require_impl_as_ref_slice(array_ref.to_owned()); + | ^^^^^^^^^^^^^^^^^^^^ help: use: `array_ref` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:112:31 + | +LL | require_impl_as_ref_slice(slice.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:114:30 + | +LL | require_as_ref_str_slice(s.to_owned(), array.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:114:44 + | +LL | require_as_ref_str_slice(s.to_owned(), array.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `array` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:115:30 + | +LL | require_as_ref_str_slice(s.to_owned(), array_ref.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:115:44 + | +LL | require_as_ref_str_slice(s.to_owned(), array_ref.to_owned()); + | ^^^^^^^^^^^^^^^^^^^^ help: use: `array_ref` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:116:30 + | +LL | require_as_ref_str_slice(s.to_owned(), slice.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:116:44 + | +LL | require_as_ref_str_slice(s.to_owned(), slice.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:117:30 + | +LL | require_as_ref_slice_str(array.to_owned(), s.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `array` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:117:48 + | +LL | require_as_ref_slice_str(array.to_owned(), s.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:118:30 + | +LL | require_as_ref_slice_str(array_ref.to_owned(), s.to_owned()); + | ^^^^^^^^^^^^^^^^^^^^ help: use: `array_ref` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:118:52 + | +LL | require_as_ref_slice_str(array_ref.to_owned(), s.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:119:30 + | +LL | require_as_ref_slice_str(slice.to_owned(), s.to_owned()); + | ^^^^^^^^^^^^^^^^ help: use: `slice` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned.rs:119:48 + | +LL | require_as_ref_slice_str(slice.to_owned(), s.to_owned()); + | ^^^^^^^^^^^^ help: use: `s` + +error: unnecessary use of `to_string` + --> $DIR/unnecessary_to_owned.rs:121:20 + | +LL | let _ = x.join(&x.to_string()); + | ^^^^^^^^^^^^^^ help: use: `&x` + +error: aborting due to 63 previous errors +