Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint map_all_any_identity #13499

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5689,6 +5689,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
3 changes: 1 addition & 2 deletions clippy_lints/src/casts/unnecessary_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,7 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx
if !snippet
.split("->")
.skip(1)
.map(|s| snippet_eq_ty(s, cast_from) || s.split("where").any(|ty| snippet_eq_ty(ty, cast_from)))
.any(|a| a)
.any(|s| snippet_eq_ty(s, cast_from) || s.split("where").any(|ty| snippet_eq_ty(ty, cast_from)))
{
return ControlFlow::Break(());
}
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 @@ -416,6 +416,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,
);
},
);
}
}
74 changes: 55 additions & 19 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 @@ -4167,29 +4168,54 @@ declare_clippy_lint! {
"calling `.first().is_some()` or `.first().is_none()` instead of `.is_empty()`"
}

declare_clippy_lint! {
/// ### What it does
/// It detects useless calls to `str::as_bytes()` before calling `len()` or `is_empty()`.
///
/// ### Why is this bad?
/// The `len()` and `is_empty()` methods are also directly available on strings, and they
/// return identical results. In particular, `len()` on a string returns the number of
/// bytes.
///
/// ### Example
/// ```
/// let len = "some string".as_bytes().len();
/// let b = "some string".as_bytes().is_empty();
/// ```
/// Use instead:
/// ```
/// let len = "some string".len();
/// let b = "some string".is_empty();
/// ```
#[clippy::version = "1.84.0"]
pub NEEDLESS_AS_BYTES,
complexity,
"detect useless calls to `as_bytes()`"
}

declare_clippy_lint! {
/// ### What it does
/// It detects useless calls to `str::as_bytes()` before calling `len()` or `is_empty()`.
/// Checks for usage of `.map(…)`, followed by `.all(identity)` or `.any(identity)`.
///
/// ### Why is this bad?
/// The `len()` and `is_empty()` methods are also directly available on strings, and they
/// return identical results. In particular, `len()` on a string returns the number of
/// bytes.
/// The `.all(…)` or `.any(…)` methods can be called directly in place of `.map(…)`.
///
/// ### Example
/// ```
/// let len = "some string".as_bytes().len();
/// let b = "some string".as_bytes().is_empty();
/// # 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 len = "some string".len();
/// let b = "some string".is_empty();
/// # 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 NEEDLESS_AS_BYTES,
pub MAP_ALL_ANY_IDENTITY,
complexity,
"detect useless calls to `as_bytes()`"
"combine `.map(_)` followed by `.all(identity)`/`.any(identity)` into a single call"
}

pub struct Methods {
Expand Down Expand Up @@ -4354,6 +4380,7 @@ impl_lint_pass!(Methods => [
MANUAL_INSPECT,
UNNECESSARY_MIN_OR_MAX,
NEEDLESS_AS_BYTES,
MAP_ALL_ANY_IDENTITY,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4561,15 +4588,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 @@ -4598,6 +4631,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