From: Manish Goregaokar Date: Mon, 3 Oct 2016 16:45:23 +0000 (+0530) Subject: Merge pull request #1255 from Manishearth/cov X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=a4198c11086c4ece865e3cfa84832c774c5be0a7;hp=6800111c8edcaaf705f16a8b2f4b0497eeea1c52;p=rust.git Merge pull request #1255 from Manishearth/cov Improve test coverage --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d02908a802..d82e16f02a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ # Change Log All notable changes to this project will be documented in this file. +## 0.0.93 — ? +* [`option_map_unwrap_or`] and [`option_map_unwrap_or_else`] are now + allowed by default. +* New lint: [`explicit_into_iter_loop`] + ## 0.0.92 — 2016-09-30 * Rustup to *rustc 1.14.0-nightly (289f3a4ca 2016-09-29)* @@ -230,6 +235,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..8b119ecf627 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 @@ -282,8 +283,8 @@ name [nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options) | warn | nonsensical combination of options for opening a file [not_unsafe_ptr_arg_deref](https://github.com/Manishearth/rust-clippy/wiki#not_unsafe_ptr_arg_deref) | warn | public functions dereferencing raw pointer arguments but not marked `unsafe` [ok_expect](https://github.com/Manishearth/rust-clippy/wiki#ok_expect) | warn | using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result -[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | warn | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)` -[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)` +[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | allow | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)` +[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | allow | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)` [option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used) | allow | using `Option.unwrap()`, which should at least get a better message using `expect()` [or_fun_call](https://github.com/Manishearth/rust-clippy/wiki#or_fun_call) | warn | using any `*or` method with a function call, which suggests `*or_else` [out_of_bounds_indexing](https://github.com/Manishearth/rust-clippy/wiki#out_of_bounds_indexing) | deny | out of bounds constant indexing diff --git a/clippy_lints/src/eval_order_dependence.rs b/clippy_lints/src/eval_order_dependence.rs index e53c830e11d..28e385abd82 100644 --- a/clippy_lints/src/eval_order_dependence.rs +++ b/clippy_lints/src/eval_order_dependence.rs @@ -104,7 +104,7 @@ fn maybe_walk_expr(&mut self, e: &Expr) { self.visit_expr(guard); } // make sure top level arm expressions aren't linted - walk_expr(self, &*arm.body); + self.maybe_walk_expr(&*arm.body); } } _ => walk_expr(self, e), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 67108eb5688..4cfd27c9453 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -271,6 +271,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { matches::SINGLE_MATCH_ELSE, mem_forget::MEM_FORGET, methods::FILTER_MAP, + methods::OPTION_MAP_UNWRAP_OR, + methods::OPTION_MAP_UNWRAP_OR_ELSE, methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, methods::WRONG_PUB_SELF_CONVENTION, @@ -346,6 +348,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, @@ -369,8 +372,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { methods::ITER_NTH, methods::NEW_RET_NO_SELF, methods::OK_EXPECT, - methods::OPTION_MAP_UNWRAP_OR, - methods::OPTION_MAP_UNWRAP_OR_ELSE, methods::OR_FUN_CALL, methods::SEARCH_IS_SOME, methods::SHOULD_IMPLEMENT_TRAIT, 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 @@ "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/methods.rs b/clippy_lints/src/methods.rs index a0a1059ba48..13664e7d912 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -168,7 +168,7 @@ /// ``` declare_lint! { pub OPTION_MAP_UNWRAP_OR, - Warn, + Allow, "using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as \ `map_or(a, f)`" } @@ -186,7 +186,7 @@ /// ``` declare_lint! { pub OPTION_MAP_UNWRAP_OR_ELSE, - Warn, + Allow, "using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as \ `map_or_else(g, f)`" } 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 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/diverging_sub_expression.rs b/tests/compile-fail/diverging_sub_expression.rs index 782c406d74c..f9a6e66da42 100644 --- a/tests/compile-fail/diverging_sub_expression.rs +++ b/tests/compile-fail/diverging_sub_expression.rs @@ -31,6 +31,10 @@ fn foobar() { 8 => break, 9 => diverge(), 3 => (println!("moo"), diverge()), //~ ERROR sub-expression diverges + 10 => match 42 { + 99 => return, + _ => ((), panic!("boo")), + }, _ => (println!("boo"), break), //~ ERROR sub-expression diverges }; } 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 @@ fn iter(&self) -> std::slice::Iter { } } -#[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