diff --git a/CHANGELOG.md b/CHANGELOG.md index 58ded1b33..8b0a1f3be 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index bc043ec62..aa36871b6 100644 --- a/README.md +++ b/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 diff --git a/clippy_lints/src/infinite_iter.rs b/clippy_lints/src/infinite_iter.rs new file mode 100644 index 000000000..72ff75dd9 --- /dev/null +++ b/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::>() +/// ``` +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 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 +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c4c949c79..892d664aa 100644 --- a/clippy_lints/src/lib.rs +++ b/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, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 1a49dad1a..905792009 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/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"]; diff --git a/tests/ui/infinite_iter.rs b/tests/ui/infinite_iter.rs new file mode 100644 index 000000000..fd433edf9 --- /dev/null +++ b/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::>(); // 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::(); // infinite iter + (0_usize..).flat_map(|x| 0..x).product::(); // 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(); +} diff --git a/tests/ui/infinite_iter.stderr b/tests/ui/infinite_iter.stderr new file mode 100644 index 000000000..b3d2f08a8 --- /dev/null +++ b/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::>(); // 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::>(); // 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::(); // 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 +