Auto merge of #13034 - rspencer01:trivial_map_over_range, r=y21

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`.

- [x] Followed [lint naming conventions][lint_naming]
- [x] Added passing UI tests (including committed `.stderr` file)
- [x] `cargo test` passes locally
- [x] Executed `cargo dev update_lints`
- [x] Added lint documentation
- [x] Run `cargo dev fmt`

changelog: new lint: [`trivial_map_over_range`] `restriction`

Author: bors

CHANGELOG.md

@@ -5697,6 +5697,7 @@ Released 2018-09-13
 [`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
 [`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
 [`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or
+[`map_with_unused_argument_over_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_with_unused_argument_over_ranges
 [`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref
 [`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool
 [`match_like_matches_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro

book/src/lint_configuration.md

@@ -710,6 +710,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`manual_try_fold`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold)
 * [`map_clone`](https://rust-lang.github.io/rust-clippy/master/index.html#map_clone)
 * [`map_unwrap_or`](https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or)
+* [`map_with_unused_argument_over_ranges`](https://rust-lang.github.io/rust-clippy/master/index.html#map_with_unused_argument_over_ranges)
 * [`match_like_matches_macro`](https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro)
 * [`mem_replace_with_default`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default)
 * [`missing_const_for_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn)

clippy_config/src/conf.rs

@@ -573,6 +573,7 @@ define_Conf! {
         manual_try_fold,
         map_clone,
         map_unwrap_or,
+        map_with_unused_argument_over_ranges,
         match_like_matches_macro,
         mem_replace_with_default,
         missing_const_for_fn,

clippy_config/src/msrvs.rs

@@ -19,7 +19,7 @@ macro_rules! msrv_aliases {
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
     1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY }
-    1,82,0 { IS_NONE_OR }
+    1,82,0 { IS_NONE_OR, REPEAT_N }
     1,81,0 { LINT_REASONS_STABILIZATION }
     1,80,0 { BOX_INTO_ITER}
     1,77,0 { C_STR_LITERALS }
@@ -55,7 +55,7 @@ msrv_aliases! {
     1,33,0 { UNDERSCORE_IMPORTS }
     1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
     1,29,0 { ITER_FLATTEN }
-    1,28,0 { FROM_BOOL }
+    1,28,0 { FROM_BOOL, REPEAT_WITH }
     1,27,0 { ITERATOR_TRY_FOLD }
     1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN }
     1,24,0 { IS_ASCII_DIGIT }

clippy_lints/src/declared_lints.rs

@@ -423,6 +423,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::MAP_FLATTEN_INFO,
     crate::methods::MAP_IDENTITY_INFO,
     crate::methods::MAP_UNWRAP_OR_INFO,
+    crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES_INFO,
     crate::methods::MUT_MUTEX_LOCK_INFO,
     crate::methods::NAIVE_BYTECOUNT_INFO,
     crate::methods::NEEDLESS_AS_BYTES_INFO,

clippy_lints/src/matches/single_match.rs

@@ -249,7 +249,10 @@ impl<'a> PatState<'a> {
         let states = match self {
             Self::Wild => return None,
             Self::Other => {
-                *self = Self::StdEnum(cx.arena.alloc_from_iter((0..adt.variants().len()).map(|_| Self::Other)));
+                *self = Self::StdEnum(
+                    cx.arena
+                        .alloc_from_iter(std::iter::repeat_with(|| Self::Other).take(adt.variants().len())),
+                );
                 let Self::StdEnum(x) = self else {
                     unreachable!();
                 };

clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs

@@ -0,0 +1,134 @@
+use crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES;
+use clippy_config::msrvs::{self, Msrv};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{eager_or_lazy, higher, usage};
+use rustc_ast::LitKind;
+use rustc_ast::ast::RangeLimits;
+use rustc_data_structures::packed::Pu128;
+use rustc_errors::Applicability;
+use rustc_hir::{Body, Closure, Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::Span;
+
+fn extract_count_with_applicability(
+    cx: &LateContext<'_>,
+    range: higher::Range<'_>,
+    applicability: &mut Applicability,
+) -> Option<String> {
+    let start = range.start?;
+    let end = range.end?;
+    // TODO: This doens't handle if either the start or end are negative literals, or if the start is
+    // not a literal. In the first case, we need to be careful about how we handle computing the
+    // count to avoid overflows. In the second, we may need to add parenthesis to make the
+    // suggestion correct.
+    if let ExprKind::Lit(lit) = start.kind
+        && let LitKind::Int(Pu128(lower_bound), _) = lit.node
+    {
+        if let ExprKind::Lit(lit) = end.kind
+            && let LitKind::Int(Pu128(upper_bound), _) = lit.node
+        {
+            // Here we can explicitly calculate the number of iterations
+            let count = if upper_bound >= lower_bound {
+                match range.limits {
+                    RangeLimits::HalfOpen => upper_bound - lower_bound,
+                    RangeLimits::Closed => (upper_bound - lower_bound).checked_add(1)?,
+                }
+            } else {
+                0
+            };
+            return Some(format!("{count}"));
+        }
+        let end_snippet = Sugg::hir_with_applicability(cx, end, "...", applicability)
+            .maybe_par()
+            .into_string();
+        if lower_bound == 0 {
+            if range.limits == RangeLimits::Closed {
+                return Some(format!("{end_snippet} + 1"));
+            }
+            return Some(end_snippet);
+        }
+        if range.limits == RangeLimits::Closed {
+            return Some(format!("{end_snippet} - {}", lower_bound - 1));
+        }
+        return Some(format!("{end_snippet} - {lower_bound}"));
+    }
+    None
+}
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    ex: &Expr<'_>,
+    receiver: &Expr<'_>,
+    arg: &Expr<'_>,
+    msrv: &Msrv,
+    method_call_span: Span,
+) {
+    let mut applicability = Applicability::MaybeIncorrect;
+    if let Some(range) = higher::Range::hir(receiver)
+        && let ExprKind::Closure(Closure { body, .. }) = arg.kind
+        && let body_hir = cx.tcx.hir().body(*body)
+        && let Body {
+            params: [param],
+            value: body_expr,
+        } = body_hir
+        && !usage::BindingUsageFinder::are_params_used(cx, body_hir)
+        && let Some(count) = extract_count_with_applicability(cx, range, &mut applicability)
+    {
+        let method_to_use_name;
+        let new_span;
+        let use_take;
+
+        if eager_or_lazy::switch_to_eager_eval(cx, body_expr) {
+            if msrv.meets(msrvs::REPEAT_N) {
+                method_to_use_name = "repeat_n";
+                let body_snippet = snippet_with_applicability(cx, body_expr.span, "..", &mut applicability);
+                new_span = (arg.span, format!("{body_snippet}, {count}"));
+                use_take = false;
+            } else {
+                method_to_use_name = "repeat";
+                let body_snippet = snippet_with_applicability(cx, body_expr.span, "..", &mut applicability);
+                new_span = (arg.span, body_snippet.to_string());
+                use_take = true;
+            }
+        } else if msrv.meets(msrvs::REPEAT_WITH) {
+            method_to_use_name = "repeat_with";
+            new_span = (param.span, String::new());
+            use_take = true;
+        } else {
+            return;
+        }
+
+        // We need to provide nonempty parts to diag.multipart_suggestion so we
+        // collate all our parts here and then remove those that are empty.
+        let mut parts = vec![
+            (
+                receiver.span.to(method_call_span),
+                format!("std::iter::{method_to_use_name}"),
+            ),
+            new_span,
+        ];
+        if use_take {
+            parts.push((ex.span.shrink_to_hi(), format!(".take({count})")));
+        }
+
+        span_lint_and_then(
+            cx,
+            MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
+            ex.span,
+            "map of a closure that does not depend on its parameter over a range",
+            |diag| {
+                diag.multipart_suggestion(
+                    if use_take {
+                        format!("remove the explicit range and use `{method_to_use_name}` and `take`")
+                    } else {
+                        format!("remove the explicit range and use `{method_to_use_name}`")
+                    },
+                    parts,
+                    applicability,
+                );
+            },
+        );
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -67,6 +67,7 @@ mod map_err_ignore;
 mod map_flatten;
 mod map_identity;
 mod map_unwrap_or;
+mod map_with_unused_argument_over_ranges;
 mod mut_mutex_lock;
 mod needless_as_bytes;
 mod needless_character_iteration;
@@ -4218,6 +4219,40 @@ declare_clippy_lint! {
     "combine `.map(_)` followed by `.all(identity)`/`.any(identity)` into a single call"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for `Iterator::map` over ranges without using the parameter which
+    /// could be more clearly expressed using `std::iter::repeat(...).take(...)`
+    /// or `std::iter::repeat_n`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// It expresses 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( 3 + 1 ).take(10).collect();
+    /// ```
+    ///
+    /// ### Known Issues
+    ///
+    /// This lint may suggest replacing a `Map<Range>` with a `Take<RepeatWith>`.
+    /// The former implements some traits that the latter does not, such as
+    /// `DoubleEndedIterator`.
+    #[clippy::version = "1.84.0"]
+    pub MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
+    restriction,
+    "map of a trivial closure (not dependent on parameter) over a range"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4381,6 +4416,7 @@ impl_lint_pass!(Methods => [
     UNNECESSARY_MIN_OR_MAX,
     NEEDLESS_AS_BYTES,
     MAP_ALL_ANY_IDENTITY,
+    MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4876,6 +4912,7 @@ impl Methods {
                     if name == "map" {
                         unused_enumerate_index::check(cx, expr, recv, m_arg);
                         map_clone::check(cx, expr, recv, m_arg, &self.msrv);
+                        map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, &self.msrv, span);
                         match method_call(recv) {
                             Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => {
                                 iter_kv_map::check(cx, map_name, expr, recv2, m_arg, &self.msrv);

tests/ui/map_with_unused_argument_over_ranges.fixed

@@ -0,0 +1,73 @@
+#![allow(
+    unused,
+    clippy::redundant_closure,
+    clippy::reversed_empty_ranges,
+    clippy::identity_op
+)]
+#![warn(clippy::map_with_unused_argument_over_ranges)]
+
+fn do_something() -> usize {
+    todo!()
+}
+
+fn do_something_interesting(x: usize, y: usize) -> usize {
+    todo!()
+}
+
+macro_rules! gen {
+    () => {
+        (0..10).map(|_| do_something());
+    };
+}
+
+fn main() {
+    // These should be raised
+    std::iter::repeat_with(|| do_something()).take(10);
+    std::iter::repeat_with(|| do_something()).take(10);
+    std::iter::repeat_with(|| do_something()).take(11);
+    std::iter::repeat_with(|| do_something()).take(7);
+    std::iter::repeat_with(|| do_something()).take(8);
+    std::iter::repeat_n(3, 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());
+    std::iter::repeat_with(|| do_something()).take(upper_fn() + 1);
+    std::iter::repeat_with(|| do_something()).take(upper_fn() - 2);
+    std::iter::repeat_with(|| do_something()).take(upper_fn() - 1);
+    (-3..9).map(|_| do_something());
+    std::iter::repeat_with(|| do_something()).take(0);
+    std::iter::repeat_with(|| do_something()).take(1);
+    std::iter::repeat_with(|| do_something()).take((1 << 4) - 0);
+    // These should not be raised
+    gen!();
+    let lower = 2;
+    let lower_fn = || 2;
+    (lower..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
+    (lower_fn()..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
+    (lower_fn()..=upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
+    (0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range
+    "Foobar".chars().map(|_| do_something()); // Not a map over range
+    // i128::MAX == 340282366920938463463374607431768211455
+    (0..=340282366920938463463374607431768211455).map(|_: u128| do_something()); // Can't be replaced due to overflow
+}
+
+#[clippy::msrv = "1.27"]
+fn msrv_1_27() {
+    (0..10).map(|_| do_something());
+}
+
+#[clippy::msrv = "1.28"]
+fn msrv_1_28() {
+    std::iter::repeat_with(|| do_something()).take(10);
+}
+
+#[clippy::msrv = "1.81"]
+fn msrv_1_82() {
+    std::iter::repeat(3).take(10);
+}