Auto merge of #6577 - nahuakang:inspect_then_for_each, r=flip1995

New Lint: inspect_then_for_each

**Work In Progress**

This PR addresses [Issue 5209](#5209) and adds a new lint called `inspect_then_for_each`.

Current seek some guidance.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints

---

changelog: Add [`inspect_for_each`] lint for the use of `inspect().for_each()` on `Iterators`.

CHANGELOG.md

@@ -1997,6 +1997,7 @@ Released 2018-09-13
 [`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax
 [`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax
 [`inline_fn_without_body`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_fn_without_body
+[`inspect_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#inspect_for_each
 [`int_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#int_plus_one
 [`integer_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_arithmetic
 [`integer_division`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_division

clippy_lints/src/lib.rs

@@ -736,6 +736,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::FROM_ITER_INSTEAD_OF_COLLECT,
         &methods::GET_UNWRAP,
         &methods::INEFFICIENT_TO_STRING,
+        &methods::INSPECT_FOR_EACH,
         &methods::INTO_ITER_ON_REF,
         &methods::ITERATOR_STEP_BY_ZERO,
         &methods::ITER_CLONED_COLLECT,
@@ -1514,6 +1515,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::FILTER_NEXT),
         LintId::of(&methods::FLAT_MAP_IDENTITY),
         LintId::of(&methods::FROM_ITER_INSTEAD_OF_COLLECT),
+        LintId::of(&methods::INSPECT_FOR_EACH),
         LintId::of(&methods::INTO_ITER_ON_REF),
         LintId::of(&methods::ITERATOR_STEP_BY_ZERO),
         LintId::of(&methods::ITER_CLONED_COLLECT),
@@ -1815,6 +1817,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::CLONE_ON_COPY),
         LintId::of(&methods::FILTER_NEXT),
         LintId::of(&methods::FLAT_MAP_IDENTITY),
+        LintId::of(&methods::INSPECT_FOR_EACH),
         LintId::of(&methods::OPTION_AS_REF_DEREF),
         LintId::of(&methods::SEARCH_IS_SOME),
         LintId::of(&methods::SKIP_WHILE_NEXT),

clippy_lints/src/methods/inspect_for_each.rs

@@ -0,0 +1,23 @@
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+use rustc_span::source_map::Span;
+
+use crate::utils::{match_trait_method, paths, span_lint_and_help};
+
+use super::INSPECT_FOR_EACH;
+
+/// lint use of `inspect().for_each()` for `Iterators`
+pub(super) fn lint<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, inspect_span: Span) {
+    if match_trait_method(cx, expr, &paths::ITERATOR) {
+        let msg = "called `inspect(..).for_each(..)` on an `Iterator`";
+        let hint = "move the code from `inspect(..)` to `for_each(..)` and remove the `inspect(..)`";
+        span_lint_and_help(
+            cx,
+            INSPECT_FOR_EACH,
+            inspect_span.with_hi(expr.span.hi()),
+            msg,
+            None,
+            hint,
+        );
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -1,5 +1,6 @@
 mod bind_instead_of_map;
 mod inefficient_to_string;
+mod inspect_for_each;
 mod manual_saturating_arithmetic;
 mod option_map_unwrap_or;
 mod unnecessary_filter_map;
@@ -1405,6 +1406,36 @@ declare_clippy_lint! {
     "use `.collect()` instead of `::from_iter()`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `inspect().for_each()`.
+    ///
+    /// **Why is this bad?** It is the same as performing the computation
+    /// inside `inspect` at the beginning of the closure in `for_each`.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// [1,2,3,4,5].iter()
+    /// .inspect(|&x| println!("inspect the number: {}", x))
+    /// .for_each(|&x| {
+    ///     assert!(x >= 0);
+    /// });
+    /// ```
+    /// Can be written as
+    /// ```rust
+    /// [1,2,3,4,5].iter()
+    /// .for_each(|&x| {
+    ///     println!("inspect the number: {}", x);
+    ///     assert!(x >= 0);
+    /// });
+    /// ```
+    pub INSPECT_FOR_EACH,
+    complexity,
+    "using `.inspect().for_each()`, which can be replaced with `.for_each()`"
+}
+
 pub struct Methods {
     msrv: Option<RustcVersion>,
 }
@@ -1467,6 +1498,7 @@ impl_lint_pass!(Methods => [
     UNNECESSARY_LAZY_EVALUATIONS,
     MAP_COLLECT_RESULT_UNIT,
     FROM_ITER_INSTEAD_OF_COLLECT,
+    INSPECT_FOR_EACH,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Methods {
@@ -1553,6 +1585,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             ["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]),
+            ["for_each", "inspect"] => inspect_for_each::lint(cx, expr, method_spans[1]),
             _ => {},
         }
 

tests/ui/inspect_for_each.rs

@@ -0,0 +1,22 @@
+#![warn(clippy::inspect_for_each)]
+
+fn main() {
+    let a: Vec<usize> = vec![1, 2, 3, 4, 5];
+
+    let mut b: Vec<usize> = Vec::new();
+    a.into_iter().inspect(|x| assert!(*x > 0)).for_each(|x| {
+        let y = do_some(x);
+        let z = do_more(y);
+        b.push(z);
+    });
+
+    assert_eq!(b, vec![4, 5, 6, 7, 8]);
+}
+
+fn do_some(a: usize) -> usize {
+    a + 1
+}
+
+fn do_more(a: usize) -> usize {
+    a + 2
+}

tests/ui/inspect_for_each.stderr

@@ -0,0 +1,16 @@
+error: called `inspect(..).for_each(..)` on an `Iterator`
+  --> $DIR/inspect_for_each.rs:7:19
+   |
+LL |       a.into_iter().inspect(|x| assert!(*x > 0)).for_each(|x| {
+   |  ___________________^
+LL | |         let y = do_some(x);
+LL | |         let z = do_more(y);
+LL | |         b.push(z);
+LL | |     });
+   | |______^
+   |
+   = note: `-D clippy::inspect-for-each` implied by `-D warnings`
+   = help: move the code from `inspect(..)` to `for_each(..)` and remove the `inspect(..)`
+
+error: aborting due to previous error
+