From 16df79a0549ba44afdfcecfbb3cbb210d2a7863f Mon Sep 17 00:00:00 2001
From: Georg Brandl <georg@python.org>
Date: Sun, 30 Aug 2015 13:10:59 +0200
Subject: [PATCH] new lint: using collect() to just exhaust an iterator

Should use a for loop instead.
---
 README.md                      |  3 ++-
 src/lib.rs                     |  1 +
 src/loops.rs                   | 20 +++++++++++++++++++-
 tests/compile-fail/for_loop.rs |  5 +++++
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 92a03b31530..4d5c59f2aef 100644
--- a/README.md
+++ b/README.md
@@ -4,7 +4,7 @@
 A collection of lints that give helpful tips to newbies and catch oversights.
 
 ##Lints
-There are 50 lints included in this crate:
+There are 51 lints included in this crate:
 
 name                                                                                                 | default | meaning
 -----------------------------------------------------------------------------------------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -56,6 +56,7 @@ name
 [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg)                 | warn    | a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`)
 [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity)                   | warn    | usage of very complex types; recommends factoring out parts into `type` definitions
 [unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp)                                 | warn    | comparing unit values (which is always `true` or `false`, respectively)
+[unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect)                     | warn    | `collect()`ing an iterator without using the result; this is usually better written as a for loop
 [while_let_loop](https://github.com/Manishearth/rust-clippy/wiki#while_let_loop)                     | warn    | `loop { if let { ... } else break }` can be written as a `while let` loop
 [zero_width_space](https://github.com/Manishearth/rust-clippy/wiki#zero_width_space)                 | deny    | using a zero-width space in a string literal, which is confusing
 
diff --git a/src/lib.rs b/src/lib.rs
index d69d352abc6..5e9205e32f9 100755
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -96,6 +96,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         loops::EXPLICIT_ITER_LOOP,
         loops::ITER_NEXT_LOOP,
         loops::NEEDLESS_RANGE_LOOP,
+        loops::UNUSED_COLLECT,
         loops::WHILE_LET_LOOP,
         matches::MATCH_REF_PATS,
         matches::SINGLE_MATCH,
diff --git a/src/loops.rs b/src/loops.rs
index eba706ed7e1..fe901794d6c 100644
--- a/src/loops.rs
+++ b/src/loops.rs
@@ -20,13 +20,17 @@ declare_lint!{ pub ITER_NEXT_LOOP, Warn,
 declare_lint!{ pub WHILE_LET_LOOP, Warn,
                "`loop { if let { ... } else break }` can be written as a `while let` loop" }
 
+declare_lint!{ pub UNUSED_COLLECT, Warn,
+               "`collect()`ing an iterator without using the result; this is usually better \
+                written as a for loop" }
+
 #[derive(Copy, Clone)]
 pub struct LoopsPass;
 
 impl LintPass for LoopsPass {
     fn get_lints(&self) -> LintArray {
         lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP,
-                    WHILE_LET_LOOP)
+                    WHILE_LET_LOOP, UNUSED_COLLECT)
     }
 
     fn check_expr(&mut self, cx: &Context, expr: &Expr) {
@@ -112,6 +116,20 @@ impl LintPass for LoopsPass {
             }
         }
     }
+
+    fn check_stmt(&mut self, cx: &Context, stmt: &Stmt) {
+        if let StmtSemi(ref expr, _) = stmt.node {
+            if let ExprMethodCall(ref method, _, ref args) = expr.node {
+                if args.len() == 1 && method.node.name == "collect" {
+                    if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) {
+                        span_lint(cx, UNUSED_COLLECT, expr.span, &format!(
+                            "you are collect()ing an iterator and throwing away the result. \
+                             Consider using an explicit for loop to exhaust the iterator"));
+                    }
+                }
+            }
+        }
+    }
 }
 
 /// Recover the essential nodes of a desugared for loop:
diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs
index eb7667b7fbd..66838651356 100755
--- a/tests/compile-fail/for_loop.rs
+++ b/tests/compile-fail/for_loop.rs
@@ -15,6 +15,7 @@ impl Unrelated {
 }
 
 #[deny(needless_range_loop, explicit_iter_loop, iter_next_loop)]
+#[deny(unused_collect)]
 #[allow(linkedlist)]
 fn main() {
     let mut vec = vec![1, 2, 3, 4];
@@ -56,4 +57,8 @@ fn main() {
     let u = Unrelated(vec![]);
     for _v in u.next() { } // no error
     for _v in u.iter() { } // no error
+
+    let mut out = vec![];
+    vec.iter().map(|x| out.push(x)).collect::<Vec<_>>(); //~ERROR you are collect()ing an iterator
+    let _y = vec.iter().map(|x| out.push(x)).collect::<Vec<_>>(); // this is fine
 }