Auto merge of #4231 - jeremystucki:flat-map, r=flip1995

Implement flat_map lint

Fixes #4224

changelog: New Lint `flat_map_identity` to detect unnecessary calls to `flat_map`

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

@@ -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,

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>,

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

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();
+}

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);
+}

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
+