diff --git a/CHANGELOG.md b/CHANGELOG.md index cbbd5497c79..9bd8affcded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -154,9 +154,7 @@ All notable changes to this project will be documented in this file. [`explicit_counter_loop`]: https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_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_flat_map`]: https://github.com/Manishearth/rust-clippy/wiki#filter_flat_map [`filter_map`]: https://github.com/Manishearth/rust-clippy/wiki#filter_map -[`filter_map_flat_map`]: https://github.com/Manishearth/rust-clippy/wiki#filter_map_flat_map [`filter_next`]: https://github.com/Manishearth/rust-clippy/wiki#filter_next [`float_arithmetic`]: https://github.com/Manishearth/rust-clippy/wiki#float_arithmetic [`float_cmp`]: https://github.com/Manishearth/rust-clippy/wiki#float_cmp diff --git a/README.md b/README.md index 8f9d47f6ee9..4345c504822 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Table of contents: ## Lints -There are 157 lints included in this crate: +There are 155 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -60,9 +60,7 @@ name [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_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_flat_map](https://github.com/Manishearth/rust-clippy/wiki#filter_flat_map) | allow | using `filter(_).flat_map(_)`, which can be rewritten using just the flat_map [filter_map](https://github.com/Manishearth/rust-clippy/wiki#filter_map) | allow | using `filter(_).map(_)`, which is more succinctly expressed as `.filter_map(_)` -[filter_map_flat_map](https://github.com/Manishearth/rust-clippy/wiki#filter_map_flat_map) | allow | using `filter_map(_).flat_map(_)`, which can be rewritten using just the flat_map [filter_next](https://github.com/Manishearth/rust-clippy/wiki#filter_next) | warn | using `filter(p).next()`, which is more succinctly expressed as `.find(p)` [float_arithmetic](https://github.com/Manishearth/rust-clippy/wiki#float_arithmetic) | allow | Any floating-point arithmetic statement [float_cmp](https://github.com/Manishearth/rust-clippy/wiki#float_cmp) | warn | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a10ab8add0c..835823b8cd3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -268,9 +268,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { items_after_statements::ITEMS_AFTER_STATEMENTS, matches::SINGLE_MATCH_ELSE, mem_forget::MEM_FORGET, - methods::FILTER_FLAT_MAP, methods::FILTER_MAP, - methods::FILTER_MAP_FLAT_MAP, methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, methods::WRONG_PUB_SELF_CONVENTION, diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 3b7122cb410..97130ee751f 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -159,11 +159,11 @@ declare_lint! { "using `filter(p).next()`, which is more succinctly expressed as `.find(p)`" } -/// **What it does:** This lint `Warn`s on `_.filter(_).map(_)`. +/// **What it does:** This lint `Warn`s on `_.filter(_).map(_)`, `_.filter(_).flat_map(_)`, `_.filter_map(_).flat_map(_)` and similar. /// -/// **Why is this bad?** Readability, this can be written more concisely as `_.filter_map(_)`. +/// **Why is this bad?** Readability, this can be written more concisely as a single method call /// -/// **Known problems:** Often requires a condition + Option creation in `filter_map` +/// **Known problems:** Often requires a condition + Option/Iterator creation inside the closure /// /// **Example:** `iter.filter(|x| x == 0).map(|x| x * 2)` declare_lint! { @@ -171,30 +171,6 @@ declare_lint! { "using `filter(_).map(_)`, which is more succinctly expressed as `.filter_map(_)`" } -/// **What it does:** This lint `Warn`s on `_.filter(_).flat_map(_)`. -/// -/// **Why is this bad?** Readability, this just needs the `flat_map` to return an empty iterator, if the value should be filtered. -/// -/// **Known problems:** Often requires a condition + Iterator creation in `flat_map` -/// -/// **Example:** `iter.filter(|x| x == 0).flat_map(|x| x.bits())` -declare_lint! { - pub FILTER_FLAT_MAP, Allow, - "using `filter(_).flat_map(_)`, which can be rewritten using just the flat_map" -} - -/// **What it does:** This lint `Warn`s on `_.filter_map(_).flat_map(_)`. -/// -/// **Why is this bad?** Readability, this just needs the `flat_map` to return an empty iterator, if the value should be filtered. -/// -/// **Known problems:** Often requires a condition + Iterator creation in `flat_map` -/// -/// **Example:** `iter.filter_map(|x| x.process()).flat_map(|x| x.bits())` -declare_lint! { - pub FILTER_MAP_FLAT_MAP, Allow, - "using `filter_map(_).flat_map(_)`, which can be rewritten using just the flat_map" -} - /// **What it does:** This lint `Warn`s on an iterator search (such as `find()`, `position()`, or /// `rposition()`) followed by a call to `is_some()`. /// @@ -393,8 +369,6 @@ impl LintPass for Pass { SEARCH_IS_SOME, TEMPORARY_CSTRING_AS_PTR, FILTER_MAP, - FILTER_FLAT_MAP, - FILTER_MAP_FLAT_MAP, ITER_NTH) } } @@ -420,6 +394,8 @@ impl LateLintPass for Pass { lint_filter_next(cx, expr, arglists[0]); } else if let Some(arglists) = method_chain_args(expr, &["filter", "map"]) { lint_filter_map(cx, expr, arglists[0], arglists[1]); + } else if let Some(arglists) = method_chain_args(expr, &["filter_map", "map"]) { + lint_filter_map_map(cx, expr, arglists[0], arglists[1]); } else if let Some(arglists) = method_chain_args(expr, &["filter", "flat_map"]) { lint_filter_flat_map(cx, expr, arglists[0], arglists[1]); } else if let Some(arglists) = method_chain_args(expr, &["filter_map", "flat_map"]) { @@ -884,8 +860,19 @@ fn lint_filter_next(cx: &LateContext, expr: &hir::Expr, filter_args: &MethodArgs fn lint_filter_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs, _map_args: &MethodArgs) { // lint if caller of `.filter().map()` is an Iterator if match_trait_method(cx, expr, &paths::ITERATOR) { - let msg = "called `filter(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.filter_map(..)` \ - instead."; + let msg = "called `filter(p).map(q)` on an `Iterator`. \ + This is more succinctly expressed by calling `.filter_map(..)` instead."; + span_lint(cx, FILTER_MAP, expr.span, msg); + } +} + +// Type of MethodArgs is potentially a Vec +/// lint use of `filter().map()` for `Iterators` +fn lint_filter_map_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs, _map_args: &MethodArgs) { + // lint if caller of `.filter().map()` is an Iterator + if match_trait_method(cx, expr, &paths::ITERATOR) { + let msg = "called `filter_map(p).map(q)` on an `Iterator`. \ + This is more succinctly expressed by only calling `.filter_map(..)` instead."; span_lint(cx, FILTER_MAP, expr.span, msg); } } @@ -895,9 +882,10 @@ fn lint_filter_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs fn lint_filter_flat_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs, _map_args: &MethodArgs) { // lint if caller of `.filter().flat_map()` is an Iterator if match_trait_method(cx, expr, &paths::ITERATOR) { - let msg = "called `filter(p).flat_map(q)` on an `Iterator`. This is more succinctly expressed by calling `.flat_map(..)` \ + let msg = "called `filter(p).flat_map(q)` on an `Iterator`. \ + This is more succinctly expressed by calling `.flat_map(..)` \ and filtering by returning an empty Iterator."; - span_lint(cx, FILTER_FLAT_MAP, expr.span, msg); + span_lint(cx, FILTER_MAP, expr.span, msg); } } @@ -906,9 +894,10 @@ fn lint_filter_flat_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &Metho fn lint_filter_map_flat_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs, _map_args: &MethodArgs) { // lint if caller of `.filter_map().flat_map()` is an Iterator if match_trait_method(cx, expr, &paths::ITERATOR) { - let msg = "called `filter_map(p).flat_map(q)` on an `Iterator`. This is more succinctly expressed by calling `.flat_map(..)` \ + let msg = "called `filter_map(p).flat_map(q)` on an `Iterator`. \ + This is more succinctly expressed by calling `.flat_map(..)` \ and filtering by returning an empty Iterator."; - span_lint(cx, FILTER_MAP_FLAT_MAP, expr.span, msg); + span_lint(cx, FILTER_MAP, expr.span, msg); } } diff --git a/tests/compile-fail/filter_methods.rs b/tests/compile-fail/filter_methods.rs index 8f37a8553b4..743c3c15aeb 100644 --- a/tests/compile-fail/filter_methods.rs +++ b/tests/compile-fail/filter_methods.rs @@ -17,4 +17,9 @@ fn main() { .filter_map(|x| x.checked_mul(2)) .flat_map(|x| x.checked_mul(2)) .collect(); + + let _: Vec<_> = vec![5i8; 6].into_iter() //~ERROR called `filter_map(p).map(q)` on an `Iterator` + .filter_map(|x| x.checked_mul(2)) + .map(|x| x.checked_mul(2)) + .collect(); }