From 1fd617d6df6055516b5cc4b265037e4188806d1d Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Mon, 24 Jun 2019 11:21:29 +0200 Subject: [PATCH 01/14] Add test for unnecessary_flat_map --- tests/ui/unnecessary_flat_map.rs | 6 ++++++ tests/ui/unnecessary_flat_map.stderr | 10 ++++++++++ 2 files changed, 16 insertions(+) create mode 100644 tests/ui/unnecessary_flat_map.rs create mode 100644 tests/ui/unnecessary_flat_map.stderr diff --git a/tests/ui/unnecessary_flat_map.rs b/tests/ui/unnecessary_flat_map.rs new file mode 100644 index 00000000000..d0072eca9d2 --- /dev/null +++ b/tests/ui/unnecessary_flat_map.rs @@ -0,0 +1,6 @@ +#![warn(clippy::flat_map)] + +fn main() { + let iterator = [[0, 1], [2, 3], [4, 5]].iter(); + iterator.flat_map(|x| x); +} diff --git a/tests/ui/unnecessary_flat_map.stderr b/tests/ui/unnecessary_flat_map.stderr new file mode 100644 index 00000000000..9ebef07f1b7 --- /dev/null +++ b/tests/ui/unnecessary_flat_map.stderr @@ -0,0 +1,10 @@ +error: called `flat_map(|x| x)` on an `Iterator`. This can be simplified by calling `flatten().` + --> $DIR/unnecessary_flat_map.rs:5:5 + | +LL | iterator.flat_map(|x| x); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::flat-map` implied by `-D warnings` + +error: aborting due to previous error + From c7da4c26fbff41c07fe03927847f3fe233d8b5ad Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Mon, 24 Jun 2019 15:08:26 +0200 Subject: [PATCH 02/14] Implement flat_map lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 1 + clippy_lints/src/methods/mod.rs | 46 +++++++++++++++++++++++++++++++++ src/lintlist/mod.rs | 7 +++++ 4 files changed, 55 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 461adb729f9..b907b7f0e79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -947,6 +947,7 @@ Released 2018-09-13 [`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 +[`flat_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#flat_map [`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic [`float_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp [`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0d2c9e6b987..5548bb0ab62 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -637,6 +637,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { methods::FILTER_MAP, methods::FILTER_MAP_NEXT, methods::FIND_MAP, + methods::FLAT_MAP, methods::MAP_FLATTEN, methods::OPTION_MAP_UNWRAP_OR, methods::OPTION_MAP_UNWRAP_OR_ELSE, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f689a2d4ef0..1578976ed85 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -312,6 +312,26 @@ declare_clippy_lint! { "using combination of `filter_map` and `next` which can usually be written as a single method call" } +declare_clippy_lint! { + /// **What it does:** Checks for usage of `flat_map(|x| x)`. + /// + /// **Why is this bad?** Readability, this can be written more concisely by using `flatten`. + /// + /// **Known problems:** None + /// + /// **Example:** + /// ```rust + /// iter.flat_map(|x| x) + /// ``` + /// Can be written as + /// ```rust + /// iter.flatten() + /// ``` + pub FLAT_MAP, + pedantic, + "call to `flat_map` where `flatten` is sufficient" +} + declare_clippy_lint! { /// **What it does:** Checks for usage of `_.find(_).map(_)`. /// @@ -844,6 +864,7 @@ declare_lint_pass!(Methods => [ FILTER_NEXT, FILTER_MAP, FILTER_MAP_NEXT, + FLAT_MAP, FIND_MAP, MAP_FLATTEN, ITER_NTH, @@ -884,6 +905,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { ["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]), + ["flat_map", ..] => lint_flat_map(cx, expr, 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]), @@ -2092,6 +2114,30 @@ fn lint_filter_map_flat_map<'a, 'tcx>( } } +/// lint use of `flat_map` for `Iterators` where `flatten` would be sufficient +fn lint_flat_map<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, flat_map_args: &'tcx [hir::Expr]) { + if_chain! { + if match_trait_method(cx, expr, &paths::ITERATOR); + + if flat_map_args.len() == 2; + if let hir::ExprKind::Closure(_, _, body_id, _, _) = flat_map_args[1].node; + let body = cx.tcx.hir().body(body_id); + + if body.arguments.len() == 1; + if let hir::PatKind::Binding(_, _, binding_ident, _) = body.arguments[0].pat.node; + if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.node; + + if path.segments.len() == 1; + if path.segments[0].ident.as_str() == binding_ident.as_str(); + + then { + let msg = "called `flat_map(|x| x)` on an `Iterator`. \ + This can be simplified by calling `flatten().`"; + span_lint(cx, FLAT_MAP, expr.span, msg); + } + } +} + /// lint searching an Iterator followed by `is_some()` fn lint_search_is_some<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 8c49a3b1838..a7727258b53 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -553,6 +553,13 @@ pub const ALL_LINTS: [Lint; 306] = [ deprecation: None, module: "methods", }, + Lint { + name: "flat_map", + group: "pedantic", + desc: "call to `flat_map` where `flatten` is sufficient", + deprecation: None, + module: "methods", + }, Lint { name: "float_arithmetic", group: "restriction", From f0ce04f81432047bdd9b3a96a57ba06264f18471 Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Sun, 11 Aug 2019 19:51:43 +0200 Subject: [PATCH 03/14] Handle calls with 'std::convert::identity' --- clippy_lints/src/methods/mod.rs | 18 ++++++++++++++++++ clippy_lints/src/utils/paths.rs | 1 + src/lintlist/mod.rs | 2 +- tests/ui/unnecessary_flat_map.rs | 5 +++++ tests/ui/unnecessary_flat_map.stderr | 10 ++++++++-- 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index e51e432749c..58b33524589 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2186,6 +2186,24 @@ fn lint_flat_map<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, fl span_lint(cx, FLAT_MAP, expr.span, msg); } } + + if_chain! { + if match_trait_method(cx, expr, &paths::ITERATOR); + + if flat_map_args.len() == 2; + + let expr = &flat_map_args[1]; + + if let hir::ExprKind::Path(ref qpath) = expr.node; + + if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY); + + then { + let msg = "called `flat_map(std::convert::identity)` on an `Iterator`. \ + This can be simplified by calling `flatten().`"; + span_lint(cx, FLAT_MAP, expr.span, msg); + } + } } /// lint searching an Iterator followed by `is_some()` diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 62b22afff95..be811da0217 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -95,6 +95,7 @@ pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "", "into_vec pub const SLICE_ITER: [&str; 3] = ["core", "slice", "Iter"]; pub const STDERR: [&str; 4] = ["std", "io", "stdio", "stderr"]; pub const STDOUT: [&str; 4] = ["std", "io", "stdio", "stdout"]; +pub const STD_CONVERT_IDENTITY: [&str; 3] = ["std", "convert", "identity"]; pub const STD_MEM_TRANSMUTE: [&str; 3] = ["std", "mem", "transmute"]; pub const STD_PTR_NULL: [&str; 3] = ["std", "ptr", "null"]; pub const STRING: [&str; 3] = ["alloc", "string", "String"]; diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 23cad950a95..08179ec34e8 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; 309] = [ +pub const ALL_LINTS: [Lint; 310] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", diff --git a/tests/ui/unnecessary_flat_map.rs b/tests/ui/unnecessary_flat_map.rs index d0072eca9d2..b61569d1b93 100644 --- a/tests/ui/unnecessary_flat_map.rs +++ b/tests/ui/unnecessary_flat_map.rs @@ -1,6 +1,11 @@ #![warn(clippy::flat_map)] +use std::convert; + fn main() { let iterator = [[0, 1], [2, 3], [4, 5]].iter(); iterator.flat_map(|x| x); + + let iterator = [[0, 1], [2, 3], [4, 5]].iter(); + iterator.flat_map(convert::identity); } diff --git a/tests/ui/unnecessary_flat_map.stderr b/tests/ui/unnecessary_flat_map.stderr index 9ebef07f1b7..c98b403d29c 100644 --- a/tests/ui/unnecessary_flat_map.stderr +++ b/tests/ui/unnecessary_flat_map.stderr @@ -1,10 +1,16 @@ error: called `flat_map(|x| x)` on an `Iterator`. This can be simplified by calling `flatten().` - --> $DIR/unnecessary_flat_map.rs:5:5 + --> $DIR/unnecessary_flat_map.rs:7:5 | LL | iterator.flat_map(|x| x); | ^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::flat-map` implied by `-D warnings` -error: aborting due to previous error +error: called `flat_map(std::convert::identity)` on an `Iterator`. This can be simplified by calling `flatten().` + --> $DIR/unnecessary_flat_map.rs:10:23 + | +LL | iterator.flat_map(convert::identity); + | ^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors From b651f19eb87db789f8fd24a7791add446d869630 Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Sun, 11 Aug 2019 20:34:25 +0200 Subject: [PATCH 04/14] =?UTF-8?q?Rename=20'flat=5Fmap'=20=E2=86=92=20'flat?= =?UTF-8?q?=5Fmap=5Fidentity'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 2 +- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/methods/mod.rs | 16 ++++++++++------ src/lintlist/mod.rs | 2 +- tests/ui/unnecessary_flat_map.rs | 2 +- tests/ui/unnecessary_flat_map.stderr | 2 +- 6 files changed, 15 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89570267a94..87dbe891ff3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -947,7 +947,7 @@ Released 2018-09-13 [`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 -[`flat_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#flat_map +[`flat_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#flat_map_identity [`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic [`float_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp [`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4c9126045f9..4cc750ceb2f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -643,7 +643,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { methods::FILTER_MAP, methods::FILTER_MAP_NEXT, methods::FIND_MAP, - methods::FLAT_MAP, + methods::FLAT_MAP_IDENTITY, methods::MAP_FLATTEN, methods::OPTION_MAP_UNWRAP_OR, methods::OPTION_MAP_UNWRAP_OR_ELSE, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 58b33524589..0ab7785501a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -357,7 +357,7 @@ declare_clippy_lint! { /// ```rust /// iter.flatten() /// ``` - pub FLAT_MAP, + pub FLAT_MAP_IDENTITY, pedantic, "call to `flat_map` where `flatten` is sufficient" } @@ -912,7 +912,7 @@ declare_lint_pass!(Methods => [ FILTER_NEXT, FILTER_MAP, FILTER_MAP_NEXT, - FLAT_MAP, + FLAT_MAP_IDENTITY, FIND_MAP, MAP_FLATTEN, ITER_NTH, @@ -953,7 +953,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { ["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]), - ["flat_map", ..] => lint_flat_map(cx, expr, arg_lists[0]), + ["flat_map", ..] => lint_flat_map_identity(cx, expr, 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]), @@ -2165,7 +2165,11 @@ fn lint_filter_map_flat_map<'a, 'tcx>( } /// lint use of `flat_map` for `Iterators` where `flatten` would be sufficient -fn lint_flat_map<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, flat_map_args: &'tcx [hir::Expr]) { +fn lint_flat_map_identity<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + expr: &'tcx hir::Expr, + flat_map_args: &'tcx [hir::Expr], +) { if_chain! { if match_trait_method(cx, expr, &paths::ITERATOR); @@ -2183,7 +2187,7 @@ fn lint_flat_map<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, fl then { let msg = "called `flat_map(|x| x)` on an `Iterator`. \ This can be simplified by calling `flatten().`"; - span_lint(cx, FLAT_MAP, expr.span, msg); + span_lint(cx, FLAT_MAP_IDENTITY, expr.span, msg); } } @@ -2201,7 +2205,7 @@ fn lint_flat_map<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, fl then { let msg = "called `flat_map(std::convert::identity)` on an `Iterator`. \ This can be simplified by calling `flatten().`"; - span_lint(cx, FLAT_MAP, expr.span, msg); + span_lint(cx, FLAT_MAP_IDENTITY, expr.span, msg); } } } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 08179ec34e8..cfe8dea8837 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -554,7 +554,7 @@ pub const ALL_LINTS: [Lint; 310] = [ module: "methods", }, Lint { - name: "flat_map", + name: "flat_map_identity", group: "pedantic", desc: "call to `flat_map` where `flatten` is sufficient", deprecation: None, diff --git a/tests/ui/unnecessary_flat_map.rs b/tests/ui/unnecessary_flat_map.rs index b61569d1b93..955e791dd2b 100644 --- a/tests/ui/unnecessary_flat_map.rs +++ b/tests/ui/unnecessary_flat_map.rs @@ -1,4 +1,4 @@ -#![warn(clippy::flat_map)] +#![warn(clippy::flat_map_identity)] use std::convert; diff --git a/tests/ui/unnecessary_flat_map.stderr b/tests/ui/unnecessary_flat_map.stderr index c98b403d29c..4872e37f324 100644 --- a/tests/ui/unnecessary_flat_map.stderr +++ b/tests/ui/unnecessary_flat_map.stderr @@ -4,7 +4,7 @@ error: called `flat_map(|x| x)` on an `Iterator`. This can be simplified by call LL | iterator.flat_map(|x| x); | ^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `-D clippy::flat-map` implied by `-D warnings` + = note: `-D clippy::flat-map-identity` implied by `-D warnings` error: called `flat_map(std::convert::identity)` on an `Iterator`. This can be simplified by calling `flatten().` --> $DIR/unnecessary_flat_map.rs:10:23 From 5fd7d44f36ae530c4cb1ccfda08f028918e53ea6 Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Sun, 11 Aug 2019 21:02:01 +0200 Subject: [PATCH 05/14] Refactor if_chain Co-authored-by: Philipp Krones --- clippy_lints/src/methods/mod.rs | 49 +++++++++++++++++---------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 0ab7785501a..dc5c579b7e5 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2174,38 +2174,39 @@ fn lint_flat_map_identity<'a, 'tcx>( if match_trait_method(cx, expr, &paths::ITERATOR); if flat_map_args.len() == 2; - if let hir::ExprKind::Closure(_, _, body_id, _, _) = flat_map_args[1].node; - let body = cx.tcx.hir().body(body_id); - - if body.arguments.len() == 1; - if let hir::PatKind::Binding(_, _, binding_ident, _) = body.arguments[0].pat.node; - if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.node; - - if path.segments.len() == 1; - if path.segments[0].ident.as_str() == binding_ident.as_str(); then { - let msg = "called `flat_map(|x| x)` on an `Iterator`. \ - This can be simplified by calling `flatten().`"; - span_lint(cx, FLAT_MAP_IDENTITY, expr.span, msg); - } - } + if_chain! { + if let hir::ExprKind::Closure(_, _, body_id, _, _) = flat_map_args[1].node; + let body = cx.tcx.hir().body(body_id); - if_chain! { - if match_trait_method(cx, expr, &paths::ITERATOR); + if body.arguments.len() == 1; + if let hir::PatKind::Binding(_, _, binding_ident, _) = body.arguments[0].pat.node; + if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.node; - if flat_map_args.len() == 2; + if path.segments.len() == 1; + if path.segments[0].ident.as_str() == binding_ident.as_str(); - let expr = &flat_map_args[1]; + then { + let msg = "called `flat_map(|x| x)` on an `Iterator`. \ + This can be simplified by calling `flatten().`"; + span_lint(cx, FLAT_MAP_IDENTITY, expr.span, msg); + } + } - if let hir::ExprKind::Path(ref qpath) = expr.node; + if_chain! { + let expr = &flat_map_args[1]; - if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY); + if let hir::ExprKind::Path(ref qpath) = expr.node; - then { - let msg = "called `flat_map(std::convert::identity)` on an `Iterator`. \ - This can be simplified by calling `flatten().`"; - span_lint(cx, FLAT_MAP_IDENTITY, expr.span, msg); + if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY); + + then { + let msg = "called `flat_map(std::convert::identity)` on an `Iterator`. \ + This can be simplified by calling `flatten().`"; + span_lint(cx, FLAT_MAP_IDENTITY, expr.span, msg); + } + } } } } From df9063013e94051aac6f20826ed9dd44294d390e Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Sun, 11 Aug 2019 22:18:58 +0200 Subject: [PATCH 06/14] Update rustdoc --- clippy_lints/src/methods/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index dc5c579b7e5..41b92b05246 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -351,11 +351,13 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust - /// iter.flat_map(|x| x) + /// # let iter = vec![vec![0]].into_iter(); + /// iter.flat_map(|x| x); /// ``` /// Can be written as /// ```rust - /// iter.flatten() + /// # let iter = vec![vec![0]].into_iter(); + /// iter.flatten(); /// ``` pub FLAT_MAP_IDENTITY, pedantic, From 6ddf8c36d774e22133f68aef162a66d1685ba42c Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Mon, 12 Aug 2019 07:53:22 +0200 Subject: [PATCH 07/14] Run 'update_lints' --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8bcfd8a8430..389fe316ade 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 309 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 310 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: From 09e9568c266a17ec398242cb85b454bdaf5f5e4c Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Mon, 12 Aug 2019 10:52:09 +0200 Subject: [PATCH 08/14] =?UTF-8?q?Change=20lint=20type=20from=20'pedantic'?= =?UTF-8?q?=20=E2=86=92=20'complexity'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 41b92b05246..2311d0df764 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -360,7 +360,7 @@ declare_clippy_lint! { /// iter.flatten(); /// ``` pub FLAT_MAP_IDENTITY, - pedantic, + complexity, "call to `flat_map` where `flatten` is sufficient" } From 3a65e4e75a067e0ff207057766171a48961bb8f5 Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Mon, 12 Aug 2019 20:35:50 +0200 Subject: [PATCH 09/14] Minor refactoring --- clippy_lints/src/methods/mod.rs | 49 +++++++++++++++------------------ 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 2311d0df764..431053846f7 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2172,42 +2172,37 @@ fn lint_flat_map_identity<'a, 'tcx>( expr: &'tcx hir::Expr, flat_map_args: &'tcx [hir::Expr], ) { - if_chain! { - if match_trait_method(cx, expr, &paths::ITERATOR); + if match_trait_method(cx, expr, &paths::ITERATOR) { + let arg_node = &flat_map_args[1].node; - if flat_map_args.len() == 2; + let apply_lint = |message: &str| { + span_lint(cx, FLAT_MAP_IDENTITY, expr.span, message); + }; - then { - if_chain! { - if let hir::ExprKind::Closure(_, _, body_id, _, _) = flat_map_args[1].node; - let body = cx.tcx.hir().body(body_id); + if_chain! { + if let hir::ExprKind::Closure(_, _, body_id, _, _) = arg_node; + let body = cx.tcx.hir().body(*body_id); - if body.arguments.len() == 1; - if let hir::PatKind::Binding(_, _, binding_ident, _) = body.arguments[0].pat.node; - if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.node; + if let hir::PatKind::Binding(_, _, binding_ident, _) = body.arguments[0].pat.node; + if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.node; - if path.segments.len() == 1; - if path.segments[0].ident.as_str() == binding_ident.as_str(); + if path.segments.len() == 1; + if path.segments[0].ident.as_str() == binding_ident.as_str(); - then { - let msg = "called `flat_map(|x| x)` on an `Iterator`. \ - This can be simplified by calling `flatten().`"; - span_lint(cx, FLAT_MAP_IDENTITY, expr.span, msg); - } + then { + apply_lint("called `flat_map(|x| x)` on an `Iterator`. \ + This can be simplified by calling `flatten().`"); } + } - if_chain! { - let expr = &flat_map_args[1]; + if_chain! { + if let hir::ExprKind::Path(ref qpath) = arg_node; - if let hir::ExprKind::Path(ref qpath) = expr.node; + if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY); - if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY); - - then { - let msg = "called `flat_map(std::convert::identity)` on an `Iterator`. \ - This can be simplified by calling `flatten().`"; - span_lint(cx, FLAT_MAP_IDENTITY, expr.span, msg); - } + then { + apply_lint("called `flat_map(std::convert::identity)` on an `Iterator`. \ + This can be simplified by calling `flatten().`"); } } } From d51136d594b5eba73b11c6caafbd38ee2c8adec9 Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Mon, 12 Aug 2019 21:42:57 +0200 Subject: [PATCH 10/14] Use 'span_lint_and_sugg' --- clippy_lints/src/methods/mod.rs | 18 +++++++++++++----- tests/ui/unnecessary_flat_map.rs | 2 ++ tests/ui/unnecessary_flat_map.stderr | 12 ++++++------ 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 431053846f7..b69534a7703 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2176,7 +2176,17 @@ fn lint_flat_map_identity<'a, 'tcx>( let arg_node = &flat_map_args[1].node; let apply_lint = |message: &str| { - span_lint(cx, FLAT_MAP_IDENTITY, expr.span, message); + if let hir::ExprKind::MethodCall(_, span, _) = &expr.node { + span_lint_and_sugg( + cx, + FLAT_MAP_IDENTITY, + *span, + message, + "try", + "flatten()".to_string(), + Applicability::MachineApplicable, + ); + } }; if_chain! { @@ -2190,8 +2200,7 @@ fn lint_flat_map_identity<'a, 'tcx>( if path.segments[0].ident.as_str() == binding_ident.as_str(); then { - apply_lint("called `flat_map(|x| x)` on an `Iterator`. \ - This can be simplified by calling `flatten().`"); + apply_lint("called `flat_map(|x| x)` on an `Iterator`"); } } @@ -2201,8 +2210,7 @@ fn lint_flat_map_identity<'a, 'tcx>( if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY); then { - apply_lint("called `flat_map(std::convert::identity)` on an `Iterator`. \ - This can be simplified by calling `flatten().`"); + apply_lint("called `flat_map(std::convert::identity)` on an `Iterator`"); } } } diff --git a/tests/ui/unnecessary_flat_map.rs b/tests/ui/unnecessary_flat_map.rs index 955e791dd2b..32eaef4754f 100644 --- a/tests/ui/unnecessary_flat_map.rs +++ b/tests/ui/unnecessary_flat_map.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::flat_map_identity)] use std::convert; diff --git a/tests/ui/unnecessary_flat_map.stderr b/tests/ui/unnecessary_flat_map.stderr index 4872e37f324..968a1e01026 100644 --- a/tests/ui/unnecessary_flat_map.stderr +++ b/tests/ui/unnecessary_flat_map.stderr @@ -1,16 +1,16 @@ -error: called `flat_map(|x| x)` on an `Iterator`. This can be simplified by calling `flatten().` - --> $DIR/unnecessary_flat_map.rs:7:5 +error: called `flat_map(|x| x)` on an `Iterator` + --> $DIR/unnecessary_flat_map.rs:9:14 | LL | iterator.flat_map(|x| x); - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ help: try: `flatten()` | = note: `-D clippy::flat-map-identity` implied by `-D warnings` -error: called `flat_map(std::convert::identity)` on an `Iterator`. This can be simplified by calling `flatten().` - --> $DIR/unnecessary_flat_map.rs:10:23 +error: called `flat_map(std::convert::identity)` on an `Iterator` + --> $DIR/unnecessary_flat_map.rs:12:14 | LL | iterator.flat_map(convert::identity); - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` error: aborting due to 2 previous errors From 4275d7b6acc9757de008e61c2dd3d55a9ca113e4 Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Mon, 12 Aug 2019 21:47:12 +0200 Subject: [PATCH 11/14] Run 'update_lints' --- clippy_lints/src/lib.rs | 3 ++- src/lintlist/mod.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4cc750ceb2f..38fa6c9b5bc 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -643,7 +643,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { methods::FILTER_MAP, methods::FILTER_MAP_NEXT, methods::FIND_MAP, - methods::FLAT_MAP_IDENTITY, methods::MAP_FLATTEN, methods::OPTION_MAP_UNWRAP_OR, methods::OPTION_MAP_UNWRAP_OR_ELSE, @@ -778,6 +777,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { methods::CLONE_ON_COPY, methods::EXPECT_FUN_CALL, methods::FILTER_NEXT, + methods::FLAT_MAP_IDENTITY, methods::INTO_ITER_ON_ARRAY, methods::INTO_ITER_ON_REF, methods::ITER_CLONED_COLLECT, @@ -1022,6 +1022,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { methods::CHARS_NEXT_CMP, methods::CLONE_ON_COPY, methods::FILTER_NEXT, + methods::FLAT_MAP_IDENTITY, methods::SEARCH_IS_SOME, methods::UNNECESSARY_FILTER_MAP, methods::USELESS_ASREF, diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index cfe8dea8837..63d1be8fe0d 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -555,7 +555,7 @@ pub const ALL_LINTS: [Lint; 310] = [ }, Lint { name: "flat_map_identity", - group: "pedantic", + group: "complexity", desc: "call to `flat_map` where `flatten` is sufficient", deprecation: None, module: "methods", From 6a263c0816fa01acd2b49dde5d99adfd195566e4 Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Tue, 13 Aug 2019 08:02:59 +0200 Subject: [PATCH 12/14] Add 'unnecessary_flat_map.fixed' --- tests/ui/unnecessary_flat_map.fixed | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 tests/ui/unnecessary_flat_map.fixed diff --git a/tests/ui/unnecessary_flat_map.fixed b/tests/ui/unnecessary_flat_map.fixed new file mode 100644 index 00000000000..a5f24a4aa18 --- /dev/null +++ b/tests/ui/unnecessary_flat_map.fixed @@ -0,0 +1,13 @@ +// run-rustfix + +#![warn(clippy::flat_map_identity)] + +use std::convert; + +fn main() { + let iterator = [[0, 1], [2, 3], [4, 5]].iter(); + iterator.flatten(); + + let iterator = [[0, 1], [2, 3], [4, 5]].iter(); + iterator.flatten(); +} From d578c43c51823952923f0698a3f2283cb3fcca0d Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Tue, 13 Aug 2019 21:50:42 +0200 Subject: [PATCH 13/14] Use correct span --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index b69534a7703..7f95cb475e2 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2180,7 +2180,7 @@ fn lint_flat_map_identity<'a, 'tcx>( span_lint_and_sugg( cx, FLAT_MAP_IDENTITY, - *span, + span.with_hi(expr.span.hi()), message, "try", "flatten()".to_string(), From 2fe5e2cea91030737f38df81e5f82e652927491f Mon Sep 17 00:00:00 2001 From: Jeremy Stucki Date: Tue, 13 Aug 2019 21:50:52 +0200 Subject: [PATCH 14/14] Update test --- tests/ui/unnecessary_flat_map.fixed | 5 +++-- tests/ui/unnecessary_flat_map.rs | 5 +++-- tests/ui/unnecessary_flat_map.stderr | 12 ++++++------ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/ui/unnecessary_flat_map.fixed b/tests/ui/unnecessary_flat_map.fixed index a5f24a4aa18..dfe3bd47e13 100644 --- a/tests/ui/unnecessary_flat_map.fixed +++ b/tests/ui/unnecessary_flat_map.fixed @@ -1,13 +1,14 @@ // run-rustfix +#![allow(unused_imports)] #![warn(clippy::flat_map_identity)] use std::convert; fn main() { let iterator = [[0, 1], [2, 3], [4, 5]].iter(); - iterator.flatten(); + let _ = iterator.flatten(); let iterator = [[0, 1], [2, 3], [4, 5]].iter(); - iterator.flatten(); + let _ = iterator.flatten(); } diff --git a/tests/ui/unnecessary_flat_map.rs b/tests/ui/unnecessary_flat_map.rs index 32eaef4754f..393b9569255 100644 --- a/tests/ui/unnecessary_flat_map.rs +++ b/tests/ui/unnecessary_flat_map.rs @@ -1,13 +1,14 @@ // run-rustfix +#![allow(unused_imports)] #![warn(clippy::flat_map_identity)] use std::convert; fn main() { let iterator = [[0, 1], [2, 3], [4, 5]].iter(); - iterator.flat_map(|x| x); + let _ = iterator.flat_map(|x| x); let iterator = [[0, 1], [2, 3], [4, 5]].iter(); - iterator.flat_map(convert::identity); + let _ = iterator.flat_map(convert::identity); } diff --git a/tests/ui/unnecessary_flat_map.stderr b/tests/ui/unnecessary_flat_map.stderr index 968a1e01026..a1cd5745e49 100644 --- a/tests/ui/unnecessary_flat_map.stderr +++ b/tests/ui/unnecessary_flat_map.stderr @@ -1,16 +1,16 @@ error: called `flat_map(|x| x)` on an `Iterator` - --> $DIR/unnecessary_flat_map.rs:9:14 + --> $DIR/unnecessary_flat_map.rs:10:22 | -LL | iterator.flat_map(|x| x); - | ^^^^^^^^^^^^^^^ help: try: `flatten()` +LL | let _ = iterator.flat_map(|x| x); + | ^^^^^^^^^^^^^^^ help: try: `flatten()` | = note: `-D clippy::flat-map-identity` implied by `-D warnings` error: called `flat_map(std::convert::identity)` on an `Iterator` - --> $DIR/unnecessary_flat_map.rs:12:14 + --> $DIR/unnecessary_flat_map.rs:13:22 | -LL | iterator.flat_map(convert::identity); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` +LL | let _ = iterator.flat_map(convert::identity); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` error: aborting due to 2 previous errors