Skip to content

Commit

Permalink
Add a lint to warn about un-necessary .into_iter()
Browse files Browse the repository at this point in the history
This should close #1094.
  • Loading branch information
elliottneilclark committed Oct 1, 2016
1 parent 8426947 commit 5fa0043
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
29 changes: 29 additions & 0 deletions clippy_lints/src/loops.rs
Expand Up @@ -58,6 +58,24 @@ declare_lint! {
"for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do"
}

/// **What it does:** Checks for loops on `y.into_iter()` where `y` will do, and
/// suggests the latter.
///
/// **Why is this bad?** Readability.
///
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// // with `y` a `Vec` or slice:
/// for x in y.into_iter() { .. }
/// ```
declare_lint! {
pub EXPLICIT_INTO_ITER_LOOP,
Warn,
"for-looping over `_.into_iter()` when `_` would do"
}

/// **What it does:** Checks for loops on `x.next()`.
///
/// **Why is this bad?** `next()` returns either `Some(value)` if there was a
Expand Down Expand Up @@ -275,6 +293,7 @@ impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(NEEDLESS_RANGE_LOOP,
EXPLICIT_ITER_LOOP,
EXPLICIT_INTO_ITER_LOOP,
ITER_NEXT_LOOP,
FOR_LOOP_OVER_RESULT,
FOR_LOOP_OVER_OPTION,
Expand Down Expand Up @@ -577,6 +596,16 @@ fn check_for_loop_arg(cx: &LateContext, pat: &Pat, arg: &Expr, expr: &Expr) {
object,
method_name));
}
} else if method_name.as_str() == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) {
let object = snippet(cx, args[0].span, "_");
span_lint(cx,
EXPLICIT_INTO_ITER_LOOP,
expr.span,
&format!("it is more idiomatic to loop over `{}` instead of `{}.{}()`",
object,
object,
method_name));

} else if method_name.as_str() == "next" && match_trait_method(cx, arg, &paths::ITERATOR) {
span_lint(cx,
ITER_NEXT_LOOP,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Expand Up @@ -23,6 +23,7 @@ pub const HASH: [&'static str; 2] = ["hash", "Hash"];
pub const HASHMAP: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"];
pub const HASHMAP_ENTRY: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"];
pub const HASHSET: [&'static str; 5] = ["std", "collections", "hash", "set", "HashSet"];
pub const INTO_ITERATOR: [&'static str; 4] = ["core", "iter", "traits", "IntoIterator"];
pub const IO_PRINT: [&'static str; 3] = ["std", "io", "_print"];
pub const ITERATOR: [&'static str; 4] = ["core", "iter", "iterator", "Iterator"];
pub const LINKED_LIST: [&'static str; 3] = ["collections", "linked_list", "LinkedList"];
Expand Down
6 changes: 5 additions & 1 deletion tests/compile-fail/for_loop.rs
Expand Up @@ -87,7 +87,7 @@ impl Unrelated {
}
}

#[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)]
#[deny(needless_range_loop, explicit_iter_loop, explicit_into_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)]
#[deny(unused_collect)]
#[allow(linkedlist, shadow_unrelated, unnecessary_mut_passed, cyclomatic_complexity, similar_names)]
#[allow(many_single_char_names)]
Expand Down Expand Up @@ -294,6 +294,10 @@ fn main() {
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`


let out_vec = vec![1,2,3];
for _v in out_vec.into_iter() { } //~ERROR it is more idiomatic to loop over `out_vec` instead of `out_vec.into_iter()`

for _v in &vec { } // these are fine
for _v in &mut vec { } // these are fine

Expand Down

0 comments on commit 5fa0043

Please sign in to comment.