From 5fa004313bd31a88b3f5f4c4467b4926e24cebac Mon Sep 17 00:00:00 2001 From: Elliott Clark Date: Fri, 30 Sep 2016 17:01:30 -0700 Subject: [PATCH 1/3] Add a lint to warn about un-necessary .into_iter() This should close #1094. --- clippy_lints/src/loops.rs | 29 +++++++++++++++++++++++++++++ clippy_lints/src/utils/paths.rs | 1 + tests/compile-fail/for_loop.rs | 6 +++++- 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 8c033a2d4ab..fad72db74ed 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -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 @@ -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, @@ -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, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index cd9b7c83eda..2f05ce37d33 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -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"]; diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index 91e31adc44d..22fcbff64b1 100644 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -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)] @@ -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 From 4a34087c7736a212d9c35f53cbc108c8386e7c01 Mon Sep 17 00:00:00 2001 From: Elliott Clark Date: Fri, 30 Sep 2016 17:50:02 -0700 Subject: [PATCH 2/3] Run util/update_lints.py after adding explicit_into_iter_loop --- CHANGELOG.md | 1 + README.md | 3 ++- clippy_lints/src/lib.rs | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d02908a802..4a0649088ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -230,6 +230,7 @@ All notable changes to this project will be documented in this file. [`eval_order_dependence`]: https://github.com/Manishearth/rust-clippy/wiki#eval_order_dependence [`expl_impl_clone_on_copy`]: https://github.com/Manishearth/rust-clippy/wiki#expl_impl_clone_on_copy [`explicit_counter_loop`]: https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop +[`explicit_into_iter_loop`]: https://github.com/Manishearth/rust-clippy/wiki#explicit_into_iter_loop [`explicit_iter_loop`]: https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop [`extend_from_slice`]: https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice [`filter_map`]: https://github.com/Manishearth/rust-clippy/wiki#filter_map diff --git a/README.md b/README.md index 7aec86c2f39..7d6ed96681a 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ You can check out this great service at [clippy.bashy.io](https://clippy.bashy.i ## Lints -There are 171 lints included in this crate: +There are 172 lints included in this crate: name | default | triggers on ---------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -220,6 +220,7 @@ name [eval_order_dependence](https://github.com/Manishearth/rust-clippy/wiki#eval_order_dependence) | warn | whether a variable read occurs before a write depends on sub-expression evaluation order [expl_impl_clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#expl_impl_clone_on_copy) | warn | implementing `Clone` explicitly on `Copy` types [explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop) | warn | for-looping with an explicit counter when `_.enumerate()` would do +[explicit_into_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_into_iter_loop) | warn | for-looping over `_.into_iter()` when `_` would do [explicit_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop) | warn | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do [extend_from_slice](https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice) | warn | `.extend_from_slice(_)` is a faster way to extend a Vec by a slice [filter_map](https://github.com/Manishearth/rust-clippy/wiki#filter_map) | allow | using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 67108eb5688..fd42d91f41d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -346,6 +346,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { lifetimes::UNUSED_LIFETIMES, loops::EMPTY_LOOP, loops::EXPLICIT_COUNTER_LOOP, + loops::EXPLICIT_INTO_ITER_LOOP, loops::EXPLICIT_ITER_LOOP, loops::FOR_KV_MAP, loops::FOR_LOOP_OVER_OPTION, From 2e2052a747a87288ffb3d35c540af5f2c46867ef Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 1 Oct 2016 14:53:31 +0200 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0649088ab..cca4245bfa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Change Log All notable changes to this project will be documented in this file. +## 0.0.93 — ? +* New lint: [`explicit_into_iter_loop`] + ## 0.0.92 — 2016-09-30 * Rustup to *rustc 1.14.0-nightly (289f3a4ca 2016-09-29)*