diff --git a/clippy_lints/src/copy_iterator.rs b/clippy_lints/src/copy_iterator.rs new file mode 100644 index 000000000000..ac0c2ed32c61 --- /dev/null +++ b/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", + ); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6064601fd80c..ee2f0ca5406b 100644 --- a/clippy_lints/src/lib.rs +++ b/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, diff --git a/tests/ui/copy_iterator.rs b/tests/ui/copy_iterator.rs new file mode 100644 index 000000000000..1b65cc4f8cc2 --- /dev/null +++ b/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 { + 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); +} diff --git a/tests/ui/copy_iterator.stderr b/tests/ui/copy_iterator.stderr new file mode 100644 index 000000000000..f520156b01e6 --- /dev/null +++ b/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 { +... | +14 | | } +15 | | } + | |_^ + | + = note: `-D copy-iterator` implied by `-D warnings` + = note: consider implementing `IntoIterator` instead + +error: aborting due to previous error +