Skip to content

Commit 6517985

Browse files
rspencer01rspencer01
committed
Use a multipart suggestion to more accuractly give a diagnostic
This lets us split our change into three parts: replacing the range with a repeat, removing the parameters, and adding the take. This makes it super explcicit and improves the quality of the suggestion.
1 parent 84c1dbc commit 6517985

File tree

3 files changed

+109
-24
lines changed

3 files changed

+109
-24
lines changed

‎clippy_lints/src/methods/map_with_unused_argument_over_ranges.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES;
22
use clippy_config::msrvs::{self, Msrv};
3-
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::diagnostics::span_lint_and_then;
44
use clippy_utils::higher;
55
use clippy_utils::source::snippet_with_applicability;
66
use rustc_ast::ast::RangeLimits;
@@ -9,6 +9,7 @@ use rustc_data_structures::packed::Pu128;
99
use rustc_errors::Applicability;
1010
use rustc_hir::{Body, Closure, Expr, ExprKind, PatKind};
1111
use rustc_lint::LateContext;
12+
use rustc_span::Span;
1213

1314
fn extract_count_with_applicability(
1415
cx: &LateContext<'_>,
@@ -49,7 +50,14 @@ fn extract_count_with_applicability(
4950
None
5051
}
5152

52-
pub(super) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, receiver: &Expr<'_>, arg: &Expr<'_>, msrv: &Msrv) {
53+
pub(super) fn check(
54+
cx: &LateContext<'_>,
55+
ex: &Expr<'_>,
56+
receiver: &Expr<'_>,
57+
arg: &Expr<'_>,
58+
msrv: &Msrv,
59+
method_call_span: Span,
60+
) {
5361
if !msrv.meets(msrvs::REPEAT_WITH) {
5462
return;
5563
}
@@ -63,16 +71,22 @@ pub(super) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, receiver: &Expr<'_>, ar
6371
{
6472
// TODO: Check if we can switch_to_eager_eval here and do away with `repeat_with` and instad use
6573
// `repeat`
66-
let snippet =
67-
snippet_with_applicability(cx, arg.span, "|| { ... }", &mut applicability).replacen("|_|", "||", 1);
68-
span_lint_and_sugg(
74+
span_lint_and_then(
6975
cx,
7076
MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
7177
ex.span,
7278
"map of a closure that does not depend on its parameter over a range",
73-
"use",
74-
format!("std::iter::repeat_with({snippet}).take({count})"),
75-
applicability,
79+
|diag| {
80+
diag.multipart_suggestion(
81+
"remove the explicit range and use `repeat_with` and `take`",
82+
vec![
83+
(receiver.span.to(method_call_span), "std::iter::repeat_with".to_owned()),
84+
(param.span, String::new()),
85+
(ex.span.shrink_to_hi(), format!(".take({count})")),
86+
],
87+
applicability,
88+
);
89+
},
7690
);
7791
}
7892
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4789,7 +4789,7 @@ impl Methods {
47894789
if name == "map" {
47904790
unused_enumerate_index::check(cx, expr, recv, m_arg);
47914791
map_clone::check(cx, expr, recv, m_arg, &self.msrv);
4792-
map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, &self.msrv);
4792+
map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, &self.msrv, span);
47934793
match method_call(recv) {
47944794
Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => {
47954795
iter_kv_map::check(cx, map_name, expr, recv2, m_arg, &self.msrv);

‎tests/ui/map_with_unused_argument_over_ranges.stderr

Lines changed: 86 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,63 @@ error: map of a closure that does not depend on its parameter over a range
22
--> tests/ui/map_with_unused_argument_over_ranges.rs:20:5
33
|
44
LL | (0..10).map(|_| do_something());
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::map-with-unused-argument-over-ranges` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::map_with_unused_argument_over_ranges)]`
9+
help: remove the explicit range and use `repeat_with` and `take`
10+
|
11+
LL - (0..10).map(|_| do_something());
12+
LL + std::iter::repeat_with(|| do_something()).take(10);
13+
|
914

1015
error: map of a closure that does not depend on its parameter over a range
1116
--> tests/ui/map_with_unused_argument_over_ranges.rs:21:5
1217
|
1318
LL | (0..=10).map(|_| do_something());
14-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(11)`
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
|
21+
help: remove the explicit range and use `repeat_with` and `take`
22+
|
23+
LL - (0..=10).map(|_| do_something());
24+
LL + std::iter::repeat_with(|| do_something()).take(11);
25+
|
1526

1627
error: map of a closure that does not depend on its parameter over a range
1728
--> tests/ui/map_with_unused_argument_over_ranges.rs:22:5
1829
|
1930
LL | (3..10).map(|_| do_something());
20-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(7)`
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
|
33+
help: remove the explicit range and use `repeat_with` and `take`
34+
|
35+
LL - (3..10).map(|_| do_something());
36+
LL + std::iter::repeat_with(|| do_something()).take(7);
37+
|
2138

2239
error: map of a closure that does not depend on its parameter over a range
2340
--> tests/ui/map_with_unused_argument_over_ranges.rs:23:5
2441
|
2542
LL | (3..=10).map(|_| do_something());
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(8)`
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44+
|
45+
help: remove the explicit range and use `repeat_with` and `take`
46+
|
47+
LL - (3..=10).map(|_| do_something());
48+
LL + std::iter::repeat_with(|| do_something()).take(8);
49+
|
2750

2851
error: map of a closure that does not depend on its parameter over a range
2952
--> tests/ui/map_with_unused_argument_over_ranges.rs:24:5
3053
|
3154
LL | (0..10).map(|_| 3);
32-
| ^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| 3).take(10)`
55+
| ^^^^^^^^^^^^^^^^^^
56+
|
57+
help: remove the explicit range and use `repeat_with` and `take`
58+
|
59+
LL - (0..10).map(|_| 3);
60+
LL + std::iter::repeat_with(|| 3).take(10);
61+
|
3362

3463
error: map of a closure that does not depend on its parameter over a range
3564
--> tests/ui/map_with_unused_argument_over_ranges.rs:25:5
@@ -40,55 +69,97 @@ LL | | x + 2
4069
LL | | });
4170
| |______^
4271
|
43-
help: use
72+
help: remove the explicit range and use `repeat_with` and `take`
4473
|
4574
LL ~ std::iter::repeat_with(|| {
46-
LL + let x = 3;
47-
LL + x + 2
75+
LL | let x = 3;
76+
LL | x + 2
4877
LL ~ }).take(10);
4978
|
5079

5180
error: map of a closure that does not depend on its parameter over a range
5281
--> tests/ui/map_with_unused_argument_over_ranges.rs:29:5
5382
|
5483
LL | (0..10).map(|_| do_something()).collect::<Vec<_>>();
55-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
84+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
85+
|
86+
help: remove the explicit range and use `repeat_with` and `take`
87+
|
88+
LL - (0..10).map(|_| do_something()).collect::<Vec<_>>();
89+
LL + std::iter::repeat_with(|| do_something()).take(10).collect::<Vec<_>>();
90+
|
5691

5792
error: map of a closure that does not depend on its parameter over a range
5893
--> tests/ui/map_with_unused_argument_over_ranges.rs:31:5
5994
|
6095
LL | (0..upper).map(|_| do_something());
61-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper)`
96+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
97+
|
98+
help: remove the explicit range and use `repeat_with` and `take`
99+
|
100+
LL - (0..upper).map(|_| do_something());
101+
LL + std::iter::repeat_with(|| do_something()).take(upper);
102+
|
62103

63104
error: map of a closure that does not depend on its parameter over a range
64105
--> tests/ui/map_with_unused_argument_over_ranges.rs:33:5
65106
|
66107
LL | (0..upper_fn()).map(|_| do_something());
67-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn())`
108+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
109+
|
110+
help: remove the explicit range and use `repeat_with` and `take`
111+
|
112+
LL - (0..upper_fn()).map(|_| do_something());
113+
LL + std::iter::repeat_with(|| do_something()).take(upper_fn());
114+
|
68115

69116
error: map of a closure that does not depend on its parameter over a range
70117
--> tests/ui/map_with_unused_argument_over_ranges.rs:34:5
71118
|
72119
LL | (0..=upper_fn()).map(|_| do_something());
73-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn() + 1)`
120+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
121+
|
122+
help: remove the explicit range and use `repeat_with` and `take`
123+
|
124+
LL - (0..=upper_fn()).map(|_| do_something());
125+
LL + std::iter::repeat_with(|| do_something()).take(upper_fn() + 1);
126+
|
74127

75128
error: map of a closure that does not depend on its parameter over a range
76129
--> tests/ui/map_with_unused_argument_over_ranges.rs:35:5
77130
|
78131
LL | (2..upper_fn()).map(|_| do_something());
79-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn() - 2)`
132+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
133+
|
134+
help: remove the explicit range and use `repeat_with` and `take`
135+
|
136+
LL - (2..upper_fn()).map(|_| do_something());
137+
LL + std::iter::repeat_with(|| do_something()).take(upper_fn() - 2);
138+
|
80139

81140
error: map of a closure that does not depend on its parameter over a range
82141
--> tests/ui/map_with_unused_argument_over_ranges.rs:36:5
83142
|
84143
LL | (2..=upper_fn()).map(|_| do_something());
85-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(upper_fn() - 1)`
144+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
145+
|
146+
help: remove the explicit range and use `repeat_with` and `take`
147+
|
148+
LL - (2..=upper_fn()).map(|_| do_something());
149+
LL + std::iter::repeat_with(|| do_something()).take(upper_fn() - 1);
150+
|
86151

87152
error: map of a closure that does not depend on its parameter over a range
88153
--> tests/ui/map_with_unused_argument_over_ranges.rs:58:5
89154
|
90155
LL | (0..10).map(|_| do_something());
91-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `std::iter::repeat_with(|| do_something()).take(10)`
156+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
157+
|
158+
help: remove the explicit range and use `repeat_with` and `take`
159+
|
160+
LL - (0..10).map(|_| do_something());
161+
LL + std::iter::repeat_with(|| do_something()).take(10);
162+
|
92163

93164
error: aborting due to 13 previous errors
94165

0 commit comments

Comments
 (0)