Skip to content

Commit 7e3e245

Browse files
rspencer01rspencer01
committed
Refactor extracing the range to its own function
This lets us iterate on this and separate its concern from the rest of the lint. It also makes the flow slightly more obvious.
1 parent b3d632e commit 7e3e245

File tree

4 files changed

+116
-59
lines changed

4 files changed

+116
-59
lines changed

‎clippy_lints/src/map_with_unused_argument_over_ranges.rs‎

Lines changed: 55 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -49,63 +49,72 @@ impl MapWithUnusedArgumentOverRanges {
4949

5050
impl_lint_pass!(MapWithUnusedArgumentOverRanges => [MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES]);
5151

52+
fn extract_count_with_applicability(
53+
cx: &LateContext<'_>,
54+
range: higher::Range<'_>,
55+
applicability: &mut Applicability,
56+
) -> Option<String> {
57+
let start = range.start?;
58+
let end = range.end?;
59+
// TODO: This doens't handle if either the start or end are negative literals, or if the start is
60+
// not a literal In the first case, we need to be careful about how we handle computing the
61+
// count to avoid overflows. In the second, we may need to add parenthesis to make the
62+
// suggestion correct.
63+
if let ExprKind::Lit(lit) = start.kind
64+
&& let LitKind::Int(Pu128(lower_bound), _) = lit.node
65+
{
66+
if let ExprKind::Lit(lit) = end.kind
67+
&& let LitKind::Int(Pu128(upper_bound), _) = lit.node
68+
{
69+
let count = if range.limits == RangeLimits::Closed {
70+
upper_bound - lower_bound + 1
71+
} else {
72+
upper_bound - lower_bound
73+
};
74+
return Some(format!("{count}"));
75+
} else {
76+
let end_snippet = snippet_with_applicability(cx, end.span, "...", applicability).into_owned();
77+
if lower_bound == 0 {
78+
if range.limits == RangeLimits::Closed {
79+
return Some(format!("{end_snippet} + 1"));
80+
} else {
81+
return Some(end_snippet);
82+
};
83+
} else if range.limits == RangeLimits::Closed {
84+
return Some(format!("{} + {}", end_snippet, lower_bound + 1));
85+
} else {
86+
return Some(format!("{} + {}", end_snippet, lower_bound));
87+
};
88+
}
89+
}
90+
None
91+
}
92+
5293
impl LateLintPass<'_> for MapWithUnusedArgumentOverRanges {
5394
fn check_expr(&mut self, cx: &LateContext<'_>, ex: &Expr<'_>) {
5495
if !self.msrv.meets(msrvs::REPEAT_WITH) {
5596
return;
5697
}
98+
let mut applicability = Applicability::MaybeIncorrect;
5799
if let ExprKind::MethodCall(path, receiver, [map_arg_expr], _call_span) = ex.kind
58100
&& path.ident.name == rustc_span::sym::map
59-
&& let Some(higher::Range {
60-
start: Some(start),
61-
end: Some(end),
62-
limits,
63-
}) = higher::Range::hir(receiver)
101+
&& let Some(range) = higher::Range::hir(receiver)
64102
&& let ExprKind::Closure(Closure { body, .. }) = map_arg_expr.kind
65103
&& let Body { params: [param], .. } = cx.tcx.hir().body(*body)
66104
&& matches!(param.pat.kind, PatKind::Wild)
67-
&& let ExprKind::Lit(lit) = start.kind
68-
&& let LitKind::Int(Pu128(lower_bound), _) = lit.node
105+
&& let Some(count) = extract_count_with_applicability(cx, range, &mut applicability)
69106
{
70-
if let ExprKind::Lit(lit) = end.kind
71-
&& let LitKind::Int(Pu128(upper_bound), _) = lit.node
72-
{
73-
let count = if limits == RangeLimits::Closed {
74-
upper_bound - lower_bound + 1
75-
} else {
76-
upper_bound - lower_bound
77-
};
78-
let mut applicability = Applicability::MaybeIncorrect;
79-
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
80-
.replacen("|_|", "||", 1);
81-
span_lint_and_sugg(
82-
cx,
83-
MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
84-
ex.span,
85-
"map of a trivial closure (not dependent on parameter) over a range",
86-
"use",
87-
format!("std::iter::repeat_with({snippet}).take({count})"),
88-
applicability,
89-
);
90-
} else if lower_bound == 0 {
91-
let mut applicability = Applicability::MaybeIncorrect;
92-
let count = if limits == RangeLimits::Closed {
93-
snippet_with_applicability(cx, end.span, "...", &mut applicability) + " + 1"
94-
} else {
95-
snippet_with_applicability(cx, end.span, "...", &mut applicability)
96-
};
97-
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
98-
.replacen("|_|", "||", 1);
99-
span_lint_and_sugg(
100-
cx,
101-
MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
102-
ex.span,
103-
"map of a trivial closure (not dependent on parameter) over a range",
104-
"use",
105-
format!("std::iter::repeat_with({snippet}).take({count})"),
106-
applicability,
107-
);
108-
}
107+
let snippet = snippet_with_applicability(cx, map_arg_expr.span, "|| { ... }", &mut applicability)
108+
.replacen("|_|", "||", 1);
109+
span_lint_and_sugg(
110+
cx,
111+
MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
112+
ex.span,
113+
"map of a trivial closure (not dependent on parameter) over a range",
114+
"use",
115+
format!("std::iter::repeat_with({snippet}).take({count})"),
116+
applicability,
117+
);
109118
}
110119
}
111120
extract_msrv_attr!(LateContext);

‎tests/ui/map_with_unused_argument_over_ranges.fixed‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ fn do_something_interesting(x: usize, y: usize) -> usize {
99
todo!()
1010
}
1111

12+
macro_rules! gen {
13+
() => {
14+
std::iter::repeat_with(|| do_something()).take(10);
15+
};
16+
}
17+
1218
fn main() {
1319
// These should be raised
1420
std::iter::repeat_with(|| do_something()).take(10);
@@ -25,10 +31,14 @@ fn main() {
2531
let upper_fn = || 4;
2632
std::iter::repeat_with(|| do_something()).take(upper_fn());
2733
std::iter::repeat_with(|| do_something()).take(upper_fn() + 1);
34+
std::iter::repeat_with(|| do_something()).take(upper_fn() + 2);
35+
std::iter::repeat_with(|| do_something()).take(upper_fn() + 3);
36+
(-3..9).map(|_| do_something());
2837
// These should not be raised
38+
gen!();
2939
let lower = 2;
3040
let lower_fn = || 2;
31-
(2..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
41+
std::iter::repeat_with(|| do_something()).take(upper_fn() + 2); // Ranges not starting at zero not yet handled
3242
(lower..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
3343
(lower_fn()..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
3444
(lower_fn()..=upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
@@ -38,7 +48,6 @@ fn main() {
3848
"Foobar".chars().map(|_| do_something()); // Not a map over range
3949
}
4050

41-
4251
#[clippy::msrv = "1.27"]
4352
fn msrv_1_27() {
4453
(0..10).map(|_| do_something());

‎tests/ui/map_with_unused_argument_over_ranges.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ fn do_something_interesting(x: usize, y: usize) -> usize {
99
todo!()
1010
}
1111

12+
macro_rules! gen {
13+
() => {
14+
(0..10).map(|_| do_something());
15+
};
16+
}
17+
1218
fn main() {
1319
// These should be raised
1420
(0..10).map(|_| do_something());
@@ -25,7 +31,11 @@ fn main() {
2531
let upper_fn = || 4;
2632
(0..upper_fn()).map(|_| do_something());
2733
(0..=upper_fn()).map(|_| do_something());
34+
(2..upper_fn()).map(|_| do_something());
35+
(2..=upper_fn()).map(|_| do_something());
36+
(-3..9).map(|_| do_something());
2837
// These should not be raised
38+
gen!();
2939
let lower = 2;
3040
let lower_fn = || 2;
3141
(2..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled

‎tests/ui/map_with_unused_argument_over_ranges.stderr‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: map of a trivial closure (not dependent on parameter) over a range
2-
--> tests/ui/map_with_unused_argument_over_ranges.rs:14:5
2+
--> tests/ui/map_with_unused_argument_over_ranges.rs:20:5
33
|
44
LL | (0..10).map(|_| do_something());
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
@@ -8,25 +8,25 @@ LL | (0..10).map(|_| do_something());
88
= help: to override `-D warnings` add `#[allow(clippy::map_with_unused_argument_over_ranges)]`
99

1010
error: map of a trivial closure (not dependent on parameter) over a range
11-
--> tests/ui/map_with_unused_argument_over_ranges.rs:15:5
11+
--> tests/ui/map_with_unused_argument_over_ranges.rs:21:5
1212
|
1313
LL | (0..=10).map(|_| do_something());
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(11)`
1515

1616
error: map of a trivial closure (not dependent on parameter) over a range
17-
--> tests/ui/map_with_unused_argument_over_ranges.rs:16:5
17+
--> tests/ui/map_with_unused_argument_over_ranges.rs:22:5
1818
|
1919
LL | (3..10).map(|_| do_something());
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(7)`
2121

2222
error: map of a trivial closure (not dependent on parameter) over a range
23-
--> tests/ui/map_with_unused_argument_over_ranges.rs:17:5
23+
--> tests/ui/map_with_unused_argument_over_ranges.rs:23:5
2424
|
2525
LL | (0..10).map(|_| 3);
2626
| ^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| 3).take(10)`
2727

2828
error: map of a trivial closure (not dependent on parameter) over a range
29-
--> tests/ui/map_with_unused_argument_over_ranges.rs:18:5
29+
--> tests/ui/map_with_unused_argument_over_ranges.rs:24:5
3030
|
3131
LL | / (0..10).map(|_| {
3232
LL | | let x = 3;
@@ -43,34 +43,63 @@ LL ~ }).take(10);
4343
|
4444

4545
error: map of a trivial closure (not dependent on parameter) over a range
46-
--> tests/ui/map_with_unused_argument_over_ranges.rs:22:5
46+
--> tests/ui/map_with_unused_argument_over_ranges.rs:28:5
4747
|
4848
LL | (0..10).map(|_| do_something()).collect::<Vec<_>>();
4949
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
5050

5151
error: map of a trivial closure (not dependent on parameter) over a range
52-
--> tests/ui/map_with_unused_argument_over_ranges.rs:24:5
52+
--> tests/ui/map_with_unused_argument_over_ranges.rs:30:5
5353
|
5454
LL | (0..upper).map(|_| do_something());
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper)`
5656

5757
error: map of a trivial closure (not dependent on parameter) over a range
58-
--> tests/ui/map_with_unused_argument_over_ranges.rs:26:5
58+
--> tests/ui/map_with_unused_argument_over_ranges.rs:32:5
5959
|
6060
LL | (0..upper_fn()).map(|_| do_something());
6161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn())`
6262

6363
error: map of a trivial closure (not dependent on parameter) over a range
64-
--> tests/ui/map_with_unused_argument_over_ranges.rs:27:5
64+
--> tests/ui/map_with_unused_argument_over_ranges.rs:33:5
6565
|
6666
LL | (0..=upper_fn()).map(|_| do_something());
6767
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn() + 1)`
6868

6969
error: map of a trivial closure (not dependent on parameter) over a range
70-
--> tests/ui/map_with_unused_argument_over_ranges.rs:49:5
70+
--> tests/ui/map_with_unused_argument_over_ranges.rs:34:5
71+
|
72+
LL | (2..upper_fn()).map(|_| do_something());
73+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn() + 2)`
74+
75+
error: map of a trivial closure (not dependent on parameter) over a range
76+
--> tests/ui/map_with_unused_argument_over_ranges.rs:35:5
77+
|
78+
LL | (2..=upper_fn()).map(|_| do_something());
79+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn() + 3)`
80+
81+
error: map of a trivial closure (not dependent on parameter) over a range
82+
--> tests/ui/map_with_unused_argument_over_ranges.rs:14:9
83+
|
84+
LL | (0..10).map(|_| do_something());
85+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
86+
...
87+
LL | gen!();
88+
| ------ in this macro invocation
89+
|
90+
= note: this error originates in the macro `gen` (in Nightly builds, run with -Z macro-backtrace for more info)
91+
92+
error: map of a trivial closure (not dependent on parameter) over a range
93+
--> tests/ui/map_with_unused_argument_over_ranges.rs:41:5
94+
|
95+
LL | (2..upper_fn()).map(|_| do_something()); // Ranges not starting at zero not yet handled
96+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn() + 2)`
97+
98+
error: map of a trivial closure (not dependent on parameter) over a range
99+
--> tests/ui/map_with_unused_argument_over_ranges.rs:58:5
71100
|
72101
LL | (0..10).map(|_| do_something());
73102
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
74103

75-
error: aborting due to 10 previous errors
104+
error: aborting due to 14 previous errors
76105

0 commit comments

Comments
 (0)