Add lint for replacing .map().collect() with .try_for_each()

clippy_lints/src/lib.rs

@@ -697,6 +697,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::ITER_NTH_ZERO,
         &methods::ITER_SKIP_NEXT,
         &methods::MANUAL_SATURATING_ARITHMETIC,
+        &methods::MAP_COLLECT_RESULT_UNIT,
         &methods::MAP_FLATTEN,
         &methods::MAP_UNWRAP_OR,
         &methods::NEW_RET_NO_SELF,
@@ -1420,6 +1421,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::ITER_NTH_ZERO),
         LintId::of(&methods::ITER_SKIP_NEXT),
         LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
+        LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
         LintId::of(&methods::NEW_RET_NO_SELF),
         LintId::of(&methods::OK_EXPECT),
         LintId::of(&methods::OPTION_AS_REF_DEREF),
@@ -1615,6 +1617,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::ITER_NTH_ZERO),
         LintId::of(&methods::ITER_SKIP_NEXT),
         LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
+        LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
         LintId::of(&methods::NEW_RET_NO_SELF),
         LintId::of(&methods::OK_EXPECT),
         LintId::of(&methods::OPTION_MAP_OR_NONE),

clippy_lints/src/methods/mod.rs

@@ -1383,6 +1383,27 @@ declare_clippy_lint! {
     "using unnecessary lazy evaluation, which can be replaced with simpler eager evaluation"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `_.map(_).collect::<Result<(),_>()`.
+    ///
+    /// **Why is this bad?** Using `try_for_each` instead is more readable and idiomatic.
+    ///
+    /// **Known problems:** None
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// (0..3).map(|t| Err(t)).collect::<Result<(), _>>();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// (0..3).try_for_each(|t| Err(t));
+    /// ```
+    pub MAP_COLLECT_RESULT_UNIT,
+    style,
+    "using `.map(_).collect::<Result<(),_>()`, which can be replaced with `try_for_each`"
+}
+
 declare_lint_pass!(Methods => [
     UNWRAP_USED,
     EXPECT_USED,
@@ -1433,6 +1454,7 @@ declare_lint_pass!(Methods => [
     FILETYPE_IS_FILE,
     OPTION_AS_REF_DEREF,
     UNNECESSARY_LAZY_EVALUATIONS,
+    MAP_COLLECT_RESULT_UNIT,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Methods {
@@ -1515,6 +1537,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             ["unwrap_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "unwrap_or"),
             ["get_or_insert_with", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "get_or_insert"),
             ["ok_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "ok_or"),
+            ["collect", "map"] => lint_map_collect(cx, expr, arg_lists[1], arg_lists[0]),
             _ => {},
         }
 
@@ -3501,6 +3524,42 @@ fn lint_option_as_ref_deref<'tcx>(
     }
 }
 
+fn lint_map_collect(
+    cx: &LateContext<'_>,
+    expr: &hir::Expr<'_>,
+    map_args: &[hir::Expr<'_>],
+    collect_args: &[hir::Expr<'_>],
+) {
+    if_chain! {
+        // called on Iterator
+        if let [map_expr] = collect_args;
+        if match_trait_method(cx, map_expr, &paths::ITERATOR);
+        // return of collect `Result<(),_>`
+        let collect_ret_ty = cx.typeck_results().expr_ty(expr);
+        if is_type_diagnostic_item(cx, collect_ret_ty, sym!(result_type));
+        if let ty::Adt(_, substs) = collect_ret_ty.kind();
+        if let Some(result_t) = substs.types().next();
+        if result_t.is_unit();
+        // get parts for snippet
+        if let [iter, map_fn] = map_args;
+        then {
+            span_lint_and_sugg(
+                cx,
+                MAP_COLLECT_RESULT_UNIT,
+                expr.span,
+                "`.map().collect()` can be replaced with `.try_for_each()`",
+                "try this",
+                format!(
+                    "{}.try_for_each({})",
+                    snippet(cx, iter.span, ".."),
+                    snippet(cx, map_fn.span, "..")
+                ),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}
+
 /// Given a `Result<T, E>` type, return its error type (`E`).
 fn get_error_type<'a>(cx: &LateContext<'_>, ty: Ty<'a>) -> Option<Ty<'a>> {
     match ty.kind() {