Implement flat_map lint

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

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:
 

clippy_lints/src/lib.rs

@@ -643,6 +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_IDENTITY,
         methods::MAP_FLATTEN,
         methods::OPTION_MAP_UNWRAP_OR,
         methods::OPTION_MAP_UNWRAP_OR_ELSE,

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,
+    pedantic,
+    "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]),
@@ -2142,6 +2166,53 @@ 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_chain! {
+        if match_trait_method(cx, expr, &paths::ITERATOR);
+
+        if flat_map_args.len() == 2;
+
+        then {
+            if_chain! {
+                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! {
+                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_IDENTITY, expr.span, msg);
+                }
+            }
+        }
+    }
+}
+
 /// lint searching an Iterator followed by `is_some()`
 fn lint_search_is_some<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,

clippy_lints/src/utils/paths.rs

@@ -95,6 +95,7 @@ pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "<impl [T]>", "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"];

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: "pedantic",
+        desc: "call to `flat_map` where `flatten` is sufficient",
+        deprecation: None,
+        module: "methods",
+    },
     Lint {
         name: "float_arithmetic",
         group: "restriction",

tests/ui/unnecessary_flat_map.rs

@@ -0,0 +1,11 @@
+#![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 = [[0, 1], [2, 3], [4, 5]].iter();
+    iterator.flat_map(convert::identity);
+}

tests/ui/unnecessary_flat_map.stderr

@@ -0,0 +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
+   |
+LL |     iterator.flat_map(|x| x);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = 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
+   |
+LL |     iterator.flat_map(convert::identity);
+   |                       ^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+