[`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
+[`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
[`implicit_saturating_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_sub
&methods::FLAT_MAP_IDENTITY,
&methods::FROM_ITER_INSTEAD_OF_COLLECT,
&methods::GET_UNWRAP,
+ &methods::IMPLICIT_CLONE,
&methods::INEFFICIENT_TO_STRING,
&methods::INSPECT_FOR_EACH,
&methods::INTO_ITER_ON_REF,
LintId::of(&matches::SINGLE_MATCH_ELSE),
LintId::of(&methods::FILTER_MAP),
LintId::of(&methods::FILTER_MAP_NEXT),
+ LintId::of(&methods::IMPLICIT_CLONE),
LintId::of(&methods::INEFFICIENT_TO_STRING),
LintId::of(&methods::MAP_FLATTEN),
LintId::of(&methods::MAP_UNWRAP_OR),
--- /dev/null
+use crate::utils::span_lint_and_sugg;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_hir::ExprKind;
+use rustc_lint::LateContext;
+use rustc_middle::ty::TyS;
+use rustc_span::symbol::Symbol;
+
+use super::IMPLICIT_CLONE;
+use clippy_utils::is_diagnostic_assoc_item;
+
+pub fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, trait_diagnostic: Symbol) {
+ if_chain! {
+ if let ExprKind::MethodCall(method_path, _, [arg], _) = &expr.kind;
+ let return_type = cx.typeck_results().expr_ty(&expr);
+ let input_type = cx.typeck_results().expr_ty(arg).peel_refs();
+ if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
+ if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did));
+ if TyS::same_type(return_type, input_type);
+ if is_diagnostic_assoc_item(cx, expr_def_id, trait_diagnostic);
+ then {
+ span_lint_and_sugg(
+ cx,IMPLICIT_CLONE,method_path.ident.span,
+ &format!("implicitly cloning a `{}` by calling `{}` on its dereferenced type", ty_name, method_path.ident.name),
+ "consider using",
+ "clone".to_string(),
+ Applicability::MachineApplicable
+ );
+ }
+ }
+}
mod bind_instead_of_map;
mod bytes_nth;
mod filter_map_identity;
+mod implicit_clone;
mod inefficient_to_string;
mod inspect_for_each;
mod manual_saturating_arithmetic;
"replace `.bytes().nth()` with `.as_bytes().get()`"
}
+declare_clippy_lint! {
+ /// **What it does:** Checks for the usage of `_.to_owned()`, `vec.to_vec()`, or similar when calling `_.clone()` would be clearer.
+ ///
+ /// **Why is this bad?** These methods do the same thing as `_.clone()` but may be confusing as
+ /// to why we are calling `to_vec` on something that is already a `Vec` or calling `to_owned` on something that is already owned.
+ ///
+ /// **Known problems:** None.
+ ///
+ /// **Example:**
+ ///
+ /// ```rust
+ /// let a = vec![1, 2, 3];
+ /// let b = a.to_vec();
+ /// let c = a.to_owned();
+ /// ```
+ /// Use instead:
+ /// ```rust
+ /// let a = vec![1, 2, 3];
+ /// let b = a.clone();
+ /// let c = a.clone();
+ /// ```
+ pub IMPLICIT_CLONE,
+ pedantic,
+ "implicitly cloning a value by invoking a function on its dereferenced type"
+}
+
pub struct Methods {
msrv: Option<RustcVersion>,
}
MAP_COLLECT_RESULT_UNIT,
FROM_ITER_INSTEAD_OF_COLLECT,
INSPECT_FOR_EACH,
+ IMPLICIT_CLONE
]);
impl<'tcx> LateLintPass<'tcx> for Methods {
["ok_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "ok_or"),
["collect", "map"] => lint_map_collect(cx, expr, arg_lists[1], arg_lists[0]),
["for_each", "inspect"] => inspect_for_each::lint(cx, expr, method_spans[1]),
+ ["to_owned", ..] => implicit_clone::check(cx, expr, sym::ToOwned),
+ ["to_os_string", ..] => implicit_clone::check(cx, expr, sym::OsStr),
+ ["to_path_buf", ..] => implicit_clone::check(cx, expr, sym::Path),
+ ["to_vec", ..] => implicit_clone::check(cx, expr, sym::slice),
_ => {},
}
trt_id.map_or(false, |trt_id| match_def_path(cx, trt_id, path))
}
+/// Checks if the method call given in `expr` belongs to a trait or other container with a given
+/// diagnostic item
+pub fn is_diagnostic_assoc_item(cx: &LateContext<'_>, def_id: DefId, diag_item: Symbol) -> bool {
+ cx.tcx
+ .opt_associated_item(def_id)
+ .and_then(|associated_item| match associated_item.container {
+ ty::TraitContainer(assoc_def_id) => Some(assoc_def_id),
+ ty::ImplContainer(assoc_def_id) => match cx.tcx.type_of(assoc_def_id).kind() {
+ ty::Adt(adt, _) => Some(adt.did),
+ ty::Slice(_) => cx.tcx.get_diagnostic_item(sym::slice), // this isn't perfect but it works
+ _ => None,
+ },
+ })
+ .map_or(false, |assoc_def_id| cx.tcx.is_diagnostic_item(diag_item, assoc_def_id))
+}
+
/// Checks if an expression references a variable of the given name.
pub fn match_var(expr: &Expr<'_>, var: Symbol) -> bool {
if let ExprKind::Path(QPath::Resolved(None, ref path)) = expr.kind {
#[allow(clippy::unnecessary_operation)]
+#[allow(clippy::implicit_clone)]
fn main() {
let x = &Baz;
error: this creates an owned instance just for comparison
- --> $DIR/without_suggestion.rs:6:5
+ --> $DIR/without_suggestion.rs:7:5
|
LL | y.to_owned() == *x;
| ^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating
= note: `-D clippy::cmp-owned` implied by `-D warnings`
error: this creates an owned instance just for comparison
- --> $DIR/without_suggestion.rs:10:5
+ --> $DIR/without_suggestion.rs:11:5
|
LL | y.to_owned() == **x;
| ^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating
error: this creates an owned instance just for comparison
- --> $DIR/without_suggestion.rs:17:9
+ --> $DIR/without_suggestion.rs:18:9
|
LL | self.to_owned() == *other
| ^^^^^^^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating
--- /dev/null
+#![warn(clippy::implicit_clone)]
+#![allow(clippy::redundant_clone)]
+use std::borrow::Borrow;
+use std::ffi::{OsStr, OsString};
+use std::path::PathBuf;
+
+fn return_owned_from_slice(slice: &[u32]) -> Vec<u32> {
+ slice.to_owned()
+}
+
+pub fn own_same<T>(v: T) -> T
+where
+ T: ToOwned<Owned = T>,
+{
+ v.to_owned()
+}
+
+pub fn own_same_from_ref<T>(v: &T) -> T
+where
+ T: ToOwned<Owned = T>,
+{
+ v.to_owned()
+}
+
+pub fn own_different<T, U>(v: T) -> U
+where
+ T: ToOwned<Owned = U>,
+{
+ v.to_owned()
+}
+
+#[derive(Copy, Clone)]
+struct Kitten {}
+impl Kitten {
+ // badly named method
+ fn to_vec(self) -> Kitten {
+ Kitten {}
+ }
+}
+impl Borrow<BorrowedKitten> for Kitten {
+ fn borrow(&self) -> &BorrowedKitten {
+ static VALUE: BorrowedKitten = BorrowedKitten {};
+ &VALUE
+ }
+}
+
+struct BorrowedKitten {}
+impl ToOwned for BorrowedKitten {
+ type Owned = Kitten;
+ fn to_owned(&self) -> Kitten {
+ Kitten {}
+ }
+}
+
+mod weird {
+ #[allow(clippy::ptr_arg)]
+ pub fn to_vec(v: &Vec<u32>) -> Vec<u32> {
+ v.clone()
+ }
+}
+
+fn main() {
+ let vec = vec![5];
+ let _ = return_owned_from_slice(&vec);
+ let _ = vec.to_owned();
+ let _ = vec.to_vec();
+
+ let vec_ref = &vec;
+ let _ = return_owned_from_slice(&vec_ref);
+ let _ = vec_ref.to_owned();
+ let _ = vec_ref.to_vec();
+
+ // we expect no lint for this
+ let _ = weird::to_vec(&vec);
+
+ // we expect no lints for this
+ let slice: &[u32] = &[1, 2, 3, 4, 5];
+ let _ = return_owned_from_slice(slice);
+ let _ = slice.to_owned();
+ let _ = slice.to_vec();
+
+ let str = "hello world".to_string();
+ let _ = str.to_owned();
+
+ // testing w/ an arbitrary type
+ let kitten = Kitten {};
+ let _ = kitten.to_owned();
+ let _ = own_same_from_ref(&kitten);
+ // this shouln't lint
+ let _ = kitten.to_vec();
+
+ // we expect no lints for this
+ let borrowed = BorrowedKitten {};
+ let _ = borrowed.to_owned();
+
+ let pathbuf = PathBuf::new();
+ let _ = pathbuf.to_owned();
+ let _ = pathbuf.to_path_buf();
+
+ let os_string = OsString::from("foo");
+ let _ = os_string.to_owned();
+ let _ = os_string.to_os_string();
+
+ // we expect no lints for this
+ let os_str = OsStr::new("foo");
+ let _ = os_str.to_owned();
+ let _ = os_str.to_os_string();
+}
--- /dev/null
+error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type
+ --> $DIR/implicit_clone.rs:65:17
+ |
+LL | let _ = vec.to_owned();
+ | ^^^^^^^^ help: consider using: `clone`
+ |
+ = note: `-D clippy::implicit-clone` implied by `-D warnings`
+
+error: implicitly cloning a `Vec` by calling `to_vec` on its dereferenced type
+ --> $DIR/implicit_clone.rs:66:17
+ |
+LL | let _ = vec.to_vec();
+ | ^^^^^^ help: consider using: `clone`
+
+error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type
+ --> $DIR/implicit_clone.rs:70:21
+ |
+LL | let _ = vec_ref.to_owned();
+ | ^^^^^^^^ help: consider using: `clone`
+
+error: implicitly cloning a `Vec` by calling `to_vec` on its dereferenced type
+ --> $DIR/implicit_clone.rs:71:21
+ |
+LL | let _ = vec_ref.to_vec();
+ | ^^^^^^ help: consider using: `clone`
+
+error: implicitly cloning a `String` by calling `to_owned` on its dereferenced type
+ --> $DIR/implicit_clone.rs:83:17
+ |
+LL | let _ = str.to_owned();
+ | ^^^^^^^^ help: consider using: `clone`
+
+error: implicitly cloning a `Kitten` by calling `to_owned` on its dereferenced type
+ --> $DIR/implicit_clone.rs:87:20
+ |
+LL | let _ = kitten.to_owned();
+ | ^^^^^^^^ help: consider using: `clone`
+
+error: implicitly cloning a `PathBuf` by calling `to_owned` on its dereferenced type
+ --> $DIR/implicit_clone.rs:97:21
+ |
+LL | let _ = pathbuf.to_owned();
+ | ^^^^^^^^ help: consider using: `clone`
+
+error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type
+ --> $DIR/implicit_clone.rs:98:21
+ |
+LL | let _ = pathbuf.to_path_buf();
+ | ^^^^^^^^^^^ help: consider using: `clone`
+
+error: implicitly cloning a `OsString` by calling `to_owned` on its dereferenced type
+ --> $DIR/implicit_clone.rs:101:23
+ |
+LL | let _ = os_string.to_owned();
+ | ^^^^^^^^ help: consider using: `clone`
+
+error: implicitly cloning a `OsString` by calling `to_os_string` on its dereferenced type
+ --> $DIR/implicit_clone.rs:102:23
+ |
+LL | let _ = os_string.to_os_string();
+ | ^^^^^^^^^^^^ help: consider using: `clone`
+
+error: aborting due to 10 previous errors
+
// run-rustfix
// rustfix-only-machine-applicable
+#![allow(clippy::implicit_clone)]
use std::ffi::OsString;
use std::path::Path;
// run-rustfix
// rustfix-only-machine-applicable
+#![allow(clippy::implicit_clone)]
use std::ffi::OsString;
use std::path::Path;
error: redundant clone
- --> $DIR/redundant_clone.rs:8:42
+ --> $DIR/redundant_clone.rs:9:42
|
LL | let _s = ["lorem", "ipsum"].join(" ").to_string();
| ^^^^^^^^^^^^ help: remove this
|
= note: `-D clippy::redundant-clone` implied by `-D warnings`
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:8:14
+ --> $DIR/redundant_clone.rs:9:14
|
LL | let _s = ["lorem", "ipsum"].join(" ").to_string();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: redundant clone
- --> $DIR/redundant_clone.rs:11:15
+ --> $DIR/redundant_clone.rs:12:15
|
LL | let _s = s.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:11:14
+ --> $DIR/redundant_clone.rs:12:14
|
LL | let _s = s.clone();
| ^
error: redundant clone
- --> $DIR/redundant_clone.rs:14:15
+ --> $DIR/redundant_clone.rs:15:15
|
LL | let _s = s.to_string();
| ^^^^^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:14:14
+ --> $DIR/redundant_clone.rs:15:14
|
LL | let _s = s.to_string();
| ^
error: redundant clone
- --> $DIR/redundant_clone.rs:17:15
+ --> $DIR/redundant_clone.rs:18:15
|
LL | let _s = s.to_owned();
| ^^^^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:17:14
+ --> $DIR/redundant_clone.rs:18:14
|
LL | let _s = s.to_owned();
| ^
error: redundant clone
- --> $DIR/redundant_clone.rs:19:42
+ --> $DIR/redundant_clone.rs:20:42
|
LL | let _s = Path::new("/a/b/").join("c").to_owned();
| ^^^^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:19:14
+ --> $DIR/redundant_clone.rs:20:14
|
LL | let _s = Path::new("/a/b/").join("c").to_owned();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: redundant clone
- --> $DIR/redundant_clone.rs:21:42
+ --> $DIR/redundant_clone.rs:22:42
|
LL | let _s = Path::new("/a/b/").join("c").to_path_buf();
| ^^^^^^^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:21:14
+ --> $DIR/redundant_clone.rs:22:14
|
LL | let _s = Path::new("/a/b/").join("c").to_path_buf();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: redundant clone
- --> $DIR/redundant_clone.rs:23:29
+ --> $DIR/redundant_clone.rs:24:29
|
LL | let _s = OsString::new().to_owned();
| ^^^^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:23:14
+ --> $DIR/redundant_clone.rs:24:14
|
LL | let _s = OsString::new().to_owned();
| ^^^^^^^^^^^^^^^
error: redundant clone
- --> $DIR/redundant_clone.rs:25:29
+ --> $DIR/redundant_clone.rs:26:29
|
LL | let _s = OsString::new().to_os_string();
| ^^^^^^^^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:25:14
+ --> $DIR/redundant_clone.rs:26:14
|
LL | let _s = OsString::new().to_os_string();
| ^^^^^^^^^^^^^^^
error: redundant clone
- --> $DIR/redundant_clone.rs:32:19
+ --> $DIR/redundant_clone.rs:33:19
|
LL | let _t = tup.0.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:32:14
+ --> $DIR/redundant_clone.rs:33:14
|
LL | let _t = tup.0.clone();
| ^^^^^
error: redundant clone
- --> $DIR/redundant_clone.rs:62:22
+ --> $DIR/redundant_clone.rs:63:22
|
LL | (a.clone(), a.clone())
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:62:21
+ --> $DIR/redundant_clone.rs:63:21
|
LL | (a.clone(), a.clone())
| ^
error: redundant clone
- --> $DIR/redundant_clone.rs:122:15
+ --> $DIR/redundant_clone.rs:123:15
|
LL | let _s = s.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:122:14
+ --> $DIR/redundant_clone.rs:123:14
|
LL | let _s = s.clone();
| ^
error: redundant clone
- --> $DIR/redundant_clone.rs:123:15
+ --> $DIR/redundant_clone.rs:124:15
|
LL | let _t = t.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:123:14
+ --> $DIR/redundant_clone.rs:124:14
|
LL | let _t = t.clone();
| ^
error: redundant clone
- --> $DIR/redundant_clone.rs:133:19
+ --> $DIR/redundant_clone.rs:134:19
|
LL | let _f = f.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
- --> $DIR/redundant_clone.rs:133:18
+ --> $DIR/redundant_clone.rs:134:18
|
LL | let _f = f.clone();
| ^
error: redundant clone
- --> $DIR/redundant_clone.rs:145:14
+ --> $DIR/redundant_clone.rs:146:14
|
LL | let y = x.clone().join("matthias");
| ^^^^^^^^ help: remove this
|
note: cloned value is neither consumed nor mutated
- --> $DIR/redundant_clone.rs:145:13
+ --> $DIR/redundant_clone.rs:146:13
|
LL | let y = x.clone().join("matthias");
| ^^^^^^^^^
projects / rust.git / commitdiff