Skip to content

Commit 208a7e6

Browse files
rspencer01rspencer01
committed
Add new map_with_unused_argument_over_ranges 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(); ``` or ```rust let something : Vec<_> = std::iter::repeat_n(1 + 2 + 3, 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`.
1 parent ccb30bf commit 208a7e6

18 files changed

+562
-10
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5661,6 +5661,7 @@ Released 2018-09-13
56615661
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
56625662
[`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
56635663
[`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or
5664+
[`map_with_unused_argument_over_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_with_unused_argument_over_ranges
56645665
[`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref
56655666
[`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool
56665667
[`match_like_matches_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
77

8-
[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
8+
[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
99

1010
Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
1111
You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

‎book/src/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
A collection of lints to catch common mistakes and improve your
77
[Rust](https://github.com/rust-lang/rust) code.
88

9-
[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
9+
[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1010

1111
Lints are divided into categories, each with a default [lint
1212
level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

‎book/src/lint_configuration.md

+1
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
710710
* [`manual_try_fold`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold)
711711
* [`map_clone`](https://rust-lang.github.io/rust-clippy/master/index.html#map_clone)
712712
* [`map_unwrap_or`](https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or)
713+
* [`map_with_unused_argument_over_ranges`](https://rust-lang.github.io/rust-clippy/master/index.html#map_with_unused_argument_over_ranges)
713714
* [`match_like_matches_macro`](https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro)
714715
* [`mem_replace_with_default`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default)
715716
* [`missing_const_for_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn)

‎clippy_config/src/conf.rs

+1
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,7 @@ define_Conf! {
570570
manual_try_fold,
571571
map_clone,
572572
map_unwrap_or,
573+
map_with_unused_argument_over_ranges,
573574
match_like_matches_macro,
574575
mem_replace_with_default,
575576
missing_const_for_fn,

‎clippy_config/src/msrvs.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ macro_rules! msrv_aliases {
1717

1818
// names may refer to stabilized feature flags or library items
1919
msrv_aliases! {
20-
1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY }
20+
1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, REPEAT_N }
2121
1,82,0 { IS_NONE_OR }
2222
1,81,0 { LINT_REASONS_STABILIZATION }
2323
1,80,0 { BOX_INTO_ITER}
@@ -54,7 +54,7 @@ msrv_aliases! {
5454
1,33,0 { UNDERSCORE_IMPORTS }
5555
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
5656
1,29,0 { ITER_FLATTEN }
57-
1,28,0 { FROM_BOOL }
57+
1,28,0 { FROM_BOOL, REPEAT_WITH }
5858
1,27,0 { ITERATOR_TRY_FOLD }
5959
1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN }
6060
1,24,0 { IS_ASCII_DIGIT }

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
420420
crate::methods::MAP_FLATTEN_INFO,
421421
crate::methods::MAP_IDENTITY_INFO,
422422
crate::methods::MAP_UNWRAP_OR_INFO,
423+
crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES_INFO,
423424
crate::methods::MUT_MUTEX_LOCK_INFO,
424425
crate::methods::NAIVE_BYTECOUNT_INFO,
425426
crate::methods::NEEDLESS_CHARACTER_ITERATION_INFO,

‎clippy_lints/src/matches/single_match.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,10 @@ impl<'a> PatState<'a> {
249249
let states = match self {
250250
Self::Wild => return None,
251251
Self::Other => {
252-
*self = Self::StdEnum(cx.arena.alloc_from_iter((0..adt.variants().len()).map(|_| Self::Other)));
252+
*self = Self::StdEnum(
253+
cx.arena
254+
.alloc_from_iter(std::iter::repeat_with(|| Self::Other).take(adt.variants().len())),
255+
);
253256
let Self::StdEnum(x) = self else {
254257
unreachable!();
255258
};

‎clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs

+135
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
use crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES;
2+
use clippy_config::msrvs::{self, Msrv};
3+
use clippy_utils::diagnostics::span_lint_and_then;
4+
use clippy_utils::source::snippet_with_applicability;
5+
use clippy_utils::sugg::Sugg;
6+
use clippy_utils::{eager_or_lazy, higher, usage};
7+
use rustc_ast::LitKind;
8+
use rustc_ast::ast::RangeLimits;
9+
use rustc_data_structures::packed::Pu128;
10+
use rustc_errors::Applicability;
11+
use rustc_hir::{Body, Closure, Expr, ExprKind};
12+
use rustc_lint::LateContext;
13+
use rustc_span::Span;
14+
15+
fn extract_count_with_applicability(
16+
cx: &LateContext<'_>,
17+
range: higher::Range<'_>,
18+
applicability: &mut Applicability,
19+
) -> Option<String> {
20+
let start = range.start?;
21+
let end = range.end?;
22+
// TODO: This doens't handle if either the start or end are negative literals, or if the start is
23+
// not a literal. In the first case, we need to be careful about how we handle computing the
24+
// count to avoid overflows. In the second, we may need to add parenthesis to make the
25+
// suggestion correct.
26+
if let ExprKind::Lit(lit) = start.kind
27+
&& let LitKind::Int(Pu128(lower_bound), _) = lit.node
28+
{
29+
if let ExprKind::Lit(lit) = end.kind
30+
&& let LitKind::Int(Pu128(upper_bound), _) = lit.node
31+
{
32+
// Here we can explicitly calculate the number of iterations
33+
let count = if upper_bound >= lower_bound {
34+
match range.limits {
35+
RangeLimits::HalfOpen => upper_bound - lower_bound,
36+
RangeLimits::Closed => (upper_bound - lower_bound).checked_add(1)?,
37+
}
38+
} else {
39+
0
40+
};
41+
return Some(format!("{count}"));
42+
}
43+
let end_snippet = Sugg::hir_with_applicability(cx, end, "...", applicability)
44+
.maybe_par()
45+
.into_string();
46+
if lower_bound == 0 {
47+
if range.limits == RangeLimits::Closed {
48+
return Some(format!("{end_snippet} + 1"));
49+
}
50+
return Some(end_snippet);
51+
}
52+
if range.limits == RangeLimits::Closed {
53+
return Some(format!("{end_snippet} - {}", lower_bound - 1));
54+
}
55+
return Some(format!("{end_snippet} - {lower_bound}"));
56+
}
57+
None
58+
}
59+
60+
pub(super) fn check(
61+
cx: &LateContext<'_>,
62+
ex: &Expr<'_>,
63+
receiver: &Expr<'_>,
64+
arg: &Expr<'_>,
65+
msrv: &Msrv,
66+
method_call_span: Span,
67+
) {
68+
let mut applicability = Applicability::MaybeIncorrect;
69+
if let Some(range) = higher::Range::hir(receiver)
70+
&& let ExprKind::Closure(Closure { body, .. }) = arg.kind
71+
&& let body_hir = cx.tcx.hir().body(*body)
72+
&& let Body {
73+
params: [param],
74+
value: body_expr,
75+
} = body_hir
76+
&& !usage::BindingUsageFinder::are_params_used(cx, body_hir)
77+
&& let Some(count) = extract_count_with_applicability(cx, range, &mut applicability)
78+
{
79+
let method_to_use_name;
80+
let new_span;
81+
let use_take;
82+
83+
if eager_or_lazy::switch_to_eager_eval(cx, body_expr) {
84+
if msrv.meets(msrvs::REPEAT_N) {
85+
method_to_use_name = "repeat_n";
86+
let body_snippet = snippet_with_applicability(cx, body_expr.span, "..", &mut applicability);
87+
new_span = (arg.span, format!("{body_snippet}, {count}"));
88+
use_take = false;
89+
} else {
90+
method_to_use_name = "repeat";
91+
let body_snippet = snippet_with_applicability(cx, body_expr.span, "..", &mut applicability);
92+
new_span = (arg.span, body_snippet.to_string());
93+
use_take = true;
94+
}
95+
} else if msrv.meets(msrvs::REPEAT_WITH) {
96+
method_to_use_name = "repeat_with";
97+
new_span = (param.span, String::new());
98+
use_take = true;
99+
} else {
100+
return;
101+
}
102+
103+
span_lint_and_then(
104+
cx,
105+
MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
106+
ex.span,
107+
"map of a closure that does not depend on its parameter over a range",
108+
|diag| {
109+
diag.multipart_suggestion(
110+
if use_take {
111+
format!("remove the explicit range and use `{method_to_use_name}` and `take`")
112+
} else {
113+
format!("remove the explicit range and use `{method_to_use_name}`")
114+
},
115+
vec![
116+
(
117+
receiver.span.to(method_call_span),
118+
format!("std::iter::{method_to_use_name}"),
119+
),
120+
new_span,
121+
(
122+
ex.span.shrink_to_hi(),
123+
if use_take {
124+
format!(".take({count})")
125+
} else {
126+
String::new()
127+
},
128+
),
129+
],
130+
applicability,
131+
);
132+
},
133+
);
134+
}
135+
}

‎clippy_lints/src/methods/mod.rs

+38
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ mod map_err_ignore;
6666
mod map_flatten;
6767
mod map_identity;
6868
mod map_unwrap_or;
69+
mod map_with_unused_argument_over_ranges;
6970
mod mut_mutex_lock;
7071
mod needless_character_iteration;
7172
mod needless_collect;
@@ -4166,6 +4167,41 @@ declare_clippy_lint! {
41664167
"calling `.first().is_some()` or `.first().is_none()` instead of `.is_empty()`"
41674168
}
41684169

4170+
declare_clippy_lint! {
4171+
/// ### What it does
4172+
///
4173+
/// Checks for `Iterator::map` over ranges without using the parameter which
4174+
/// could be more clearly expressed using `std::iter::repeat(...).take(...)`
4175+
/// or `std::iter::repeat_n`.
4176+
///
4177+
/// ### Why is this bad?
4178+
///
4179+
/// It expresses the intent more clearly to `take` the correct number of times
4180+
/// from a generating function than to apply a closure to each number in a
4181+
/// range only to discard them.
4182+
///
4183+
/// ### Example
4184+
///
4185+
/// ```no_run
4186+
/// let random_numbers : Vec<_> = (0..10).map(|_| { 3 + 1 }).collect();
4187+
/// ```
4188+
/// Use instead:
4189+
/// ```no_run
4190+
/// let f : Vec<_> = std::iter::repeat( 3 + 1 ).take(10).collect();
4191+
/// ```
4192+
///
4193+
/// ### Known Issues
4194+
///
4195+
/// This lint may suggest replacing a `Map<Range>` with a `Take<RepeatWith>` or
4196+
/// `Take<Repeat>`. The former implements some traits that the latter two do
4197+
/// not, such as `DoubleEndedIterator`. As a result, this may not always be an
4198+
/// appropriate suggestion.
4199+
#[clippy::version = "1.81.0"]
4200+
pub MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
4201+
restriction,
4202+
"map of a trivial closure (not dependent on parameter) over a range"
4203+
}
4204+
41694205
pub struct Methods {
41704206
avoid_breaking_exported_api: bool,
41714207
msrv: Msrv,
@@ -4327,6 +4363,7 @@ impl_lint_pass!(Methods => [
43274363
NEEDLESS_CHARACTER_ITERATION,
43284364
MANUAL_INSPECT,
43294365
UNNECESSARY_MIN_OR_MAX,
4366+
MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
43304367
]);
43314368

43324369
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4802,6 +4839,7 @@ impl Methods {
48024839
if name == "map" {
48034840
unused_enumerate_index::check(cx, expr, recv, m_arg);
48044841
map_clone::check(cx, expr, recv, m_arg, &self.msrv);
4842+
map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, &self.msrv, span);
48054843
match method_call(recv) {
48064844
Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => {
48074845
iter_kv_map::check(cx, map_name, expr, recv2, m_arg, &self.msrv);

‎tests/ui/map_with_unused_argument_over_ranges.fixed

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#![allow(
2+
unused,
3+
clippy::redundant_closure,
4+
clippy::reversed_empty_ranges,
5+
clippy::identity_op
6+
)]
7+
#![warn(clippy::map_with_unused_argument_over_ranges)]
8+
9+
fn do_something() -> usize {
10+
todo!()
11+
}
12+
13+
fn do_something_interesting(x: usize, y: usize) -> usize {
14+
todo!()
15+
}
16+
17+
macro_rules! gen {
18+
() => {
19+
(0..10).map(|_| do_something());
20+
};
21+
}
22+
23+
fn main() {
24+
// These should be raised
25+
std::iter::repeat_with(|| do_something()).take(10);
26+
std::iter::repeat_with(|| do_something()).take(10);
27+
std::iter::repeat_with(|| do_something()).take(11);
28+
std::iter::repeat_with(|| do_something()).take(7);
29+
std::iter::repeat_with(|| do_something()).take(8);
30+
std::iter::repeat_n(3, 10);
31+
std::iter::repeat_with(|| {
32+
let x = 3;
33+
x + 2
34+
}).take(10);
35+
std::iter::repeat_with(|| do_something()).take(10).collect::<Vec<_>>();
36+
let upper = 4;
37+
std::iter::repeat_with(|| do_something()).take(upper);
38+
let upper_fn = || 4;
39+
std::iter::repeat_with(|| do_something()).take(upper_fn());
40+
std::iter::repeat_with(|| do_something()).take(upper_fn() + 1);
41+
std::iter::repeat_with(|| do_something()).take(upper_fn() - 2);
42+
std::iter::repeat_with(|| do_something()).take(upper_fn() - 1);
43+
(-3..9).map(|_| do_something());
44+
std::iter::repeat_with(|| do_something()).take(0);
45+
std::iter::repeat_with(|| do_something()).take(1);
46+
std::iter::repeat_with(|| do_something()).take((1 << 4) - 0);
47+
// These should not be raised
48+
gen!();
49+
let lower = 2;
50+
let lower_fn = || 2;
51+
(lower..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
52+
(lower_fn()..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
53+
(lower_fn()..=upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
54+
(0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range
55+
"Foobar".chars().map(|_| do_something()); // Not a map over range
56+
// i128::MAX == 340282366920938463463374607431768211455
57+
(0..=340282366920938463463374607431768211455).map(|_: u128| do_something()); // Can't be replaced due to overflow
58+
}
59+
60+
#[clippy::msrv = "1.27"]
61+
fn msrv_1_27() {
62+
(0..10).map(|_| do_something());
63+
}
64+
65+
#[clippy::msrv = "1.28"]
66+
fn msrv_1_28() {
67+
std::iter::repeat_with(|| do_something()).take(10);
68+
}
69+
70+
#[clippy::msrv = "1.82"]
71+
fn msrv_1_82() {
72+
std::iter::repeat(3).take(10);
73+
}

0 commit comments

Comments
 (0)