Skip to content

Commit

Permalink
Add new lint: map_all_any_identity
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed Oct 13, 2024
1 parent cc51437 commit 334d2f4
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5653,6 +5653,7 @@ Released 2018-09-13
[`manual_unwrap_or_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or_default
[`manual_while_let_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_while_let_some
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
[`map_all_any_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_all_any_identity
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
[`map_collect_result_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_collect_result_unit
[`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::methods::MANUAL_SPLIT_ONCE_INFO,
crate::methods::MANUAL_STR_REPEAT_INFO,
crate::methods::MANUAL_TRY_FOLD_INFO,
crate::methods::MAP_ALL_ANY_IDENTITY_INFO,
crate::methods::MAP_CLONE_INFO,
crate::methods::MAP_COLLECT_RESULT_UNIT_INFO,
crate::methods::MAP_ERR_IGNORE_INFO,
Expand Down
43 changes: 43 additions & 0 deletions clippy_lints/src/methods/map_all_any_identity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::SpanRangeExt;
use clippy_utils::{is_expr_identity_function, is_trait_method};
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_span::{Span, sym};

use super::MAP_ALL_ANY_IDENTITY;

#[allow(clippy::too_many_arguments)]
pub(super) fn check(
cx: &LateContext<'_>,
expr: &Expr<'_>,
recv: &Expr<'_>,
map_call_span: Span,
map_arg: &Expr<'_>,
any_call_span: Span,
any_arg: &Expr<'_>,
method: &str,
) {
if is_trait_method(cx, expr, sym::Iterator)
&& is_trait_method(cx, recv, sym::Iterator)
&& is_expr_identity_function(cx, any_arg)
&& let map_any_call_span = map_call_span.with_hi(any_call_span.hi())
&& let Some(map_arg) = map_arg.span.get_source_text(cx)
{
span_lint_and_then(
cx,
MAP_ALL_ANY_IDENTITY,
map_any_call_span,
format!("usage of `.map(...).{method}(identity)`"),
|diag| {
diag.span_suggestion_verbose(
map_any_call_span,
format!("use `.{method}(...)` instead"),
format!("{method}({map_arg})"),
Applicability::MachineApplicable,
);
},
);
}
}
54 changes: 45 additions & 9 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ mod manual_ok_or;
mod manual_saturating_arithmetic;
mod manual_str_repeat;
mod manual_try_fold;
mod map_all_any_identity;
mod map_clone;
mod map_collect_result_unit;
mod map_err_ignore;
Expand Down Expand Up @@ -4166,6 +4167,31 @@ declare_clippy_lint! {
"calling `.first().is_some()` or `.first().is_none()` instead of `.is_empty()`"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.map(…)`, followed by `.all(identity)` or `.any(identity)`.
///
/// ### Why is this bad?
/// The `.all(…)` or `.any(…)` methods can be called directly in place of `.map(…)`.
///
/// ### Example
/// ```
/// # let mut v = [""];
/// let e1 = v.iter().map(|s| s.is_empty()).all(|a| a);
/// let e2 = v.iter().map(|s| s.is_empty()).any(std::convert::identity);
/// ```
/// Use instead:
/// ```
/// # let mut v = [""];
/// let e1 = v.iter().all(|s| s.is_empty());
/// let e2 = v.iter().any(|s| s.is_empty());
/// ```
#[clippy::version = "1.84.0"]
pub MAP_ALL_ANY_IDENTITY,
complexity,
"combine `.map(_)` followed by `.all(identity)`/`.any(identity)` into a single call"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -4327,6 +4353,7 @@ impl_lint_pass!(Methods => [
NEEDLESS_CHARACTER_ITERATION,
MANUAL_INSPECT,
UNNECESSARY_MIN_OR_MAX,
MAP_ALL_ANY_IDENTITY,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4534,15 +4561,21 @@ impl Methods {
("all", [arg]) => {
unused_enumerate_index::check(cx, expr, recv, arg);
needless_character_iteration::check(cx, expr, recv, arg, true);
if let Some(("cloned", recv2, [], _, _)) = method_call(recv) {
iter_overeager_cloned::check(
cx,
expr,
recv,
recv2,
iter_overeager_cloned::Op::NeedlessMove(arg),
false,
);
match method_call(recv) {
Some(("cloned", recv2, [], _, _)) => {
iter_overeager_cloned::check(
cx,
expr,
recv,
recv2,
iter_overeager_cloned::Op::NeedlessMove(arg),
false,
);
},
Some(("map", _, [map_arg], _, map_call_span)) => {
map_all_any_identity::check(cx, expr, recv, map_call_span, map_arg, call_span, arg, "all");
},
_ => {},
}
},
("and_then", [arg]) => {
Expand Down Expand Up @@ -4571,6 +4604,9 @@ impl Methods {
{
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
},
Some(("map", _, [map_arg], _, map_call_span)) => {
map_all_any_identity::check(cx, expr, recv, map_call_span, map_arg, call_span, arg, "any");
},
_ => {},
}
},
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/map_all_any_identity.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![warn(clippy::map_all_any_identity)]

fn main() {
let _ = ["foo"].into_iter().any(|s| s == "foo");
//~^ map_all_any_identity
let _ = ["foo"].into_iter().all(|s| s == "foo");
//~^ map_all_any_identity

//
// Do not lint
//
// Not identity
let _ = ["foo"].into_iter().map(|s| s.len()).any(|n| n > 0);
// Macro
macro_rules! map {
($x:expr) => {
$x.into_iter().map(|s| s == "foo")
};
}
map!(["foo"]).any(|a| a);
}
21 changes: 21 additions & 0 deletions tests/ui/map_all_any_identity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![warn(clippy::map_all_any_identity)]

fn main() {
let _ = ["foo"].into_iter().map(|s| s == "foo").any(|a| a);
//~^ map_all_any_identity
let _ = ["foo"].into_iter().map(|s| s == "foo").all(std::convert::identity);
//~^ map_all_any_identity

//
// Do not lint
//
// Not identity
let _ = ["foo"].into_iter().map(|s| s.len()).any(|n| n > 0);
// Macro
macro_rules! map {
($x:expr) => {
$x.into_iter().map(|s| s == "foo")
};
}
map!(["foo"]).any(|a| a);
}
26 changes: 26 additions & 0 deletions tests/ui/map_all_any_identity.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error: usage of `.map(...).any(identity)`
--> tests/ui/map_all_any_identity.rs:4:33
|
LL | let _ = ["foo"].into_iter().map(|s| s == "foo").any(|a| a);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::map-all-any-identity` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::map_all_any_identity)]`
help: use `.any(...)` instead
|
LL | let _ = ["foo"].into_iter().any(|s| s == "foo");
| ~~~~~~~~~~~~~~~~~~~

error: usage of `.map(...).all(identity)`
--> tests/ui/map_all_any_identity.rs:6:33
|
LL | let _ = ["foo"].into_iter().map(|s| s == "foo").all(std::convert::identity);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `.all(...)` instead
|
LL | let _ = ["foo"].into_iter().all(|s| s == "foo");
| ~~~~~~~~~~~~~~~~~~~

error: aborting due to 2 previous errors

0 comments on commit 334d2f4

Please sign in to comment.