Skip to content

Commit 91a94cf

Browse files
committed
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`.
1 parent c412528 commit 91a94cf

10 files changed

+214
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5884,6 +5884,7 @@ Released 2018-09-13
58845884
[`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts
58855885
[`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null
58865886
[`trim_split_whitespace`]: https://rust-lang.github.io/rust-clippy/master/index.html#trim_split_whitespace
5887+
[`trivial_map_over_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_map_over_range
58875888
[`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
58885889
[`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
58895890
[`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
705705
crate::transmute::UNSOUND_COLLECTION_TRANSMUTE_INFO,
706706
crate::transmute::USELESS_TRANSMUTE_INFO,
707707
crate::transmute::WRONG_TRANSMUTE_INFO,
708+
crate::trivial_map_over_range::TRIVIAL_MAP_OVER_RANGE_INFO,
708709
crate::tuple_array_conversions::TUPLE_ARRAY_CONVERSIONS_INFO,
709710
crate::types::BORROWED_BOX_INFO,
710711
crate::types::BOX_COLLECTION_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,7 @@ mod to_string_trait_impl;
345345
mod trailing_empty_array;
346346
mod trait_bounds;
347347
mod transmute;
348+
mod trivial_map_over_range;
348349
mod tuple_array_conversions;
349350
mod types;
350351
mod unconditional_recursion;
@@ -1172,6 +1173,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11721173
});
11731174
store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(msrv())));
11741175
store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers));
1176+
store.register_late_pass(|_| Box::new(trivial_map_over_range::TrivialMapOverRange));
11751177
// add lints here, do not remove this comment, it's used in `new_lint`
11761178
}
11771179

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_applicability;
3+
use rustc_ast::LitKind;
4+
use rustc_data_structures::packed::Pu128;
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{Body, Closure, Expr, ExprKind, LangItem, PatKind, QPath};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_session::declare_lint_pass;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
///
13+
/// Lint to prevent trivial `map`s over ranges when one could use `take`.
14+
///
15+
/// ### Why is this bad?
16+
///
17+
/// It expreses the intent more clearly to `take` the correct number of times
18+
/// from a generating function than to apply a closure to each number in a
19+
/// range only to discard them.
20+
///
21+
/// ### Example
22+
/// ```no_run
23+
/// let random_numbers : Vec<_> = (0..10).map(|_| { 3 + 1 }).collect();
24+
/// ```
25+
/// Use instead:
26+
/// ```no_run
27+
/// let f : Vec<_> = std::iter::from_fn(|| { 3 + 1 }).take(10).collect();
28+
/// ```
29+
#[clippy::version = "1.81.0"]
30+
pub TRIVIAL_MAP_OVER_RANGE,
31+
style,
32+
"map of a trivial closure (not dependent on parameter) over a range"
33+
}
34+
35+
declare_lint_pass!(TrivialMapOverRange => [TRIVIAL_MAP_OVER_RANGE]);
36+
37+
impl LateLintPass<'_> for TrivialMapOverRange {
38+
fn check_expr(&mut self, cx: &LateContext<'_>, ex: &Expr<'_>) {
39+
if let ExprKind::MethodCall(path, receiver, [map_arg_expr], _call_span) = ex.kind
40+
&& path.ident.name == rustc_span::sym::map
41+
&& let ExprKind::Struct(qpath, fields, _) = receiver.kind
42+
&& matches!(qpath, QPath::LangItem(LangItem::Range, _))
43+
&& fields.len() == 2
44+
&& let ExprKind::Closure(Closure { body, .. }) = map_arg_expr.kind
45+
&& let Body { params: [param], .. } = cx.tcx.hir().body(*body)
46+
&& matches!(param.pat.kind, PatKind::Wild)
47+
&& let ExprKind::Lit(lit) = fields[0].expr.kind
48+
&& let LitKind::Int(Pu128(lower_bound), _) = lit.node
49+
{
50+
if let ExprKind::Lit(lit) = fields[1].expr.kind
51+
&& let LitKind::Int(Pu128(upper_bound), _) = lit.node
52+
{
53+
let count = upper_bound - lower_bound;
54+
let mut applicability = Applicability::MaybeIncorrect;
55+
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
56+
.replacen("|_|", "||", 1);
57+
span_lint_and_sugg(
58+
cx,
59+
TRIVIAL_MAP_OVER_RANGE,
60+
ex.span,
61+
"map of a trivial closure (not dependent on parameter) over a range",
62+
"use",
63+
format!("std::iter::repeat_with({snippet}).take({count})"),
64+
applicability,
65+
);
66+
} else if lower_bound == 0 {
67+
let mut applicability = Applicability::MaybeIncorrect;
68+
let count = snippet_with_applicability(cx, fields[1].expr.span, "...", &mut applicability);
69+
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
70+
.replacen("|_|", "||", 1);
71+
span_lint_and_sugg(
72+
cx,
73+
TRIVIAL_MAP_OVER_RANGE,
74+
ex.span,
75+
"map of a trivial closure (not dependent on parameter) over a range",
76+
"use",
77+
format!("std::iter::repeat_with({snippet}).take({count})"),
78+
applicability,
79+
);
80+
}
81+
}
82+
}
83+
}

tests/ui/repeat_vec_with_capacity.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![allow(clippy::trivial_map_over_range)]
12
#![warn(clippy::repeat_vec_with_capacity)]
23

34
fn main() {

tests/ui/repeat_vec_with_capacity.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![allow(clippy::trivial_map_over_range)]
12
#![warn(clippy::repeat_vec_with_capacity)]
23

34
fn main() {

tests/ui/repeat_vec_with_capacity.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
2-
--> tests/ui/repeat_vec_with_capacity.rs:5:9
2+
--> tests/ui/repeat_vec_with_capacity.rs:6:9
33
|
44
LL | vec![Vec::<()>::with_capacity(42); 123];
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -13,7 +13,7 @@ LL | (0..123).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
1313
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1414

1515
error: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
16-
--> tests/ui/repeat_vec_with_capacity.rs:11:9
16+
--> tests/ui/repeat_vec_with_capacity.rs:12:9
1717
|
1818
LL | vec![Vec::<()>::with_capacity(42); n];
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -25,7 +25,7 @@ LL | (0..n).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
2525
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2626

2727
error: repeating `Vec::with_capacity` using `iter::repeat`, which does not retain capacity
28-
--> tests/ui/repeat_vec_with_capacity.rs:26:9
28+
--> tests/ui/repeat_vec_with_capacity.rs:27:9
2929
|
3030
LL | std::iter::repeat(Vec::<()>::with_capacity(42));
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

tests/ui/trivial_map_over_range.fixed

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![allow(unused, clippy::redundant_closure)]
2+
#![warn(clippy::trivial_map_over_range)]
3+
4+
fn do_something() -> usize {
5+
todo!()
6+
}
7+
8+
fn do_something_interesting(x: usize, y: usize) -> usize {
9+
todo!()
10+
}
11+
12+
fn main() {
13+
// These should be raised
14+
std::iter::repeat_with(|| do_something()).take(10);
15+
std::iter::repeat_with(|| do_something()).take(7);
16+
std::iter::repeat_with(|| 3).take(10);
17+
std::iter::repeat_with(|| {
18+
let x = 3;
19+
x + 2
20+
}).take(10);
21+
std::iter::repeat_with(|| do_something()).take(10).collect::<Vec<_>>();
22+
let upper = 4;
23+
std::iter::repeat_with(|| do_something()).take(upper);
24+
let upper_fn = || 4;
25+
std::iter::repeat_with(|| do_something()).take(upper_fn());
26+
// These should not be raised
27+
(0..=10).map(|_| do_something()); // Inclusive bounds not yet handled
28+
(0..10).map(|x| do_something()); // We do not detect unused parameters
29+
(0..10).map(|x| do_something()).collect::<Vec<_>>(); // We do not detect unused parameters
30+
(0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range
31+
"Foobar".chars().map(|_| do_something()); // Not a map over range
32+
}

tests/ui/trivial_map_over_range.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![allow(unused, clippy::redundant_closure)]
2+
#![warn(clippy::trivial_map_over_range)]
3+
4+
fn do_something() -> usize {
5+
todo!()
6+
}
7+
8+
fn do_something_interesting(x: usize, y: usize) -> usize {
9+
todo!()
10+
}
11+
12+
fn main() {
13+
// These should be raised
14+
(0..10).map(|_| do_something());
15+
(3..10).map(|_| do_something());
16+
(0..10).map(|_| 3);
17+
(0..10).map(|_| {
18+
let x = 3;
19+
x + 2
20+
});
21+
(0..10).map(|_| do_something()).collect::<Vec<_>>();
22+
let upper = 4;
23+
(0..upper).map(|_| do_something());
24+
let upper_fn = || 4;
25+
(0..upper_fn()).map(|_| do_something());
26+
// These should not be raised
27+
(0..=10).map(|_| do_something()); // Inclusive bounds not yet handled
28+
(0..10).map(|x| do_something()); // We do not detect unused parameters
29+
(0..10).map(|x| do_something()).collect::<Vec<_>>(); // We do not detect unused parameters
30+
(0..10).map(|x| do_something_interesting(x, 4)); // Actual map over range
31+
"Foobar".chars().map(|_| do_something()); // Not a map over range
32+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
error: map of a trivial closure (not dependent on parameter) over a range
2+
--> tests/ui/trivial_map_over_range.rs:14:5
3+
|
4+
LL | (0..10).map(|_| do_something());
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
6+
|
7+
= note: `-D clippy::trivial-map-over-range` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::trivial_map_over_range)]`
9+
10+
error: map of a trivial closure (not dependent on parameter) over a range
11+
--> tests/ui/trivial_map_over_range.rs:15:5
12+
|
13+
LL | (3..10).map(|_| do_something());
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(7)`
15+
16+
error: map of a trivial closure (not dependent on parameter) over a range
17+
--> tests/ui/trivial_map_over_range.rs:16:5
18+
|
19+
LL | (0..10).map(|_| 3);
20+
| ^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| 3).take(10)`
21+
22+
error: map of a trivial closure (not dependent on parameter) over a range
23+
--> tests/ui/trivial_map_over_range.rs:17:5
24+
|
25+
LL | / (0..10).map(|_| {
26+
LL | | let x = 3;
27+
LL | | x + 2
28+
LL | | });
29+
| |______^
30+
|
31+
help: use
32+
|
33+
LL ~ std::iter::repeat_with(|| {
34+
LL + let x = 3;
35+
LL + x + 2
36+
LL ~ }).take(10);
37+
|
38+
39+
error: map of a trivial closure (not dependent on parameter) over a range
40+
--> tests/ui/trivial_map_over_range.rs:21:5
41+
|
42+
LL | (0..10).map(|_| do_something()).collect::<Vec<_>>();
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
44+
45+
error: map of a trivial closure (not dependent on parameter) over a range
46+
--> tests/ui/trivial_map_over_range.rs:23:5
47+
|
48+
LL | (0..upper).map(|_| do_something());
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper)`
50+
51+
error: map of a trivial closure (not dependent on parameter) over a range
52+
--> tests/ui/trivial_map_over_range.rs:25:5
53+
|
54+
LL | (0..upper_fn()).map(|_| do_something());
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn())`
56+
57+
error: aborting due to 7 previous errors
58+

0 commit comments

Comments
 (0)