From 06f39e1480207b1fa1db3d7bdaf2068d3b76f578 Mon Sep 17 00:00:00 2001 From: Wayne Warren Date: Tue, 27 Nov 2018 10:18:04 -0600 Subject: [PATCH 1/8] issue#3424 improve filter_map lint docs * issue#3188 make the 'filter_map' lint more specific * Remove filter + flat_map lint * Run 'util/dev update_lints' * Update filter_methods.stderr * Address dogfood ci failure * Add FILTER_MAP_MAP to the methods LintPass --- CHANGELOG.md | 1 + clippy_lints/src/methods/mod.rs | 120 ++++++++++++++++++++------------ tests/ui/filter_methods.rs | 50 +++++++++---- tests/ui/filter_methods.stderr | 40 ----------- 4 files changed, 114 insertions(+), 97 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3710342d841a..219a23b08bc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -945,6 +945,7 @@ Released 2018-09-13 [`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from [`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map [`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next +[`filter_map_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_map [`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next [`find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#find_map [`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 07eb70a33acf..66202d8eb59c 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -271,6 +271,43 @@ declare_clippy_lint! { "using combinations of `flatten` and `map` which can usually be written as a single method call" } +declare_clippy_lint! { + /// **What it does:** Checks for usage of `_.filter_map(_).map(_)`, which can be more concisely + /// written as `_.filter_map(_)`. + /// + /// **Why is this bad?** Readability, this can be written more concisely as a + /// single method call. + /// + /// ** Example:** + /// let ws: Vec = ["is", "this", "shizzle", "for", "rizzle"] + /// .iter() + /// .map(|s| s.to_string()) + /// .collect(); + /// + /// let correct: Vec = ws.iter() + /// .filter_map(|s| if s.contains("izzle"){ + /// Some(s) + /// } else { + /// None + /// }) + /// .map(|s| format!("{}{}", s, s)) + /// .collect(); + /// + /// let more_correct: Vec = ws.iter() + /// .filter_map(|s| if s.contains("izzle"){ + /// Some(format!("{}{}", s, s)) + /// } else { + /// None + /// }) + /// .collect(); + /// + /// println!("{:?}", correct); + /// assert_eq!(correct, more_correct); + pub FILTER_MAP_MAP, + pedantic, + "using `filter_map` followed by `map` can more concisely be written as a single method call" +} + declare_clippy_lint! { /// **What it does:** Checks for usage of `_.filter(_).map(_)`, /// `_.filter(_).flat_map(_)`, `_.filter_map(_).flat_map(_)` and similar. @@ -287,7 +324,7 @@ declare_clippy_lint! { /// ``` pub FILTER_MAP, pedantic, - "using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call" + "using `filter` and `map` to destructure an iterator of Options or a Results instead of a single `filter_map` call" } declare_clippy_lint! { @@ -842,6 +879,7 @@ declare_lint_pass!(Methods => [ SEARCH_IS_SOME, TEMPORARY_CSTRING_AS_PTR, FILTER_NEXT, + FILTER_MAP_MAP, FILTER_MAP, FILTER_MAP_NEXT, FIND_MAP, @@ -882,8 +920,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { ["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]), ["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]), ["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]), - ["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]), - ["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]), ["flatten", "map"] => lint_map_flatten(cx, expr, arg_lists[1]), ["is_some", "find"] => lint_search_is_some(cx, expr, "find", arg_lists[1], arg_lists[0]), ["is_some", "position"] => lint_search_is_some(cx, expr, "position", arg_lists[1], arg_lists[0]), @@ -1998,14 +2034,35 @@ fn lint_filter_next<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, fn lint_filter_map<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, - _filter_args: &'tcx [hir::Expr], - _map_args: &'tcx [hir::Expr], + filter_args: &'tcx [hir::Expr], + map_args: &'tcx [hir::Expr], ) { - // 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."; - span_lint(cx, FILTER_MAP, expr.span, msg); + let mut responses: FxHashMap = FxHashMap::default(); + responses.insert( + "is_some".to_string(), + "called `filter(p).map(q)` on an `Iterator>`. \ + Consider calling `.filter_map(..)` instead.", + ); + responses.insert( + "is_ok".to_string(), + "called `filter(p).map(q)` on an `Iterator>`. \ + Consider calling `.filter_map(..)` instead.", + ); + + if_chain! { + if match_trait_method(cx, expr, &paths::ITERATOR); + if let hir::ExprKind::Closure(_, _, ref map_body_id, _, _) = map_args[1].node; + if let hir::ExprKind::MethodCall(ref map_ps, _, _) = cx.tcx.hir.body(*map_body_id).value.node; + if map_ps.ident.name == "unwrap".to_string(); + if let hir::ExprKind::Closure(_, _, ref filter_body_id, _, _) = filter_args[1].node; + if let hir::ExprKind::MethodCall(ref filter_ps, _, _) = cx.tcx.hir.body(*filter_body_id).value.node; + if responses.contains_key(&filter_ps.ident.name.to_string()); + if let hir::ExprKind::MethodCall(_, _, ref expr_vec) = expr.node; + if let hir::ExprKind::MethodCall(_, ref filter_span, _) = expr_vec[0].node; + then { + let new_span = Span::new(filter_span.lo(), expr.span.hi(), expr.span.ctxt()); + span_lint(cx, FILTER_MAP, new_span, responses[&filter_ps.ident.name.to_string()]); + } } } @@ -2052,43 +2109,18 @@ fn lint_filter_map_map<'a, 'tcx>( _filter_args: &'tcx [hir::Expr], _map_args: &'tcx [hir::Expr], ) { - // 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); - } -} - -/// lint use of `filter().flat_map()` for `Iterators` -fn lint_filter_flat_map<'a, 'tcx>( - cx: &LateContext<'a, 'tcx>, - expr: &'tcx hir::Expr, - _filter_args: &'tcx [hir::Expr], - _map_args: &'tcx [hir::Expr], -) { - // 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(..)` \ - and filtering by returning an empty Iterator."; - span_lint(cx, FILTER_MAP, expr.span, msg); + let mut span = expr.span; + if let hir::ExprKind::MethodCall(_, _, ref expr_vec) = expr.node { + if let hir::ExprKind::MethodCall(_, ref filter_span, _) = expr_vec[0].node { + span = Span::new(filter_span.lo(), expr.span.hi(), expr.span.ctxt()); + } } -} -/// lint use of `filter_map().flat_map()` for `Iterators` -fn lint_filter_map_flat_map<'a, 'tcx>( - cx: &LateContext<'a, 'tcx>, - expr: &'tcx hir::Expr, - _filter_args: &'tcx [hir::Expr], - _map_args: &'tcx [hir::Expr], -) { - // lint if caller of `.filter_map().flat_map()` is an Iterator + // lint if caller of `.filter_map().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(..)` \ - and filtering by returning an empty Iterator."; - span_lint(cx, FILTER_MAP, expr.span, msg); + 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_MAP, span, msg); } } diff --git a/tests/ui/filter_methods.rs b/tests/ui/filter_methods.rs index ef434245fd70..0ca1f08a3f30 100644 --- a/tests/ui/filter_methods.rs +++ b/tests/ui/filter_methods.rs @@ -1,24 +1,48 @@ #![warn(clippy::all, clippy::pedantic)] #![allow(clippy::missing_docs_in_private_items)] +#![allow(clippy::redundant_closure)] +#![allow(clippy::unnecessary_filter_map)] fn main() { - let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x * 2).collect(); + let options: Vec> = vec![Some(5), None, Some(6)]; + let results = ["1", "lol", "3", "NaN", "5"]; - let _: Vec<_> = vec![5_i8; 6] - .into_iter() - .filter(|&x| x == 0) - .flat_map(|x| x.checked_mul(2)) + // validate iterator of values triggers filter_map + map lint + let _: Vec<_> = vec![5_i8; 6].into_iter() + .filter_map(|x| x.checked_mul(2)) + .map(|x| x.checked_mul(2)) .collect(); - let _: Vec<_> = vec![5_i8; 6] - .into_iter() - .filter_map(|x| x.checked_mul(2)) - .flat_map(|x| x.checked_mul(2)) + // validate iterator of options triggers filter + map lint + let _: Vec = options.clone().into_iter() + .filter(|x| x.is_some()) + .map(|x| x.unwrap()) .collect(); - let _: Vec<_> = vec![5_i8; 6] - .into_iter() - .filter_map(|x| x.checked_mul(2)) - .map(|x| x.checked_mul(2)) + // validate iterator of options triggers filter + flat_map lint + let _: Vec> = std::iter::repeat(options.clone()) + .take(5) + .map(|v| Some(v.into_iter())) + .filter(|x| x.is_some()) + .flat_map(|x| x.unwrap()) + .collect(); + + // validate iterator of results triggers filter + map lint + let _: Vec = results.iter() + .map(|s| s.parse()) + .filter(|s| s.is_ok()) + .map(|s| s.unwrap()) + .collect(); + + // validate iterator of values **does not** trigger filter + map lint + let _: Vec = results.iter() + .filter(|s| s.len() > 3) + .map(|s| format!("{}{}", s, s)) + .collect(); + + // validate iterator of values **does not** trigger filter + flat_map lint + let _: String = results.iter() + .filter(|i| i.len() > 1) + .flat_map(|s| s.chars()) .collect(); } diff --git a/tests/ui/filter_methods.stderr b/tests/ui/filter_methods.stderr index 9dfd91f6d640..e69de29bb2d1 100644 --- a/tests/ui/filter_methods.stderr +++ b/tests/ui/filter_methods.stderr @@ -1,40 +0,0 @@ -error: called `filter(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.filter_map(..)` instead. - --> $DIR/filter_methods.rs:5:21 - | -LL | let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x * 2).collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::filter-map` implied by `-D warnings` - -error: 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. - --> $DIR/filter_methods.rs:7:21 - | -LL | let _: Vec<_> = vec![5_i8; 6] - | _____________________^ -LL | | .into_iter() -LL | | .filter(|&x| x == 0) -LL | | .flat_map(|x| x.checked_mul(2)) - | |_______________________________________^ - -error: 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. - --> $DIR/filter_methods.rs:13:21 - | -LL | let _: Vec<_> = vec![5_i8; 6] - | _____________________^ -LL | | .into_iter() -LL | | .filter_map(|x| x.checked_mul(2)) -LL | | .flat_map(|x| x.checked_mul(2)) - | |_______________________________________^ - -error: called `filter_map(p).map(q)` on an `Iterator`. This is more succinctly expressed by only calling `.filter_map(..)` instead. - --> $DIR/filter_methods.rs:19:21 - | -LL | let _: Vec<_> = vec![5_i8; 6] - | _____________________^ -LL | | .into_iter() -LL | | .filter_map(|x| x.checked_mul(2)) -LL | | .map(|x| x.checked_mul(2)) - | |__________________________________^ - -error: aborting due to 4 previous errors - From 19d335b911a04c1ae568ab77f99f03b30e263bdc Mon Sep 17 00:00:00 2001 From: Wayne Warren Date: Sun, 23 Jun 2019 16:01:34 +0000 Subject: [PATCH 2/8] Fix lingering rebase breakages --- clippy_lints/src/methods/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 66202d8eb59c..521f80dd3f5e 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -13,6 +13,7 @@ use rustc::hir::intravisit::{self, Visitor}; use rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass}; use rustc::ty::{self, Predicate, Ty}; use rustc::{declare_lint_pass, declare_tool_lint}; +use crate::rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; use syntax::ast; use syntax::source_map::{BytePos, Span}; @@ -2052,10 +2053,10 @@ fn lint_filter_map<'a, 'tcx>( if_chain! { if match_trait_method(cx, expr, &paths::ITERATOR); if let hir::ExprKind::Closure(_, _, ref map_body_id, _, _) = map_args[1].node; - if let hir::ExprKind::MethodCall(ref map_ps, _, _) = cx.tcx.hir.body(*map_body_id).value.node; - if map_ps.ident.name == "unwrap".to_string(); + if let hir::ExprKind::MethodCall(ref map_ps, _, _) = cx.tcx.hir().body(*map_body_id).value.node; + if map_ps.ident.name == sym!(unwrap); if let hir::ExprKind::Closure(_, _, ref filter_body_id, _, _) = filter_args[1].node; - if let hir::ExprKind::MethodCall(ref filter_ps, _, _) = cx.tcx.hir.body(*filter_body_id).value.node; + if let hir::ExprKind::MethodCall(ref filter_ps, _, _) = cx.tcx.hir().body(*filter_body_id).value.node; if responses.contains_key(&filter_ps.ident.name.to_string()); if let hir::ExprKind::MethodCall(_, _, ref expr_vec) = expr.node; if let hir::ExprKind::MethodCall(_, ref filter_span, _) = expr_vec[0].node; From 8c6470846014a828e63b3e0668b845ff7891a6e5 Mon Sep 17 00:00:00 2001 From: Wayne Warren Date: Sun, 23 Jun 2019 16:03:52 +0000 Subject: [PATCH 3/8] Recreate tests/ui/filter_methods.stderr after rebase --- tests/ui/filter_methods.stderr | 58 ++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/ui/filter_methods.stderr b/tests/ui/filter_methods.stderr index e69de29bb2d1..559132201c99 100644 --- a/tests/ui/filter_methods.stderr +++ b/tests/ui/filter_methods.stderr @@ -0,0 +1,58 @@ +error: redundant closure found + --> $DIR/filter_methods.rs:19:14 + | +LL | .map(|x| x.unwrap()) + | ^^^^^^^^^^^^^^ help: remove closure as shown: `std::option::Option::unwrap` + | + = note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings` + +error: called `filter(p).map(q)` on an `Iterator>`. Consider calling `.filter_map(..)` instead. + --> $DIR/filter_methods.rs:18:10 + | +LL | .filter(|x| x.is_some()) + | __________^ +LL | | .map(|x| x.unwrap()) + | |____________________________^ + | + = note: `-D clippy::filter-map` implied by `-D warnings` + +error: redundant closure found + --> $DIR/filter_methods.rs:18:17 + | +LL | .filter(|x| x.is_some()) + | ^^^^^^^^^^^^^^^ help: remove closure as shown: `std::option::Option::is_some` + +error: redundant closure found + --> $DIR/filter_methods.rs:27:19 + | +LL | .flat_map(|x| x.unwrap()) + | ^^^^^^^^^^^^^^ help: remove closure as shown: `std::option::Option::unwrap` + +error: redundant closure found + --> $DIR/filter_methods.rs:26:17 + | +LL | .filter(|x| x.is_some()) + | ^^^^^^^^^^^^^^^ help: remove closure as shown: `std::option::Option::is_some` + +error: redundant closure found + --> $DIR/filter_methods.rs:34:14 + | +LL | .map(|s| s.unwrap()) + | ^^^^^^^^^^^^^^ help: remove closure as shown: `std::result::Result::unwrap` + +error: called `filter(p).map(q)` on an `Iterator>`. Consider calling `.filter_map(..)` instead. + --> $DIR/filter_methods.rs:33:10 + | +LL | .filter(|s| s.is_ok()) + | __________^ +LL | | .map(|s| s.unwrap()) + | |____________________________^ + +error: redundant closure found + --> $DIR/filter_methods.rs:33:17 + | +LL | .filter(|s| s.is_ok()) + | ^^^^^^^^^^^^^ help: remove closure as shown: `std::result::Result::is_ok` + +error: aborting due to 8 previous errors + From eeab7dbc2f9cb3347a007807c60f224f8a7fc444 Mon Sep 17 00:00:00 2001 From: Wayne Warren Date: Sun, 23 Jun 2019 16:05:23 +0000 Subject: [PATCH 4/8] Re-run 'util/dev update-lints' --- CHANGELOG.md | 2 +- README.md | 2 +- clippy_lints/src/lib.rs | 1 + src/lintlist/mod.rs | 11 +++++++++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 219a23b08bc5..8a06eba8adc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -944,8 +944,8 @@ Released 2018-09-13 [`extra_unused_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes [`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from [`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map -[`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next [`filter_map_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_map +[`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next [`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next [`find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#find_map [`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic diff --git a/README.md b/README.md index 51f618da4b78..c1a25aa13006 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 305 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 306 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b56413fc2c44..6585b6fbb6dd 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -633,6 +633,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { loops::EXPLICIT_ITER_LOOP, matches::SINGLE_MATCH_ELSE, methods::FILTER_MAP, + methods::FILTER_MAP_MAP, methods::FILTER_MAP_NEXT, methods::FIND_MAP, methods::MAP_FLATTEN, diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 663b6a5e7092..d52f036d1e39 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 305] = [ +pub const ALL_LINTS: [Lint; 306] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -528,7 +528,14 @@ pub const ALL_LINTS: [Lint; 305] = [ Lint { name: "filter_map", group: "pedantic", - desc: "using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call", + desc: "using `filter` and `map` to destructure an iterator of Options or a Results instead of a single `filter_map` call", + deprecation: None, + module: "methods", + }, + Lint { + name: "filter_map_map", + group: "pedantic", + desc: "using `filter_map` followed by `map` can more concisely be written as a single method call", deprecation: None, module: "methods", }, From f96eea7451aae2ce5162afffb329b6d7d9a7977a Mon Sep 17 00:00:00 2001 From: wayne Date: Mon, 24 Jun 2019 20:46:58 -0500 Subject: [PATCH 5/8] Update clippy_lints/src/methods/mod.rs Co-Authored-By: Philipp Krones --- clippy_lints/src/methods/mod.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 521f80dd3f5e..37f96a93ab11 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -280,7 +280,32 @@ declare_clippy_lint! { /// single method call. /// /// ** Example:** + /// ```rust /// let ws: Vec = ["is", "this", "shizzle", "for", "rizzle"] + /// .iter() + /// .map(|s| s.to_string()) + /// .collect(); + /// + /// let correct: Vec = ws + /// .iter() + /// .filter_map(|s| if s.contains("izzle") { Some(s) } else { None }) + /// .map(|s| format!("{}{}", s, s)) + /// .collect(); + /// + /// let more_correct: Vec = ws + /// .iter() + /// .filter_map(|s| { + /// if s.contains("izzle") { + /// Some(format!("{}{}", s, s)) + /// } else { + /// None + /// } + /// }) + /// .collect(); + /// + /// println!("{:?}", correct); + /// assert_eq!(correct, more_correct); + /// ``` /// .iter() /// .map(|s| s.to_string()) /// .collect(); From 9101623a36c4f62b16d03008add0557c83f42d80 Mon Sep 17 00:00:00 2001 From: Wayne Warren Date: Tue, 25 Jun 2019 02:09:17 +0000 Subject: [PATCH 6/8] Update filter_methods stderr --- tests/ui/filter_methods.stderr | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/ui/filter_methods.stderr b/tests/ui/filter_methods.stderr index 559132201c99..745541e386ee 100644 --- a/tests/ui/filter_methods.stderr +++ b/tests/ui/filter_methods.stderr @@ -1,3 +1,13 @@ +error: called `filter_map(p).map(q)` on an `Iterator`. This is more succinctly expressed by only calling `.filter_map(..)` instead. + --> $DIR/filter_methods.rs:12:10 + | +LL | .filter_map(|x| x.checked_mul(2)) + | __________^ +LL | | .map(|x| x.checked_mul(2)) + | |__________________________________^ + | + = note: `-D clippy::filter-map-map` implied by `-D warnings` + error: redundant closure found --> $DIR/filter_methods.rs:19:14 | @@ -54,5 +64,5 @@ error: redundant closure found LL | .filter(|s| s.is_ok()) | ^^^^^^^^^^^^^ help: remove closure as shown: `std::result::Result::is_ok` -error: aborting due to 8 previous errors +error: aborting due to 9 previous errors From 9b45aecf877015d94917475481f9463880e80347 Mon Sep 17 00:00:00 2001 From: Wayne Warren Date: Tue, 25 Jun 2019 02:10:01 +0000 Subject: [PATCH 7/8] Restore test to validate filter_map + flat_map doesn't trigger a lint --- tests/ui/filter_methods.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/ui/filter_methods.rs b/tests/ui/filter_methods.rs index 0ca1f08a3f30..690cb4bd94e3 100644 --- a/tests/ui/filter_methods.rs +++ b/tests/ui/filter_methods.rs @@ -45,4 +45,11 @@ fn main() { .filter(|i| i.len() > 1) .flat_map(|s| s.chars()) .collect(); + + // validate filter_map + flat_map **does not** trigger linter + let _: Vec<_> = vec![5_i8; 6] + .into_iter() + .filter_map(|x| x.checked_mul(2)) + .flat_map(|x| x.checked_mul(2)) + .collect(); } From ee9c13d1b9199787689cac94c7ee5774add349b7 Mon Sep 17 00:00:00 2001 From: Wayne Warren Date: Tue, 25 Jun 2019 02:12:58 +0000 Subject: [PATCH 8/8] Fix docstring for FILTER_MAP lint --- clippy_lints/src/methods/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 37f96a93ab11..82993c3bd535 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -335,8 +335,8 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for usage of `_.filter(_).map(_)`, - /// `_.filter(_).flat_map(_)`, `_.filter_map(_).flat_map(_)` and similar. + /// **What it does:** Checks for usage of `_.filter(_).map(_)` to destructure an iterator of + /// Options or Results instead of a single `_.filter_map(_)` call. /// /// **Why is this bad?** Readability, this can be written more concisely as a /// single method call. @@ -350,7 +350,7 @@ declare_clippy_lint! { /// ``` pub FILTER_MAP, pedantic, - "using `filter` and `map` to destructure an iterator of Options or a Results instead of a single `filter_map` call" + "using `filter` and `map` to destructure an iterator of Options or Results instead of a single `filter_map` call" } declare_clippy_lint! {