diff --git a/README.md b/README.md index d9dea816318..bc82b2ee533 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ non_ascii_literal | allow | using any literal non-ASCII chars in a string l option_unwrap_used | allow | using `Option.unwrap()`, which should at least get a better message using `expect()` precedence | warn | expressions where precedence may trip up the unwary reader of the source; suggests adding parentheses, e.g. `x << 2 + y` will be parsed as `x << (2 + y)` ptr_arg | allow | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively +range_step_by_zero | warn | using Range::step_by(0), which produces an infinite iterator redundant_closure | warn | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`) result_unwrap_used | allow | using `Result.unwrap()`, which might be better handled single_match | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead diff --git a/src/lib.rs b/src/lib.rs index 01a2d65606c..91859005bf3 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,6 +35,7 @@ pub mod returns; pub mod lifetimes; pub mod loops; +pub mod ranges; #[plugin_registrar] pub fn plugin_registrar(reg: &mut Registry) { @@ -64,6 +65,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_lint_pass(box types::LetPass as LintPassObject); reg.register_lint_pass(box loops::LoopsPass as LintPassObject); reg.register_lint_pass(box lifetimes::LifetimePass as LintPassObject); + reg.register_lint_pass(box ranges::StepByZero as LintPassObject); reg.register_lint_group("clippy", vec![ approx_const::APPROX_CONSTANT, @@ -93,6 +95,7 @@ pub fn plugin_registrar(reg: &mut Registry) { mut_mut::MUT_MUT, needless_bool::NEEDLESS_BOOL, ptr_arg::PTR_ARG, + ranges::RANGE_STEP_BY_ZERO, returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, strings::STRING_ADD, diff --git a/src/ranges.rs b/src/ranges.rs new file mode 100644 index 00000000000..2981574e502 --- /dev/null +++ b/src/ranges.rs @@ -0,0 +1,52 @@ +use rustc::lint::{Context, LintArray, LintPass}; +use rustc::middle::ty::TypeVariants::TyStruct; +use syntax::ast::*; +use syntax::codemap::Spanned; +use utils::{match_def_path, walk_ptrs_ty}; + +declare_lint! { + pub RANGE_STEP_BY_ZERO, Warn, + "using Range::step_by(0), which produces an infinite iterator" +} + +#[derive(Copy,Clone)] +pub struct StepByZero; + +impl LintPass for StepByZero { + fn get_lints(&self) -> LintArray { + lint_array!(RANGE_STEP_BY_ZERO) + } + + fn check_expr(&mut self, cx: &Context, expr: &Expr) { + if let ExprMethodCall(Spanned { node: ref ident, .. }, _, + ref args) = expr.node { + // Only warn on literal ranges. + if ident.name.as_str() == "step_by" && args.len() == 2 && + is_range(cx, &args[0]) && is_lit_zero(&args[1]) { + cx.span_lint(RANGE_STEP_BY_ZERO, expr.span, + "Range::step_by(0) produces an infinite iterator. \ + Consider using `std::iter::repeat()` instead") + } + } + } +} + +fn is_range(cx: &Context, expr: &Expr) -> bool { + // No need for walk_ptrs_ty here because step_by moves self, so it + // can't be called on a borrowed range. + if let TyStruct(did, _) = cx.tcx.expr_ty(expr).sty { + // Note: RangeTo and RangeFull don't have step_by + match_def_path(cx, did.did, &["core", "ops", "Range"]) || + match_def_path(cx, did.did, &["core", "ops", "RangeFrom"]) + } else { false } +} + +fn is_lit_zero(expr: &Expr) -> bool { + // FIXME: use constant folding + if let ExprLit(ref spanned) = expr.node { + if let LitInt(0, _) = spanned.node { + return true; + } + } + false +} diff --git a/tests/compile-fail/range.rs b/tests/compile-fail/range.rs new file mode 100644 index 00000000000..324f129fafa --- /dev/null +++ b/tests/compile-fail/range.rs @@ -0,0 +1,24 @@ +#![feature(step_by)] +#![feature(plugin)] +#![plugin(clippy)] + +struct NotARange; +impl NotARange { + fn step_by(&self, _: u32) {} +} + +#[deny(range_step_by_zero)] +fn main() { + (0..1).step_by(0); //~ERROR Range::step_by(0) produces an infinite iterator + // No warning for non-zero step + (0..1).step_by(1); + + (1..).step_by(0); //~ERROR Range::step_by(0) produces an infinite iterator + + let x = 0..1; + x.step_by(0); //~ERROR Range::step_by(0) produces an infinite iterator + + // No error, not a range. + let y = NotARange; + y.step_by(0); +}