diff --git a/README.md b/README.md index bc82b2ee533..5ec1040b2c4 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ float_cmp | warn | using `==` or `!=` on float values (as floating identity_op | warn | using identity operations, e.g. `x + 0` or `y / 1` ineffective_bit_mask | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2` inline_always | warn | `#[inline(always)]` is a bad idea in most cases +iter_next_loop | warn | for-looping over `_.next()` which is probably not intended len_without_is_empty | warn | traits and impls that have `.len()` but not `.is_empty()` len_zero | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead let_and_return | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a function diff --git a/src/lib.rs b/src/lib.rs index 967865b2c0c..4c0617b1696 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,6 +80,7 @@ pub fn plugin_registrar(reg: &mut Registry) { len_zero::LEN_ZERO, lifetimes::NEEDLESS_LIFETIMES, loops::EXPLICIT_ITER_LOOP, + loops::ITER_NEXT_LOOP, loops::NEEDLESS_RANGE_LOOP, methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, diff --git a/src/loops.rs b/src/loops.rs index 092b5ce1196..44827d08bcd 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -1,9 +1,10 @@ use rustc::lint::*; use syntax::ast::*; use syntax::visit::{Visitor, walk_expr}; +use rustc::middle::ty; use std::collections::HashSet; -use utils::{snippet, span_lint, get_parent_expr}; +use utils::{snippet, span_lint, get_parent_expr, match_def_path}; declare_lint!{ pub NEEDLESS_RANGE_LOOP, Warn, "for-looping over a range of indices where an iterator over items would do" } @@ -11,12 +12,15 @@ declare_lint!{ pub NEEDLESS_RANGE_LOOP, Warn, declare_lint!{ pub EXPLICIT_ITER_LOOP, Warn, "for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do" } +declare_lint!{ pub ITER_NEXT_LOOP, Warn, + "for-looping over `_.next()` which is probably not intended" } + #[derive(Copy, Clone)] pub struct LoopsPass; impl LintPass for LoopsPass { fn get_lints(&self) -> LintArray { - lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP) + lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP) } fn check_expr(&mut self, cx: &Context, expr: &Expr) { @@ -47,11 +51,11 @@ impl LintPass for LoopsPass { } } - // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x if let ExprMethodCall(ref method, _, ref args) = arg.node { - // just the receiver, no arguments to iter() or iter_mut() + // just the receiver, no arguments if args.len() == 1 { let method_name = method.node.name; + // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x if method_name == "iter" { let object = snippet(cx, args[0].span, "_"); span_lint(cx, EXPLICIT_ITER_LOOP, expr.span, &format!( @@ -62,6 +66,19 @@ impl LintPass for LoopsPass { span_lint(cx, EXPLICIT_ITER_LOOP, expr.span, &format!( "it is more idiomatic to loop over `&mut {}` instead of `{}.iter_mut()`", object, object)); + // check for looping over Iterator::next() which is not what you want + } else if method_name == "next" { + let method_call = ty::MethodCall::expr(arg.id); + let trt_id = cx.tcx.tables + .borrow().method_map.get(&method_call) + .and_then(|callee| cx.tcx.trait_of_item(callee.def_id)); + if let Some(trt_id) = trt_id { + if match_def_path(cx, trt_id, &["core", "iter", "Iterator"]) { + span_lint(cx, ITER_NEXT_LOOP, expr.span, + "you are iterating over `Iterator::next()` which is an Option; \ + this will compile but is probably not what you want"); + } + } } } } diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index 550f9869291..a4e3cc31a88 100755 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -1,7 +1,14 @@ #![feature(plugin)] #![plugin(clippy)] -#[deny(needless_range_loop, explicit_iter_loop)] +struct Unrelated(Vec); +impl Unrelated { + fn next(&self) -> std::slice::Iter { + self.0.iter() + } +} + +#[deny(needless_range_loop, explicit_iter_loop, iter_next_loop)] fn main() { let mut vec = vec![1, 2, 3, 4]; let vec2 = vec![1, 2, 3, 4]; @@ -20,4 +27,9 @@ fn main() { for _v in &vec { } // these are fine for _v in &mut vec { } // these are fine + + for _v in vec.iter().next() { } //~ERROR you are iterating over `Iterator::next()` + + let u = Unrelated(vec![]); + for _v in u.next() { } // no error }