Auto merge of #13499 - samueltardieu:map-all-any-identity, r=xFrednet

New lint `map_all_any_identity`

This lint has been inspired by code encountered in Clippy itself (see the first commit).

changelog: [`map_all_any_identity`]: new lint

CHANGELOG.md

@@ -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

clippy_lints/src/casts/unnecessary_cast.rs

@@ -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(());
                 }

clippy_lints/src/declared_lints.rs

@@ -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,

clippy_lints/src/methods/map_all_any_identity.rs

@@ -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,
+                );
+            },
+        );
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -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;
@@ -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 {
@@ -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.
@@ -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]) => {
@@ -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");
+                        },
                         _ => {},
                     }
                 },

tests/ui/map_all_any_identity.fixed

@@ -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);
+}

tests/ui/map_all_any_identity.rs

@@ -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);
+}

tests/ui/map_all_any_identity.stderr

@@ -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
+