implement empty range lint as described in #330

README.md

@@ -6,7 +6,7 @@ A collection of lints that give helpful tips to newbies and catch oversights.
 [Jump to usage instructions](#usage)
 
 ##Lints
-There are 56 lints included in this crate:
+There are 57 lints included in this crate:
 
 name                                                                                                   | default | meaning
 -------------------------------------------------------------------------------------------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -48,6 +48,7 @@ name
 [redundant_closure](https://github.com/Manishearth/rust-clippy/wiki#redundant_closure)                 | warn    | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)
 [redundant_pattern](https://github.com/Manishearth/rust-clippy/wiki#redundant_pattern)                 | warn    | using `name @ _` in a pattern
 [result_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#result_unwrap_used)               | allow   | using `Result.unwrap()`, which might be better handled
+[reverse_range_loop](https://github.com/Manishearth/rust-clippy/wiki#reverse_range_loop)               | warn    | Iterating over an empty range, such as `10..0` or `5..5`
 [shadow_reuse](https://github.com/Manishearth/rust-clippy/wiki#shadow_reuse)                           | allow   | rebinding a name to an expression that re-uses the original value, e.g. `let x = x + 1`
 [shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same)                             | allow   | rebinding a name to itself, e.g. `let mut x = &mut x`
 [shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated)                   | allow   | The name is re-bound without even using the original value

src/lib.rs

@@ -122,6 +122,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         loops::EXPLICIT_ITER_LOOP,
         loops::ITER_NEXT_LOOP,
         loops::NEEDLESS_RANGE_LOOP,
+        loops::REVERSE_RANGE_LOOP,
         loops::UNUSED_COLLECT,
         loops::WHILE_LET_LOOP,
         matches::MATCH_REF_PATS,

src/loops.rs

@@ -25,13 +25,16 @@ declare_lint!{ pub UNUSED_COLLECT, Warn,
                "`collect()`ing an iterator without using the result; this is usually better \
                 written as a for loop" }
 
+declare_lint!{ pub REVERSE_RANGE_LOOP, Warn,
+               "Iterating over an empty range, such as `10..0` or `5..5`" }
+
 #[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)
+                    WHILE_LET_LOOP, UNUSED_COLLECT, REVERSE_RANGE_LOOP)
     }
 
     fn check_expr(&mut self, cx: &Context, expr: &Expr) {
@@ -69,6 +72,37 @@ impl LintPass for LoopsPass {
                 }
             }
 
+            // if this for-loop is iterating over a two-sided range...
+            if let ExprRange(Some(ref start_expr), Some(ref stop_expr)) = arg.node {
+                // and both sides are literals...
+                if let ExprLit(ref start_lit) = start_expr.node {
+                    if let ExprLit(ref stop_lit) = stop_expr.node {
+                        // and they are both integers...
+                        if let LitInt(start_idx, _) = start_lit.node {
+                            if let LitInt(stop_idx, _) = stop_lit.node {
+                                // and the start index is greater than the stop index,
+                                // this loop will never run. This is often confusing for developers
+                                // who think that this will iterate from the larger value to the
+                                // smaller value.
+                                if start_idx > stop_idx {
+                                    span_lint(cx, REVERSE_RANGE_LOOP, expr.span, &format!(
+                                        "this range is empty and this for loop will never run. \
+                                         Consider using `({}..{}).rev()` if you are attempting to \
+                                         iterate over this range in reverse", stop_idx, start_idx));
+                                }
+
+                                // if they are equal, it's also problematic - this loop
+                                // will never run.
+                                if start_idx == stop_idx {
+                                    span_lint(cx, REVERSE_RANGE_LOOP, expr.span,
+                                        "this range is empty and this for loop will never run");
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+
             if let ExprMethodCall(ref method, _, ref args) = arg.node {
                 // just the receiver, no arguments
                 if args.len() == 1 {
@@ -126,7 +160,7 @@ 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 args.len() == 1 && method.node.name == "collect" &&
                         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. \

tests/compile-fail/for_loop.rs

@@ -14,7 +14,7 @@ impl Unrelated {
     }
 }
 
-#[deny(needless_range_loop, explicit_iter_loop, iter_next_loop)]
+#[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop)]
 #[deny(unused_collect)]
 #[allow(linkedlist)]
 fn main() {
@@ -34,6 +34,18 @@ fn main() {
         println!("{}", vec[i]);
     }
 
+    for i in 10..0 { //~ERROR this range is empty and this for loop will never run. Consider using `(0..10).rev()`
+        println!("{}", i);
+    }
+
+    for i in 5..5 { //~ERROR this range is empty and this for loop will never run
+        println!("{}", i);
+    }
+
+    for i in 0..10 { // not an error, the start index is less than the end index
+        println!("{}", i);
+    }
+
     for _v in vec.iter() { } //~ERROR it is more idiomatic to loop over `&vec`
     for _v in vec.iter_mut() { } //~ERROR it is more idiomatic to loop over `&mut vec`