diff --git a/CHANGELOG.md b/CHANGELOG.md index db838a3e2e8d..87dbe891ff38 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_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/README.md b/README.md index 8bcfd8a8430c..389fe316ade2 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: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1ab943c5923b..38fa6c9b5bcd 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -777,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, @@ -1021,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/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index fc78355c399f..39ac112481e4 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -342,6 +342,28 @@ 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 + /// # let iter = vec![vec![0]].into_iter(); + /// iter.flat_map(|x| x); + /// ``` + /// Can be written as + /// ```rust + /// # let iter = vec![vec![0]].into_iter(); + /// iter.flatten(); + /// ``` + pub FLAT_MAP_IDENTITY, + complexity, + "call to `flat_map` where `flatten` is sufficient" +} + declare_clippy_lint! { /// **What it does:** Checks for usage of `_.find(_).map(_)`. /// @@ -892,6 +914,7 @@ declare_lint_pass!(Methods => [ FILTER_NEXT, FILTER_MAP, FILTER_MAP_NEXT, + FLAT_MAP_IDENTITY, FIND_MAP, MAP_FLATTEN, ITER_NTH, @@ -932,6 +955,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_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]), @@ -2143,6 +2167,56 @@ fn lint_filter_map_flat_map<'a, 'tcx>( } } +/// lint use of `flat_map` for `Iterators` where `flatten` would be sufficient +fn lint_flat_map_identity<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + expr: &'tcx hir::Expr, + flat_map_args: &'tcx [hir::Expr], +) { + if match_trait_method(cx, expr, &paths::ITERATOR) { + let arg_node = &flat_map_args[1].node; + + let apply_lint = |message: &str| { + if let hir::ExprKind::MethodCall(_, span, _) = &expr.node { + span_lint_and_sugg( + cx, + FLAT_MAP_IDENTITY, + span.with_hi(expr.span.hi()), + message, + "try", + "flatten()".to_string(), + Applicability::MachineApplicable, + ); + } + }; + + if_chain! { + if let hir::ExprKind::Closure(_, _, body_id, _, _) = arg_node; + let body = cx.tcx.hir().body(*body_id); + + 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 { + apply_lint("called `flat_map(|x| x)` on an `Iterator`"); + } + } + + if_chain! { + if let hir::ExprKind::Path(ref qpath) = arg_node; + + if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY); + + then { + apply_lint("called `flat_map(std::convert::identity)` on an `Iterator`"); + } + } + } +} + /// lint searching an Iterator followed by `is_some()` fn lint_search_is_some<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 62b22afff95b..be811da02179 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 b41ed4a5910e..63d1be8fe0df 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", @@ -553,6 +553,13 @@ pub const ALL_LINTS: [Lint; 309] = [ deprecation: None, module: "methods", }, + Lint { + name: "flat_map_identity", + group: "complexity", + desc: "call to `flat_map` where `flatten` is sufficient", + deprecation: None, + module: "methods", + }, Lint { name: "float_arithmetic", group: "restriction", diff --git a/tests/ui/unnecessary_flat_map.fixed b/tests/ui/unnecessary_flat_map.fixed new file mode 100644 index 000000000000..dfe3bd47e139 --- /dev/null +++ b/tests/ui/unnecessary_flat_map.fixed @@ -0,0 +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(); + let _ = iterator.flatten(); + + let iterator = [[0, 1], [2, 3], [4, 5]].iter(); + let _ = iterator.flatten(); +} diff --git a/tests/ui/unnecessary_flat_map.rs b/tests/ui/unnecessary_flat_map.rs new file mode 100644 index 000000000000..393b95692554 --- /dev/null +++ b/tests/ui/unnecessary_flat_map.rs @@ -0,0 +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(); + let _ = iterator.flat_map(|x| x); + + let iterator = [[0, 1], [2, 3], [4, 5]].iter(); + let _ = iterator.flat_map(convert::identity); +} diff --git a/tests/ui/unnecessary_flat_map.stderr b/tests/ui/unnecessary_flat_map.stderr new file mode 100644 index 000000000000..a1cd5745e494 --- /dev/null +++ b/tests/ui/unnecessary_flat_map.stderr @@ -0,0 +1,16 @@ +error: called `flat_map(|x| x)` on an `Iterator` + --> $DIR/unnecessary_flat_map.rs:10:22 + | +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:13:22 + | +LL | let _ = iterator.flat_map(convert::identity); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` + +error: aborting due to 2 previous errors +