From 3269070261e8324c2b3074cd491ddf2ac6cf19d3 Mon Sep 17 00:00:00 2001 From: nahuakang Date: Mon, 11 Jan 2021 23:56:12 +0100 Subject: [PATCH] Create new lint for the usage of inspect for each. --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/methods/inspect_for_each.rs | 23 ++++++++++++++ clippy_lints/src/methods/mod.rs | 33 ++++++++++++++++++++ tests/ui/inspect_for_each.rs | 22 +++++++++++++ tests/ui/inspect_for_each.stderr | 16 ++++++++++ 6 files changed, 98 insertions(+) create mode 100644 clippy_lints/src/methods/inspect_for_each.rs create mode 100644 tests/ui/inspect_for_each.rs create mode 100644 tests/ui/inspect_for_each.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 64864c2e278..a91b7c64770 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1996,6 +1996,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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f12994c7a60..17662b81205 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -733,6 +733,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, @@ -1507,6 +1508,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), @@ -1807,6 +1809,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), diff --git a/clippy_lints/src/methods/inspect_for_each.rs b/clippy_lints/src/methods/inspect_for_each.rs new file mode 100644 index 00000000000..6d41ee38a27 --- /dev/null +++ b/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, + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f13f2491d6e..018696ef0cf 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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 @@ "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, } @@ -1467,6 +1498,7 @@ pub fn new(msrv: Option) -> Self { 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 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { ["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]), _ => {}, } diff --git a/tests/ui/inspect_for_each.rs b/tests/ui/inspect_for_each.rs new file mode 100644 index 00000000000..7fe45c83bca --- /dev/null +++ b/tests/ui/inspect_for_each.rs @@ -0,0 +1,22 @@ +#![warn(clippy::inspect_for_each)] + +fn main() { + let a: Vec = vec![1, 2, 3, 4, 5]; + + let mut b: Vec = 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 +} diff --git a/tests/ui/inspect_for_each.stderr b/tests/ui/inspect_for_each.stderr new file mode 100644 index 00000000000..9f976bb7458 --- /dev/null +++ b/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 +