Merge pull request #2924 from niklasf/copy-iterator

Add copy_iterator lint (#1534)

clippy_lints/src/copy_iterator.rs

@@ -0,0 +1,57 @@
+use crate::utils::{is_copy, match_path, paths, span_note_and_lint};
+use rustc::hir::{Item, ItemKind};
+use rustc::lint::*;
+use rustc::{declare_lint, lint_array};
+
+/// **What it does:** Checks for types that implement `Copy` as well as
+/// `Iterator`.
+///
+/// **Why is this bad?** Implicit copies can be confusing when working with
+/// iterator combinators.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// #[derive(Copy, Clone)]
+/// struct Countdown(u8);
+///
+/// impl Iterator for Countdown {
+///     // ...
+/// }
+///
+/// let a: Vec<_> = my_iterator.take(1).collect();
+/// let b: Vec<_> = my_iterator.collect();
+/// ```
+declare_clippy_lint! {
+    pub COPY_ITERATOR,
+    pedantic,
+    "implementing `Iterator` on a `Copy` type"
+}
+
+pub struct CopyIterator;
+
+impl LintPass for CopyIterator {
+    fn get_lints(&self) -> LintArray {
+        lint_array![COPY_ITERATOR]
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyIterator {
+    fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
+        if let ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, _) = item.node {
+            let ty = cx.tcx.type_of(cx.tcx.hir.local_def_id(item.id));
+
+            if is_copy(cx, ty) && match_path(&trait_ref.path, &paths::ITERATOR) {
+                span_note_and_lint(
+                    cx,
+                    COPY_ITERATOR,
+                    item.span,
+                    "you are implementing `Iterator` on a `Copy` type",
+                    item.span,
+                    "consider implementing `IntoIterator` instead",
+                );
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -66,6 +66,7 @@ pub mod bytecount;
 pub mod collapsible_if;
 pub mod const_static_lifetime;
 pub mod copies;
+pub mod copy_iterator;
 pub mod cyclomatic_complexity;
 pub mod default_trait_access;
 pub mod derive;
@@ -338,6 +339,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box types::InvalidUpcastComparisons);
     reg.register_late_lint_pass(box regex::Pass::default());
     reg.register_late_lint_pass(box copies::CopyAndPaste);
+    reg.register_late_lint_pass(box copy_iterator::CopyIterator);
     reg.register_late_lint_pass(box format::Pass);
     reg.register_early_lint_pass(box formatting::Formatting);
     reg.register_late_lint_pass(box swap::Swap);
@@ -431,6 +433,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_lint_group("clippy_pedantic", vec![
         attrs::INLINE_ALWAYS,
         copies::MATCH_SAME_ARMS,
+        copy_iterator::COPY_ITERATOR,
         default_trait_access::DEFAULT_TRAIT_ACCESS,
         derive::EXPL_IMPL_CLONE_ON_COPY,
         doc::DOC_MARKDOWN,

tests/ui/copy_iterator.rs

@@ -0,0 +1,23 @@
+#![warn(copy_iterator)]
+
+#[derive(Copy, Clone)]
+struct Countdown(u8);
+
+impl Iterator for Countdown {
+    type Item = u8;
+
+    fn next(&mut self) -> Option<u8> {
+        self.0.checked_sub(1).map(|c| {
+            self.0 = c;
+            c
+        })
+    }
+}
+
+fn main() {
+    let my_iterator = Countdown(5);
+    let a: Vec<_> = my_iterator.take(1).collect();
+    assert_eq!(a.len(), 1);
+    let b: Vec<_> = my_iterator.collect();
+    assert_eq!(b.len(), 5);
+}

tests/ui/copy_iterator.stderr

@@ -0,0 +1,17 @@
+error: you are implementing `Iterator` on a `Copy` type
+  --> $DIR/copy_iterator.rs:6:1
+   |
+6  | / impl Iterator for Countdown {
+7  | |     type Item = u8;
+8  | |
+9  | |     fn next(&mut self) -> Option<u8> {
+...  |
+14 | |     }
+15 | | }
+   | |_^
+   |
+   = note: `-D copy-iterator` implied by `-D warnings`
+   = note: consider implementing `IntoIterator` instead
+
+error: aborting due to previous error
+