Suggest for loop instead of while-let when looping over iterators

Author: fhartwig

README.md

@@ -6,7 +6,8 @@ A collection of lints to catch common mistakes and improve your Rust code.
 [Jump to usage instructions](#usage)
 
 ##Lints
-There are 68 lints included in this crate:
+There are 69 lints included in this crate:
+There are 65 lints included in this crate:
 
 name                                                                                                   | default | meaning
 -------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -74,6 +75,7 @@ name
 [unstable_as_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice)                 | warn    | as_slice is not stable and can be replaced by & v[..]see https://github.com/rust-lang/rust/issues/27729
 [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
+[while_let_on_iterator](https://github.com/Manishearth/rust-clippy/wiki#while_let_on_iterator)         | warn    | using a while-let loop instead of a for loop on an iterator
 [wrong_pub_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_pub_self_convention) | allow   | defining a public method named with an established prefix (like "into_") that takes `self` with the wrong convention
 [wrong_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_self_convention)         | warn    | defining a method named with an established prefix (like "into_") that takes `self` with the wrong convention
 [zero_divided_by_zero](https://github.com/Manishearth/rust-clippy/wiki#zero_divided_by_zero)           | warn    | usage of `0.0 / 0.0` to obtain NaN instead of std::f32::NaN or std::f64::NaN

src/lib.rs

@@ -138,6 +138,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         loops::REVERSE_RANGE_LOOP,
         loops::UNUSED_COLLECT,
         loops::WHILE_LET_LOOP,
+        loops::WHILE_LET_ON_ITERATOR,
         matches::MATCH_BOOL,
         matches::MATCH_REF_PATS,
         matches::SINGLE_MATCH,

src/loops.rs

@@ -38,14 +38,17 @@ declare_lint!{ pub EXPLICIT_COUNTER_LOOP, Warn,
 
 declare_lint!{ pub EMPTY_LOOP, Warn, "empty `loop {}` detected" }
 
+declare_lint!{ pub WHILE_LET_ON_ITERATOR, Warn, "using a while-let loop instead of a for loop on an iterator" }
+
 #[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, UNUSED_COLLECT, REVERSE_RANGE_LOOP,
-                    EXPLICIT_COUNTER_LOOP, EMPTY_LOOP)
+                    EXPLICIT_COUNTER_LOOP, EMPTY_LOOP,
+                    WHILE_LET_ON_ITERATOR)
     }
 }
 
@@ -228,6 +231,17 @@ impl LateLintPass for LoopsPass {
                 }
             }
         }
+        if let ExprMatch(ref expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
+            let pat = &arms[0].pats[0].node;
+            if let (&PatEnum(ref path, _), &ExprMethodCall(method_name, _, _)) = (pat, &expr.node) {
+                if method_name.node.as_str() == "next" &&
+                        match_trait_method(cx, expr, &["core", "iter", "Iterator"]) &&
+                        path.segments.last().unwrap().identifier.name.as_str() == "Some" {
+                    span_lint(cx, WHILE_LET_ON_ITERATOR, expr.span,
+                              "this loop could be written as a `for` loop");
+                }
+            }
+        }
     }
 
     fn check_stmt(&mut self, cx: &LateContext, stmt: &Stmt) {

tests/compile-fail/while_loop.rs

@@ -1,7 +1,7 @@
 #![feature(plugin)]
 #![plugin(clippy)]
 
-#![deny(while_let_loop, empty_loop)]
+#![deny(while_let_loop, empty_loop, while_let_on_iterator)]
 #![allow(dead_code, unused)]
 
 fn main() {
@@ -53,6 +53,23 @@ fn main() {
     while let Some(x) = y { // no error, obviously
         println!("{}", x);
     }
+
+
+    while let Option::Some(x) = (1..20).next() { //~ERROR this loop could be written as a `for` loop
+        println!("{}", x);
+    }
+
+    while let Some(x) = (1..20).next() { //~ERROR this loop could be written as a `for` loop
+        println!("{}", x);
+    }
+
+    while let Some(_) = (1..20).next() {} //~ERROR this loop could be written as a `for` loop
+
+    while let None = (1..20).next() {} // this is fine (if nonsensical)
+
+    if let Some(x) = (1..20).next() { // also fine
+        println!("{}", x)
+    }
 }
 
 // regression test (#360)