Merge pull request #1992 from rust-lang-nursery/infinite_iter

New lint: (maybe_)infinite_iter

CHANGELOG.md

@@ -481,6 +481,7 @@ All notable changes to this project will be documented in this file.
 [`inconsistent_digit_grouping`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#inconsistent_digit_grouping
 [`indexing_slicing`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#indexing_slicing
 [`ineffective_bit_mask`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#ineffective_bit_mask
+[`infinite_iter`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#infinite_iter
 [`inline_always`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#inline_always
 [`integer_arithmetic`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#integer_arithmetic
 [`invalid_regex`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#invalid_regex
@@ -508,6 +509,7 @@ All notable changes to this project will be documented in this file.
 [`match_ref_pats`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#match_ref_pats
 [`match_same_arms`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#match_same_arms
 [`match_wild_err_arm`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#match_wild_err_arm
+[`maybe_infinite_iter`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#maybe_infinite_iter
 [`mem_forget`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#mem_forget
 [`min_max`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#min_max
 [`misrefactored_assign_op`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#misrefactored_assign_op

README.md

@@ -180,7 +180,7 @@ transparently:
 
 ## Lints
 
-There are 206 lints included in this crate:
+There are 208 lints included in this crate:
 
 name                                                                                                                         | default | triggers on
 -----------------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -252,6 +252,7 @@ name
 [inconsistent_digit_grouping](https://github.com/rust-lang-nursery/rust-clippy/wiki#inconsistent_digit_grouping)             | warn    | integer literals with digits grouped inconsistently
 [indexing_slicing](https://github.com/rust-lang-nursery/rust-clippy/wiki#indexing_slicing)                                   | allow   | indexing/slicing usage
 [ineffective_bit_mask](https://github.com/rust-lang-nursery/rust-clippy/wiki#ineffective_bit_mask)                           | warn    | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`
+[infinite_iter](https://github.com/rust-lang-nursery/rust-clippy/wiki#infinite_iter)                                         | warn    | infinite iteration
 [inline_always](https://github.com/rust-lang-nursery/rust-clippy/wiki#inline_always)                                         | warn    | use of `#[inline(always)]`
 [integer_arithmetic](https://github.com/rust-lang-nursery/rust-clippy/wiki#integer_arithmetic)                               | allow   | any integer arithmetic statement
 [invalid_regex](https://github.com/rust-lang-nursery/rust-clippy/wiki#invalid_regex)                                         | deny    | invalid regular expressions
@@ -279,6 +280,7 @@ name
 [match_ref_pats](https://github.com/rust-lang-nursery/rust-clippy/wiki#match_ref_pats)                                       | warn    | a match or `if let` with all arms prefixed with `&` instead of deref-ing the match expression
 [match_same_arms](https://github.com/rust-lang-nursery/rust-clippy/wiki#match_same_arms)                                     | warn    | `match` with identical arm bodies
 [match_wild_err_arm](https://github.com/rust-lang-nursery/rust-clippy/wiki#match_wild_err_arm)                               | warn    | a match with `Err(_)` arm and take drastic actions
+[maybe_infinite_iter](https://github.com/rust-lang-nursery/rust-clippy/wiki#maybe_infinite_iter)                             | allow   | possible infinite iteration
 [mem_forget](https://github.com/rust-lang-nursery/rust-clippy/wiki#mem_forget)                                               | allow   | `mem::forget` usage on `Drop` types, likely to cause memory leaks
 [min_max](https://github.com/rust-lang-nursery/rust-clippy/wiki#min_max)                                                     | warn    | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant
 [misrefactored_assign_op](https://github.com/rust-lang-nursery/rust-clippy/wiki#misrefactored_assign_op)                     | warn    | having a variable on both sides of an assign op

clippy_lints/src/infinite_iter.rs

@@ -0,0 +1,232 @@
+use rustc::hir::*;
+use rustc::lint::*;
+use utils::{get_trait_def_id, implements_trait, higher, match_qpath, paths, span_lint};
+
+/// **What it does:** Checks for iteration that is guaranteed to be infinite.
+///
+/// **Why is this bad?** While there may be places where this is acceptable
+/// (e.g. in event streams), in most cases this is simply an error.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// repeat(1_u8).iter().collect::<Vec<_>>()
+/// ```
+declare_lint! {
+    pub INFINITE_ITER,
+    Warn,
+    "infinite iteration"
+}
+
+/// **What it does:** Checks for iteration that may be infinite.
+///
+/// **Why is this bad?** While there may be places where this is acceptable
+/// (e.g. in event streams), in most cases this is simply an error.
+///
+/// **Known problems:** The code may have a condition to stop iteration, but
+/// this lint is not clever enough to analyze it.
+///
+/// **Example:**
+/// ```rust
+/// [0..].iter().zip(infinite_iter.take_while(|x| x > 5))
+/// ```
+declare_lint! {
+    pub MAYBE_INFINITE_ITER,
+    Allow,
+    "possible infinite iteration"
+}
+
+#[derive(Copy, Clone)]
+pub struct Pass;
+
+impl LintPass for Pass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(INFINITE_ITER, MAYBE_INFINITE_ITER)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        let (lint, msg) = match complete_infinite_iter(cx, expr) {
+            Infinite => (INFINITE_ITER, "infinite iteration detected"),
+            MaybeInfinite => (MAYBE_INFINITE_ITER,
+                        "possible infinite iteration detected"),
+            Finite => { return; }
+        };
+        span_lint(cx, lint, expr.span, msg)
+    }
+}
+
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+enum Finiteness {
+    Infinite,
+    MaybeInfinite,
+    Finite
+}
+
+use self::Finiteness::{Infinite, MaybeInfinite, Finite};
+
+impl Finiteness {
+    fn and(self, b: Self) -> Self {
+        match (self, b) {
+            (Finite, _) | (_, Finite) => Finite,
+            (MaybeInfinite, _) | (_, MaybeInfinite) => MaybeInfinite,
+            _ => Infinite
+        }
+    }
+
+    fn or(self, b: Self) -> Self {
+        match (self, b) {
+            (Infinite, _) | (_, Infinite) => Infinite,
+            (MaybeInfinite, _) | (_, MaybeInfinite) => MaybeInfinite,
+            _ => Finite
+        }
+    }
+}
+
+impl From<bool> for Finiteness {
+    fn from(b: bool) -> Self {
+        if b { Infinite } else { Finite }
+    }
+}
+
+/// This tells us what to look for to know if the iterator returned by
+/// this method is infinite
+#[derive(Copy, Clone)]
+enum Heuristic {
+    /// infinite no matter what
+    Always,
+    /// infinite if the first argument is
+    First,
+    /// infinite if any of the supplied arguments is
+    Any,
+    /// infinite if all of the supplied arguments are
+    All
+}
+
+use self::Heuristic::{Always, First, Any, All};
+
+/// a slice of (method name, number of args, heuristic, bounds) tuples
+/// that will be used to determine whether the method in question
+/// returns an infinite or possibly infinite iterator. The finiteness
+/// is an upper bound, e.g. some methods can return a possibly
+/// infinite iterator at worst, e.g. `take_while`.
+static HEURISTICS : &[(&str, usize, Heuristic, Finiteness)] = &[
+    ("zip", 2, All, Infinite),
+    ("chain", 2, Any, Infinite),
+    ("cycle", 1, Always, Infinite),
+    ("map", 2, First, Infinite),
+    ("by_ref", 1, First, Infinite),
+    ("cloned", 1, First, Infinite),
+    ("rev", 1, First, Infinite),
+    ("inspect", 1, First, Infinite),
+    ("enumerate", 1, First, Infinite),
+    ("peekable", 2, First, Infinite),
+    ("fuse", 1, First, Infinite),
+    ("skip", 2, First, Infinite),
+    ("skip_while", 1, First, Infinite),
+    ("filter", 2, First, Infinite),
+    ("filter_map", 2, First, Infinite),
+    ("flat_map", 2, First, Infinite),
+    ("unzip", 1, First, Infinite),
+    ("take_while", 2, First, MaybeInfinite),
+    ("scan", 3, First, MaybeInfinite)
+];
+
+fn is_infinite(cx: &LateContext, expr: &Expr) -> Finiteness {
+    match expr.node {
+        ExprMethodCall(ref method, _, ref args) => {
+            for &(name, len, heuristic, cap) in HEURISTICS.iter() {
+                if method.name == name && args.len() == len {
+                    return (match heuristic {
+                        Always => Infinite,
+                        First => is_infinite(cx, &args[0]),
+                        Any => is_infinite(cx, &args[0]).or(is_infinite(cx, &args[1])),
+                        All => is_infinite(cx, &args[0]).and(is_infinite(cx, &args[1])),
+                    }).and(cap);
+                }
+            }
+            if method.name == "flat_map" && args.len() == 2 {
+                if let ExprClosure(_, _, body_id, _) = args[1].node {
+                    let body = cx.tcx.hir.body(body_id);
+                    return is_infinite(cx, &body.value);
+                }
+            }
+            Finite
+        },
+        ExprBlock(ref block) =>
+            block.expr.as_ref().map_or(Finite, |e| is_infinite(cx, e)),
+        ExprBox(ref e) | ExprAddrOf(_, ref e) => is_infinite(cx, e),
+        ExprCall(ref path, _) => {
+            if let ExprPath(ref qpath) = path.node {
+                match_qpath(qpath, &paths::REPEAT).into()
+            } else { Finite }
+        },
+        ExprStruct(..) => {
+            higher::range(expr).map_or(false, |r| r.end.is_none()).into()
+        },
+        _ => Finite
+    }
+}
+
+/// the names and argument lengths of methods that *may* exhaust their
+/// iterators
+static POSSIBLY_COMPLETING_METHODS : &[(&str, usize)] = &[
+    ("find", 2),
+    ("rfind", 2),
+    ("position", 2),
+    ("rposition", 2),
+    ("any", 2),
+    ("all", 2)
+];
+
+/// the names and argument lengths of methods that *always* exhaust
+/// their iterators
+static COMPLETING_METHODS : &[(&str, usize)] = &[
+    ("count", 1),
+    ("collect", 1),
+    ("fold", 3),
+    ("for_each", 2),
+    ("partition", 2),
+    ("max", 1),
+    ("max_by", 2),
+    ("max_by_key", 2),
+    ("min", 1),
+    ("min_by", 2),
+    ("min_by_key", 2),
+    ("sum", 1),
+    ("product", 1)
+];
+
+fn complete_infinite_iter(cx: &LateContext, expr: &Expr) -> Finiteness {
+    match expr.node {
+        ExprMethodCall(ref method, _, ref args) => {
+            for &(name, len) in COMPLETING_METHODS.iter() {
+                if method.name == name && args.len() == len {
+                    return is_infinite(cx, &args[0]);
+                }
+            }
+            for &(name, len) in POSSIBLY_COMPLETING_METHODS.iter() {
+                if method.name == name && args.len() == len {
+                    return MaybeInfinite.and(is_infinite(cx, &args[0]));
+                }
+            }
+            if method.name == "last" && args.len() == 1 &&
+                    get_trait_def_id(cx, &paths::DOUBLE_ENDED_ITERATOR).map_or(false,
+                        |id| !implements_trait(cx,
+                                               cx.tables.expr_ty(&args[0]),
+                                               id,
+                                               &[])) {
+                return is_infinite(cx, &args[0]);
+            }
+        },
+        ExprBinary(op, ref l, ref r) => {
+            if op.node.is_comparison() {
+                return is_infinite(cx, l).and(is_infinite(cx, r)).and(MaybeInfinite)
+            }
+        }, //TODO: ExprLoop + Match
+        _ => ()
+    }
+    Finite
+}

clippy_lints/src/lib.rs

@@ -97,6 +97,7 @@ pub mod functions;
 pub mod identity_op;
 pub mod if_let_redundant_pattern_matching;
 pub mod if_not_else;
+pub mod infinite_iter;
 pub mod items_after_statements;
 pub mod large_enum_variant;
 pub mod len_zero;
@@ -323,6 +324,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_early_lint_pass(box literal_digit_grouping::LiteralDigitGrouping);
     reg.register_late_lint_pass(box use_self::UseSelf);
     reg.register_late_lint_pass(box bytecount::ByteCount);
+    reg.register_late_lint_pass(box infinite_iter::Pass);
 
     reg.register_lint_group("clippy_restrictions", vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -338,6 +340,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         enum_variants::PUB_ENUM_VARIANT_NAMES,
         enum_variants::STUTTER,
         if_not_else::IF_NOT_ELSE,
+        infinite_iter::MAYBE_INFINITE_ITER,
         items_after_statements::ITEMS_AFTER_STATEMENTS,
         matches::SINGLE_MATCH_ELSE,
         mem_forget::MEM_FORGET,
@@ -422,6 +425,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         functions::TOO_MANY_ARGUMENTS,
         identity_op::IDENTITY_OP,
         if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING,
+        infinite_iter::INFINITE_ITER,
         large_enum_variant::LARGE_ENUM_VARIANT,
         len_zero::LEN_WITHOUT_IS_EMPTY,
         len_zero::LEN_ZERO,

clippy_lints/src/utils/paths.rs

@@ -21,6 +21,7 @@ pub const CSTRING_NEW: [&'static str; 5] = ["std", "ffi", "c_str", "CString", "n
 pub const DEBUG_FMT_METHOD: [&'static str; 4] = ["core", "fmt", "Debug", "fmt"];
 pub const DEFAULT_TRAIT: [&'static str; 3] = ["core", "default", "Default"];
 pub const DISPLAY_FMT_METHOD: [&'static str; 4] = ["core", "fmt", "Display", "fmt"];
+pub const DOUBLE_ENDED_ITERATOR: [&'static str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];
 pub const DROP: [&'static str; 3] = ["core", "mem", "drop"];
 pub const FMT_ARGUMENTS_NEWV1: [&'static str; 4] = ["core", "fmt", "Arguments", "new_v1"];
 pub const FMT_ARGUMENTV1_NEW: [&'static str; 4] = ["core", "fmt", "ArgumentV1", "new"];
@@ -65,6 +66,7 @@ pub const REGEX_BYTES_NEW: [&'static str; 4] = ["regex", "re_bytes", "Regex", "n
 pub const REGEX_BYTES_SET_NEW: [&'static str; 5] = ["regex", "re_set", "bytes", "RegexSet", "new"];
 pub const REGEX_NEW: [&'static str; 4] = ["regex", "re_unicode", "Regex", "new"];
 pub const REGEX_SET_NEW: [&'static str; 5] = ["regex", "re_set", "unicode", "RegexSet", "new"];
+pub const REPEAT: [&'static str; 3] = ["core", "iter", "repeat"];
 pub const RESULT: [&'static str; 3] = ["core", "result", "Result"];
 pub const RESULT_ERR: [&'static str; 4] = ["core", "result", "Result", "Err"];
 pub const RESULT_OK: [&'static str; 4] = ["core", "result", "Result", "Ok"];

tests/ui/infinite_iter.rs

@@ -0,0 +1,40 @@
+#![feature(plugin)]
+#![feature(iterator_for_each)]
+#![plugin(clippy)]
+use std::iter::repeat;
+
+fn square_is_lower_64(x: &u32) -> bool { x * x < 64 }
+
+#[allow(maybe_infinite_iter)]
+#[deny(infinite_iter)]
+fn infinite_iters() {
+    repeat(0_u8).collect::<Vec<_>>(); // infinite iter
+    (0..8_u32).take_while(square_is_lower_64).cycle().count(); // infinite iter
+    (0..8_u64).chain(0..).max(); // infinite iter
+    (0_usize..).chain([0usize, 1, 2].iter().cloned()).skip_while(|x| *x != 42).min(); // infinite iter
+    (0..8_u32).rev().cycle().map(|x| x + 1_u32).for_each(|x| println!("{}", x)); // infinite iter
+    (0..3_u32).flat_map(|x| x..).sum::<u32>(); // infinite iter
+    (0_usize..).flat_map(|x| 0..x).product::<usize>();  // infinite iter
+    (0_u64..).filter(|x| x % 2 == 0).last(); // infinite iter
+    (0..42_u64).by_ref().last(); // not an infinite, because ranges are double-ended
+    (0..).next(); // iterator is not exhausted
+}
+
+#[deny(maybe_infinite_iter)]
+fn potential_infinite_iters() {
+    (0..).zip((0..).take_while(square_is_lower_64)).count(); // maybe infinite iter
+    repeat(42).take_while(|x| *x == 42).chain(0..42).max(); // maybe infinite iter
+    (1..).scan(0, |state, x| { *state += x; Some(*state) }).min(); // maybe infinite iter
+    (0..).find(|x| *x == 24); // maybe infinite iter
+    (0..).position(|x| x == 24); // maybe infinite iter
+    (0..).any(|x| x == 24); // maybe infinite iter
+    (0..).all(|x| x == 24); // maybe infinite iter
+
+    (0..).zip(0..42).take_while(|&(x, _)| x != 42).count(); // not infinite
+    repeat(42).take_while(|x| *x == 42).next(); // iterator is not exhausted
+}
+
+fn main() {
+    infinite_iters();
+    potential_infinite_iters();
+}

tests/ui/infinite_iter.stderr

@@ -0,0 +1,100 @@
+error: you are collect()ing an iterator and throwing away the result. Consider using an explicit for loop to exhaust the iterator
+  --> $DIR/infinite_iter.rs:11:5
+   |
+11 |     repeat(0_u8).collect::<Vec<_>>(); // infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D unused-collect` implied by `-D warnings`
+
+error: infinite iteration detected
+  --> $DIR/infinite_iter.rs:11:5
+   |
+11 |     repeat(0_u8).collect::<Vec<_>>(); // infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/infinite_iter.rs:9:8
+   |
+9  | #[deny(infinite_iter)]
+   |        ^^^^^^^^^^^^^
+
+error: infinite iteration detected
+  --> $DIR/infinite_iter.rs:12:5
+   |
+12 |     (0..8_u32).take_while(square_is_lower_64).cycle().count(); // infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: infinite iteration detected
+  --> $DIR/infinite_iter.rs:13:5
+   |
+13 |     (0..8_u64).chain(0..).max(); // infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: infinite iteration detected
+  --> $DIR/infinite_iter.rs:15:5
+   |
+15 |     (0..8_u32).rev().cycle().map(|x| x + 1_u32).for_each(|x| println!("{}", x)); // infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: infinite iteration detected
+  --> $DIR/infinite_iter.rs:17:5
+   |
+17 |     (0_usize..).flat_map(|x| 0..x).product::<usize>();  // infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: infinite iteration detected
+  --> $DIR/infinite_iter.rs:18:5
+   |
+18 |     (0_u64..).filter(|x| x % 2 == 0).last(); // infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: possible infinite iteration detected
+  --> $DIR/infinite_iter.rs:25:5
+   |
+25 |     (0..).zip((0..).take_while(square_is_lower_64)).count(); // maybe infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/infinite_iter.rs:23:8
+   |
+23 | #[deny(maybe_infinite_iter)]
+   |        ^^^^^^^^^^^^^^^^^^^
+
+error: possible infinite iteration detected
+  --> $DIR/infinite_iter.rs:26:5
+   |
+26 |     repeat(42).take_while(|x| *x == 42).chain(0..42).max(); // maybe infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: possible infinite iteration detected
+  --> $DIR/infinite_iter.rs:27:5
+   |
+27 |     (1..).scan(0, |state, x| { *state += x; Some(*state) }).min(); // maybe infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: possible infinite iteration detected
+  --> $DIR/infinite_iter.rs:28:5
+   |
+28 |     (0..).find(|x| *x == 24); // maybe infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: possible infinite iteration detected
+  --> $DIR/infinite_iter.rs:29:5
+   |
+29 |     (0..).position(|x| x == 24); // maybe infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: possible infinite iteration detected
+  --> $DIR/infinite_iter.rs:30:5
+   |
+30 |     (0..).any(|x| x == 24); // maybe infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+error: possible infinite iteration detected
+  --> $DIR/infinite_iter.rs:31:5
+   |
+31 |     (0..).all(|x| x == 24); // maybe infinite iter
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 14 previous errors
+