Add new trivial_map_over_range lint

This lint checks for code that looks like
```rust
  let something : Vec<_> = (0..100).map(|_| {
    1 + 2 + 3
  }).collect();
```
which is more clear as
```rust
  let something : Vec<_> = std::iter::repeat_with(|| {
    1 + 2 + 3
  }).take(100).collect();
```

That is, a map over a range which does nothing with the parameter
passed to it is simply a function (or closure) being called `n`
times and could be more semantically expressed using `take`.

CHANGELOG.md

@@ -5885,6 +5885,7 @@ Released 2018-09-13
 [`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts
 [`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null
 [`trim_split_whitespace`]: https://rust-lang.github.io/rust-clippy/master/index.html#trim_split_whitespace
+[`trivial_map_over_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_map_over_range
 [`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
 [`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
 [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err

clippy_lints/src/declared_lints.rs

@@ -706,6 +706,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::transmute::UNSOUND_COLLECTION_TRANSMUTE_INFO,
     crate::transmute::USELESS_TRANSMUTE_INFO,
     crate::transmute::WRONG_TRANSMUTE_INFO,
+    crate::trivial_map_over_range::TRIVIAL_MAP_OVER_RANGE_INFO,
     crate::tuple_array_conversions::TUPLE_ARRAY_CONVERSIONS_INFO,
     crate::types::BORROWED_BOX_INFO,
     crate::types::BOX_COLLECTION_INFO,

clippy_lints/src/lib.rs

@@ -346,6 +346,7 @@ mod to_string_trait_impl;
 mod trailing_empty_array;
 mod trait_bounds;
 mod transmute;
+mod trivial_map_over_range;
 mod tuple_array_conversions;
 mod types;
 mod unconditional_recursion;
@@ -1174,6 +1175,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(msrv())));
     store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers));
     store.register_late_pass(|_| Box::new(set_contains_or_insert::HashsetInsertAfterContains));
+    store.register_late_pass(|_| Box::new(trivial_map_over_range::TrivialMapOverRange));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/trivial_map_over_range.rs

@@ -0,0 +1,83 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_ast::LitKind;
+use rustc_data_structures::packed::Pu128;
+use rustc_errors::Applicability;
+use rustc_hir::{Body, Closure, Expr, ExprKind, LangItem, PatKind, QPath};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Lint to prevent trivial `map`s over ranges when one could use `take`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// It expreses the intent more clearly to `take` the correct number of times
+    /// from a generating function than to apply a closure to each number in a
+    /// range only to discard them.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let random_numbers : Vec<_> = (0..10).map(|_| { 3 + 1 }).collect();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let f : Vec<_> = std::iter::repeat_with(|| { 3 + 1 }).take(10).collect();
+    /// ```
+    #[clippy::version = "1.81.0"]
+    pub TRIVIAL_MAP_OVER_RANGE,
+    style,
+    "map of a trivial closure (not dependent on parameter) over a range"
+}
+
+declare_lint_pass!(TrivialMapOverRange => [TRIVIAL_MAP_OVER_RANGE]);
+
+impl LateLintPass<'_> for TrivialMapOverRange {
+    fn check_expr(&mut self, cx: &LateContext<'_>, ex: &Expr<'_>) {
+        if let ExprKind::MethodCall(path, receiver, [map_arg_expr], _call_span) = ex.kind
+            && path.ident.name == rustc_span::sym::map
+            && let ExprKind::Struct(qpath, fields, _) = receiver.kind
+            && matches!(qpath, QPath::LangItem(LangItem::Range, _))
+            && fields.len() == 2
+            && let ExprKind::Closure(Closure { body, .. }) = map_arg_expr.kind
+            && let Body { params: [param], .. } = cx.tcx.hir().body(*body)
+            && matches!(param.pat.kind, PatKind::Wild)
+            && let ExprKind::Lit(lit) = fields[0].expr.kind
+            && let LitKind::Int(Pu128(lower_bound), _) = lit.node
+        {
+            if let ExprKind::Lit(lit) = fields[1].expr.kind
+                && let LitKind::Int(Pu128(upper_bound), _) = lit.node
+            {
+                let count = upper_bound - lower_bound;
+                let mut applicability = Applicability::MaybeIncorrect;
+                let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
+                    .replacen("|_|", "||", 1);
+                span_lint_and_sugg(
+                    cx,
+                    TRIVIAL_MAP_OVER_RANGE,
+                    ex.span,
+                    "map of a trivial closure (not dependent on parameter) over a range",
+                    "use",
+                    format!("std::iter::repeat_with({snippet}).take({count})"),
+                    applicability,
+                );
+            } else if lower_bound == 0 {
+                let mut applicability = Applicability::MaybeIncorrect;
+                let count = snippet_with_applicability(cx, fields[1].expr.span, "...", &mut applicability);
+                let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
+                    .replacen("|_|", "||", 1);
+                span_lint_and_sugg(
+                    cx,
+                    TRIVIAL_MAP_OVER_RANGE,
+                    ex.span,
+                    "map of a trivial closure (not dependent on parameter) over a range",
+                    "use",
+                    format!("std::iter::repeat_with({snippet}).take({count})"),
+                    applicability,
+                );
+            }
+        }
+    }
+}

tests/ui/repeat_vec_with_capacity.fixed

@@ -1,3 +1,4 @@
+#![allow(clippy::trivial_map_over_range)]
 #![warn(clippy::repeat_vec_with_capacity)]
 
 fn main() {

tests/ui/repeat_vec_with_capacity.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::trivial_map_over_range)]
 #![warn(clippy::repeat_vec_with_capacity)]
 
 fn main() {

tests/ui/repeat_vec_with_capacity.stderr

@@ -1,5 +1,5 @@
 error: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
-  --> tests/ui/repeat_vec_with_capacity.rs:5:9
+  --> tests/ui/repeat_vec_with_capacity.rs:6:9
    |
 LL |         vec![Vec::<()>::with_capacity(42); 123];
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -13,7 +13,7 @@ LL |         (0..123).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
    |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
-  --> tests/ui/repeat_vec_with_capacity.rs:11:9
+  --> tests/ui/repeat_vec_with_capacity.rs:12:9
    |
 LL |         vec![Vec::<()>::with_capacity(42); n];
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -25,7 +25,7 @@ LL |         (0..n).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
    |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: repeating `Vec::with_capacity` using `iter::repeat`, which does not retain capacity
-  --> tests/ui/repeat_vec_with_capacity.rs:26:9
+  --> tests/ui/repeat_vec_with_capacity.rs:27:9
    |
 LL |         std::iter::repeat(Vec::<()>::with_capacity(42));
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

tests/ui/trivial_map_over_range.fixed

@@ -0,0 +1,32 @@
+#![allow(unused, clippy::redundant_closure)]
+#![warn(clippy::trivial_map_over_range)]
+
+fn do_something() -> usize {
+    todo!()
+}
+
+fn do_something_interesting(x: usize, y: usize) -> usize {
+    todo!()
+}
+
+fn main() {
+    // These should be raised
+    std::iter::repeat_with(|| do_something()).take(10);
+    std::iter::repeat_with(|| do_something()).take(7);
+    std::iter::repeat_with(|| 3).take(10);
+    std::iter::repeat_with(|| {
+        let x = 3;
+        x + 2
+    }).take(10);
+    std::iter::repeat_with(|| do_something()).take(10).collect::<Vec<_>>();
+    let upper = 4;
+    std::iter::repeat_with(|| do_something()).take(upper);
+    let upper_fn = || 4;
+    std::iter::repeat_with(|| do_something()).take(upper_fn());
+    // These should not be raised
+    (0..=10).map(|_| do_something()); // Inclusive bounds not yet handled
+    (0..10).map(|x| do_something()); // We do not detect unused parameters
+    (0..10).map(|x| do_something()).collect::<Vec<_>>(); // We do not detect unused parameters
+    (0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range
+    "Foobar".chars().map(|_| do_something()); // Not a map over range
+}

tests/ui/trivial_map_over_range.rs

@@ -0,0 +1,32 @@
+#![allow(unused, clippy::redundant_closure)]
+#![warn(clippy::trivial_map_over_range)]
+
+fn do_something() -> usize {
+    todo!()
+}
+
+fn do_something_interesting(x: usize, y: usize) -> usize {
+    todo!()
+}
+
+fn main() {
+    // These should be raised
+    (0..10).map(|_| do_something());
+    (3..10).map(|_| do_something());
+    (0..10).map(|_| 3);
+    (0..10).map(|_| {
+        let x = 3;
+        x + 2
+    });
+    (0..10).map(|_| do_something()).collect::<Vec<_>>();
+    let upper = 4;
+    (0..upper).map(|_| do_something());
+    let upper_fn = || 4;
+    (0..upper_fn()).map(|_| do_something());
+    // These should not be raised
+    (0..=10).map(|_| do_something()); // Inclusive bounds not yet handled
+    (0..10).map(|x| do_something()); // We do not detect unused parameters
+    (0..10).map(|x| do_something()).collect::<Vec<_>>(); // We do not detect unused parameters
+    (0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range
+    "Foobar".chars().map(|_| do_something()); // Not a map over range
+}

tests/ui/trivial_map_over_range.stderr

@@ -0,0 +1,58 @@
+error: map of a trivial closure (not dependent on parameter) over a range
+  --> tests/ui/trivial_map_over_range.rs:14:5
+   |
+LL |     (0..10).map(|_| do_something());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
+   |
+   = note: `-D clippy::trivial-map-over-range` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::trivial_map_over_range)]`
+
+error: map of a trivial closure (not dependent on parameter) over a range
+  --> tests/ui/trivial_map_over_range.rs:15:5
+   |
+LL |     (3..10).map(|_| do_something());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(7)`
+
+error: map of a trivial closure (not dependent on parameter) over a range
+  --> tests/ui/trivial_map_over_range.rs:16:5
+   |
+LL |     (0..10).map(|_| 3);
+   |     ^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| 3).take(10)`
+
+error: map of a trivial closure (not dependent on parameter) over a range
+  --> tests/ui/trivial_map_over_range.rs:17:5
+   |
+LL | /     (0..10).map(|_| {
+LL | |         let x = 3;
+LL | |         x + 2
+LL | |     });
+   | |______^
+   |
+help: use
+   |
+LL ~     std::iter::repeat_with(|| {
+LL +         let x = 3;
+LL +         x + 2
+LL ~     }).take(10);
+   |
+
+error: map of a trivial closure (not dependent on parameter) over a range
+  --> tests/ui/trivial_map_over_range.rs:21:5
+   |
+LL |     (0..10).map(|_| do_something()).collect::<Vec<_>>();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
+
+error: map of a trivial closure (not dependent on parameter) over a range
+  --> tests/ui/trivial_map_over_range.rs:23:5
+   |
+LL |     (0..upper).map(|_| do_something());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper)`
+
+error: map of a trivial closure (not dependent on parameter) over a range
+  --> tests/ui/trivial_map_over_range.rs:25:5
+   |
+LL |     (0..upper_fn()).map(|_| do_something());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn())`
+
+error: aborting due to 7 previous errors
+