Add cloned_instead_of_copied lint

Author: camsteffen

CHANGELOG.md

@@ -2148,6 +2148,7 @@ Released 2018-09-13
 [`clone_double_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref
 [`clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
 [`clone_on_ref_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr
+[`cloned_instead_of_copied`]: https://rust-lang.github.io/rust-clippy/master/index.html#cloned_instead_of_copied
 [`cmp_nan`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_nan
 [`cmp_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_null
 [`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned

clippy_lints/src/lib.rs

@@ -759,6 +759,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         methods::BYTES_NTH,
         methods::CHARS_LAST_CMP,
         methods::CHARS_NEXT_CMP,
+        methods::CLONED_INSTEAD_OF_COPIED,
         methods::CLONE_DOUBLE_REF,
         methods::CLONE_ON_COPY,
         methods::CLONE_ON_REF_PTR,
@@ -1380,6 +1381,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
         LintId::of(matches::MATCH_WILD_ERR_ARM),
         LintId::of(matches::SINGLE_MATCH_ELSE),
+        LintId::of(methods::CLONED_INSTEAD_OF_COPIED),
         LintId::of(methods::FILTER_MAP_NEXT),
         LintId::of(methods::IMPLICIT_CLONE),
         LintId::of(methods::INEFFICIENT_TO_STRING),

clippy_lints/src/methods/cloned_instead_of_copied.rs

@@ -0,0 +1,38 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_trait_method;
+use clippy_utils::ty::{get_iterator_item_ty, is_copy};
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use rustc_span::{sym, Span};
+
+use super::CLONED_INSTEAD_OF_COPIED;
+
+pub fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span) {
+    let recv_ty = cx.typeck_results().expr_ty_adjusted(recv);
+    let inner_ty = match recv_ty.kind() {
+        // `Option<T>` -> `T`
+        ty::Adt(adt, subst) if cx.tcx.is_diagnostic_item(sym::option_type, adt.did) => subst.type_at(0),
+        _ if is_trait_method(cx, expr, sym::Iterator) => match get_iterator_item_ty(cx, recv_ty) {
+            // <T as Iterator>::Item
+            Some(ty) => ty,
+            _ => return,
+        },
+        _ => return,
+    };
+    match inner_ty.kind() {
+        // &T where T: Copy
+        ty::Ref(_, ty, _) if is_copy(cx, ty) => {},
+        _ => return,
+    };
+    span_lint_and_sugg(
+        cx,
+        CLONED_INSTEAD_OF_COPIED,
+        span,
+        "used `cloned` where `copied` could be used instead",
+        "try",
+        "copied".into(),
+        Applicability::MachineApplicable,
+    )
+}

clippy_lints/src/methods/mod.rs

@@ -8,6 +8,7 @@ mod chars_next_cmp;
 mod chars_next_cmp_with_unwrap;
 mod clone_on_copy;
 mod clone_on_ref_ptr;
+mod cloned_instead_of_copied;
 mod expect_fun_call;
 mod expect_used;
 mod filetype_is_file;
@@ -73,6 +74,29 @@ use rustc_span::symbol::SymbolStr;
 use rustc_span::{sym, Span};
 use rustc_typeck::hir_ty_to_ty;
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usages of `cloned()` on an `Iterator` or `Option` where
+    /// `copied()` could be used instead.
+    ///
+    /// **Why is this bad?** `copied()` is better because it guarantees that the type being cloned
+    /// implements `Copy`.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// [1, 2, 3].iter().cloned();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// [1, 2, 3].iter().copied();
+    /// ```
+    pub CLONED_INSTEAD_OF_COPIED,
+    pedantic,
+    "used `cloned` where `copied` could be used instead"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for `.unwrap()` calls on `Option`s and on `Result`s.
     ///
@@ -1638,6 +1662,7 @@ impl_lint_pass!(Methods => [
     CLONE_ON_COPY,
     CLONE_ON_REF_PTR,
     CLONE_DOUBLE_REF,
+    CLONED_INSTEAD_OF_COPIED,
     INEFFICIENT_TO_STRING,
     NEW_RET_NO_SELF,
     SINGLE_CHAR_PATTERN,
@@ -1909,6 +1934,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             ("as_mut", []) => useless_asref::check(cx, expr, "as_mut", recv),
             ("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv),
             ("assume_init", []) => uninit_assumed_init::check(cx, expr, recv),
+            ("cloned", []) => cloned_instead_of_copied::check(cx, expr, recv, span),
             ("collect", []) => match method_call!(recv) {
                 Some(("cloned", [recv2], _)) => iter_cloned_collect::check(cx, expr, recv2),
                 Some(("map", [m_recv, m_arg], _)) => {

clippy_utils/src/ty.rs

@@ -13,7 +13,7 @@ use rustc_lint::LateContext;
 use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
 use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TypeFoldable, UintTy};
 use rustc_span::sym;
-use rustc_span::symbol::Symbol;
+use rustc_span::symbol::{Ident, Symbol};
 use rustc_span::DUMMY_SP;
 use rustc_trait_selection::traits::query::normalize::AtExt;
 
@@ -52,6 +52,25 @@ pub fn contains_adt_constructor(ty: Ty<'_>, adt: &AdtDef) -> bool {
     })
 }
 
+/// Resolves `<T as Iterator>::Item` for `T`
+/// Do not invoke without first verifying that the type implements `Iterator`
+pub fn get_iterator_item_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
+    cx.tcx
+        .get_diagnostic_item(sym::Iterator)
+        .and_then(|iter_did| {
+            cx.tcx.associated_items(iter_did).find_by_name_and_kind(
+                cx.tcx,
+                Ident::from_str("Item"),
+                ty::AssocKind::Type,
+                iter_did,
+            )
+        })
+        .map(|assoc| {
+            let proj = cx.tcx.mk_projection(assoc.def_id, cx.tcx.mk_substs_trait(ty, &[]));
+            cx.tcx.normalize_erasing_regions(cx.param_env, proj)
+        })
+}
+
 /// Returns true if ty has `iter` or `iter_mut` methods
 pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option<Symbol> {
     // FIXME: instead of this hard-coded list, we should check if `<adt>::iter`

tests/ui/cloned_instead_of_copied.fixed

@@ -0,0 +1,15 @@
+// run-rustfix
+#![warn(clippy::cloned_instead_of_copied)]
+
+fn main() {
+    // yay
+    let _ = [1].iter().copied();
+    let _ = vec!["hi"].iter().copied();
+    let _ = Some(&1).copied();
+    let _ = Box::new([1].iter()).copied();
+    let _ = Box::new(Some(&1)).copied();
+
+    // nay
+    let _ = [String::new()].iter().cloned();
+    let _ = Some(&String::new()).cloned();
+}

tests/ui/cloned_instead_of_copied.rs

@@ -0,0 +1,15 @@
+// run-rustfix
+#![warn(clippy::cloned_instead_of_copied)]
+
+fn main() {
+    // yay
+    let _ = [1].iter().cloned();
+    let _ = vec!["hi"].iter().cloned();
+    let _ = Some(&1).cloned();
+    let _ = Box::new([1].iter()).cloned();
+    let _ = Box::new(Some(&1)).cloned();
+
+    // nay
+    let _ = [String::new()].iter().cloned();
+    let _ = Some(&String::new()).cloned();
+}

tests/ui/cloned_instead_of_copied.stderr

@@ -0,0 +1,34 @@
+error: used `cloned` where `copied` could be used instead
+  --> $DIR/cloned_instead_of_copied.rs:6:24
+   |
+LL |     let _ = [1].iter().cloned();
+   |                        ^^^^^^ help: try: `copied`
+   |
+   = note: `-D clippy::cloned-instead-of-copied` implied by `-D warnings`
+
+error: used `cloned` where `copied` could be used instead
+  --> $DIR/cloned_instead_of_copied.rs:7:31
+   |
+LL |     let _ = vec!["hi"].iter().cloned();
+   |                               ^^^^^^ help: try: `copied`
+
+error: used `cloned` where `copied` could be used instead
+  --> $DIR/cloned_instead_of_copied.rs:8:22
+   |
+LL |     let _ = Some(&1).cloned();
+   |                      ^^^^^^ help: try: `copied`
+
+error: used `cloned` where `copied` could be used instead
+  --> $DIR/cloned_instead_of_copied.rs:9:34
+   |
+LL |     let _ = Box::new([1].iter()).cloned();
+   |                                  ^^^^^^ help: try: `copied`
+
+error: used `cloned` where `copied` could be used instead
+  --> $DIR/cloned_instead_of_copied.rs:10:32
+   |
+LL |     let _ = Box::new(Some(&1)).cloned();
+   |                                ^^^^^^ help: try: `copied`
+
+error: aborting due to 5 previous errors
+