Skip to content

Commit 9bd7fa0

Browse files
mcartonmcarton
committed
Improve NEEDLESS_RANGE_LOOP error reporting
1 parent e613c8b commit 9bd7fa0

File tree

2 files changed

+73
-30
lines changed

2 files changed

+73
-30
lines changed

‎clippy_lints/src/loops.rs

+21-20
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use std::borrow::Cow;
1313
use std::collections::HashMap;
1414
use syntax::ast;
1515

16-
use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro,
16+
use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, multispan_sugg, in_external_macro,
1717
span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher,
1818
walk_ptrs_ty};
1919
use utils::paths;
@@ -377,32 +377,33 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
377377
};
378378

379379
if visitor.nonindex {
380-
span_lint(cx,
381-
NEEDLESS_RANGE_LOOP,
382-
expr.span,
383-
&format!("the loop variable `{}` is used to index `{}`. Consider using `for ({}, \
384-
item) in {}.iter().enumerate(){}{}` or similar iterators",
385-
ident.node,
386-
indexed,
387-
ident.node,
388-
indexed,
389-
take,
390-
skip));
380+
span_lint_and_then(cx,
381+
NEEDLESS_RANGE_LOOP,
382+
expr.span,
383+
&format!("the loop variable `{}` is used to index `{}`", ident.node, indexed),
384+
|db| {
385+
multispan_sugg(db, "consider using an iterator".to_string(), &[
386+
(pat.span, &format!("({}, <item>)", ident.node)),
387+
(arg.span, &format!("{}.iter().enumerate(){}{}", indexed, take, skip)),
388+
]);
389+
});
391390
} else {
392391
let repl = if starts_at_zero && take.is_empty() {
393392
format!("&{}", indexed)
394393
} else {
395394
format!("{}.iter(){}{}", indexed, take, skip)
396395
};
397396

398-
span_lint(cx,
399-
NEEDLESS_RANGE_LOOP,
400-
expr.span,
401-
&format!("the loop variable `{}` is only used to index `{}`. \
402-
Consider using `for item in {}` or similar iterators",
403-
ident.node,
404-
indexed,
405-
repl));
397+
span_lint_and_then(cx,
398+
NEEDLESS_RANGE_LOOP,
399+
expr.span,
400+
&format!("the loop variable `{}` is only used to index `{}`.", ident.node, indexed),
401+
|db| {
402+
multispan_sugg(db, "consider using an iterator".to_string(), &[
403+
(pat.span, "<item>"),
404+
(arg.span, &repl),
405+
]);
406+
});
406407
}
407408
}
408409
}

‎tests/compile-fail/for_loop.rs

+52-10
Original file line numberDiff line numberDiff line change
@@ -96,66 +96,108 @@ fn main() {
9696
let mut vec = vec![1, 2, 3, 4];
9797
let vec2 = vec![1, 2, 3, 4];
9898
for i in 0..vec.len() {
99-
//~^ ERROR `i` is only used to index `vec`. Consider using `for item in &vec`
99+
//~^ ERROR `i` is only used to index `vec`
100+
//~| HELP consider
101+
//~| HELP consider
102+
//~| SUGGESTION for <item> in &vec {
100103
println!("{}", vec[i]);
101104
}
102105

106+
for i in 0..vec.len() { let _ = vec[i]; }
107+
//~^ ERROR `i` is only used to index `vec`
108+
//~| HELP consider
109+
//~| HELP consider
110+
//~| SUGGESTION for <item> in &vec { let _ = vec[i]; }
111+
103112
// ICE #746
104113
for j in 0..4 {
105114
//~^ ERROR `j` is only used to index `STATIC`
115+
//~| HELP consider
116+
//~| HELP consider
117+
//~| SUGGESTION for <item> in STATIC.iter().take(4) {
106118
println!("{:?}", STATIC[j]);
107119
}
108120

109121
for j in 0..4 {
110122
//~^ ERROR `j` is only used to index `CONST`
123+
//~| HELP consider
124+
//~| HELP consider
125+
//~| SUGGESTION for <item> in CONST.iter().take(4) {
111126
println!("{:?}", CONST[j]);
112127
}
113128

114129
for i in 0..vec.len() {
115-
//~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate()`
130+
//~^ ERROR `i` is used to index `vec`
131+
//~| HELP consider
132+
//~| HELP consider
133+
//~| SUGGESTION for (i, <item>) in vec.iter().enumerate() {
116134
println!("{} {}", vec[i], i);
117135
}
118136
for i in 0..vec.len() { // not an error, indexing more than one variable
119137
println!("{} {}", vec[i], vec2[i]);
120138
}
121139

122140
for i in 0..vec.len() {
123-
//~^ ERROR `i` is only used to index `vec2`. Consider using `for item in vec2.iter().take(vec.len())`
141+
//~^ ERROR `i` is only used to index `vec2`
142+
//~| HELP consider
143+
//~| HELP consider
144+
//~| SUGGESTION for <item> in vec2.iter().take(vec.len()) {
124145
println!("{}", vec2[i]);
125146
}
126147

127148
for i in 5..vec.len() {
128-
//~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().skip(5)`
149+
//~^ ERROR `i` is only used to index `vec`
150+
//~| HELP consider
151+
//~| HELP consider
152+
//~| SUGGESTION for <item> in vec.iter().skip(5) {
129153
println!("{}", vec[i]);
130154
}
131155

132156
for i in 0..MAX_LEN {
133-
//~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(MAX_LEN)`
157+
//~^ ERROR `i` is only used to index `vec`
158+
//~| HELP consider
159+
//~| HELP consider
160+
//~| SUGGESTION for <item> in vec.iter().take(MAX_LEN) {
134161
println!("{}", vec[i]);
135162
}
136163

137164
for i in 0...MAX_LEN {
138-
//~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(MAX_LEN)`
165+
//~^ ERROR `i` is only used to index `vec`
166+
//~| HELP consider
167+
//~| HELP consider
168+
//~| SUGGESTION for <item> in vec.iter().take(MAX_LEN) {
139169
println!("{}", vec[i]);
140170
}
141171

142172
for i in 5..10 {
143-
//~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
173+
//~^ ERROR `i` is only used to index `vec`
174+
//~| HELP consider
175+
//~| HELP consider
176+
//~| SUGGESTION for <item> in vec.iter().take(10).skip(5) {
144177
println!("{}", vec[i]);
145178
}
146179

147180
for i in 5...10 {
148-
//~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
181+
//~^ ERROR `i` is only used to index `vec`
182+
//~| HELP consider
183+
//~| HELP consider
184+
//~| SUGGESTION for <item> in vec.iter().take(10).skip(5) {
149185
println!("{}", vec[i]);
150186
}
151187

152188
for i in 5..vec.len() {
153-
//~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().skip(5)`
189+
//~^ ERROR `i` is used to index `vec`
190+
//~| HELP consider
191+
//~| HELP consider
192+
//~| SUGGESTION for (i, <item>) in vec.iter().enumerate().skip(5) {
154193
println!("{} {}", vec[i], i);
155194
}
156195

157196
for i in 5..10 {
158-
//~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().take(10).skip(5)`
197+
//~^ ERROR `i` is used to index `vec`
198+
//~| HELP consider
199+
//~| HELP consider
200+
//~| SUGGESTION for (i, <item>) in vec.iter().enumerate().take(10).skip(5) {
159201
println!("{} {}", vec[i], i);
160202
}
161203

0 commit comments

Comments
 (0)