Skip to content

Commit

Permalink
Create new lint for the usage of inspect for each.
Browse files Browse the repository at this point in the history
  • Loading branch information
nahuakang committed Jan 19, 2021
1 parent 583715f commit 3269070
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
23 changes: 23 additions & 0 deletions clippy_lints/src/methods/inspect_for_each.rs
Original file line number Diff line number Diff line change
@@ -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,
);
}
}
33 changes: 33 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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>,
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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]),
_ => {},
}

Expand Down
22 changes: 22 additions & 0 deletions tests/ui/inspect_for_each.rs
Original file line number Diff line number Diff line change
@@ -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
}
16 changes: 16 additions & 0 deletions tests/ui/inspect_for_each.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 3269070

Please sign in to comment.