Merge pull request #1255 from Manishearth/cov

Improve test coverage

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5d02908a8027b40ebee35afde70749613f84cc7c..d82e16f02a9ed5b297bfedb1595593a1069e660c 100644 (file)
--- 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 7aec86c2f39e64221b06d264345c33b8b10e6874..8b119ecf627482279c84ab4abac4e8f62c07c802 100644 (file)
--- 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 e53c830e11dec860bf8bde8758c0762261ae69fa..28e385abd8253aefb1c574bdcde9953bf1dd0dff 100644 (file)
--- 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 67108eb5688c0adab080286e58584feae62745d9..4cfd27c9453a5f33692b80a1982c87c2f24ccb29 100644 (file)
--- 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 8c033a2d4ab40dc03bdb0ba107a2415cef9ecda9..fad72db74ed4a382d8f6668346aac330b504c600 100644 (file)
--- 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 a0a1059ba4896d57ba0368b68f633294f2cf4d65..13664e7d9129393147cb3124594047f9cbae8cac 100644 (file)
--- 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 cd9b7c83eda6d76a30f4586cee5a6aad2578d713..2f05ce37d334528fb9e3b106512a48d9af1aca30 100644 (file)
--- 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 782c406d74c881ff25d24a7b80cd8739e47a8fd7..f9a6e66da42f3fd600bbb24769275fe13a4b2240 100644 (file)
--- 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 91e31adc44d580dc75534f22dfc5f7e3dddd37de..22fcbff64b15586421b4290107267093e20cd24d 100644 (file)
--- a/tests/compile-fail/for_loop.rs
+++ b/tests/compile-fail/for_loop.rs
@@ -87,7 +87,7 @@ fn iter(&self) -> std::slice::Iter<u8> {
     }
 }
 
-#[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