Skip to content

Commit f2f6637

Browse files
mcartonmcarton
authored
Merge pull request #1486 from killercup/for-loop-suggestions
Add suggestions to `EXPLICIT_[INTO_]ITER_LOOP`
2 parents d825f19 + 2357dfe commit f2f6637

File tree

2 files changed

+86
-36
lines changed

2 files changed

+86
-36
lines changed

‎clippy_lints/src/loops.rs

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -588,28 +588,36 @@ fn check_for_loop_arg(cx: &LateContext, pat: &Pat, arg: &Expr, expr: &Expr) {
588588
if &*method_name.as_str() == "iter" || &*method_name.as_str() == "iter_mut" {
589589
if is_ref_iterable_type(cx, &args[0]) {
590590
let object = snippet(cx, args[0].span, "_");
591-
span_lint(cx,
592-
EXPLICIT_ITER_LOOP,
593-
expr.span,
594-
&format!("it is more idiomatic to loop over `&{}{}` instead of `{}.{}()`",
595-
if &*method_name.as_str() == "iter_mut" {
596-
"mut "
597-
} else {
598-
""
599-
},
600-
object,
601-
object,
602-
method_name));
591+
let suggestion = format!("&{}{}",
592+
if &*method_name.as_str() == "iter_mut" {
593+
"mut "
594+
} else {
595+
""
596+
},
597+
object);
598+
span_lint_and_then(cx,
599+
EXPLICIT_ITER_LOOP,
600+
arg.span,
601+
&format!("it is more idiomatic to loop over `{}` instead of `{}.{}()`",
602+
suggestion,
603+
object,
604+
method_name),
605+
|db| {
606+
db.span_suggestion(arg.span, "to write this more concisely, try looping over", suggestion);
607+
});
603608
}
604609
} else if &*method_name.as_str() == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) {
605610
let object = snippet(cx, args[0].span, "_");
606-
span_lint(cx,
607-
EXPLICIT_INTO_ITER_LOOP,
608-
expr.span,
609-
&format!("it is more idiomatic to loop over `{}` instead of `{}.{}()`",
610-
object,
611-
object,
612-
method_name));
611+
span_lint_and_then(cx,
612+
EXPLICIT_INTO_ITER_LOOP,
613+
arg.span,
614+
&format!("it is more idiomatic to loop over `{}` instead of `{}.{}()`",
615+
object,
616+
object,
617+
method_name),
618+
|db| {
619+
db.span_suggestion(arg.span, "to write this more concisely, try looping over", object.to_string());
620+
});
613621

614622
} else if &*method_name.as_str() == "next" && match_trait_method(cx, arg, &paths::ITERATOR) {
615623
span_lint(cx,

‎tests/compile-fail/for_loop.rs

Lines changed: 59 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -286,39 +286,81 @@ fn main() {
286286
id_col[i] = 1f64;
287287
}
288288

289-
/*
290-
for i in (10..0).map(|x| x * 2) {
291-
println!("{}", i);
292-
}*/
293-
294-
for _v in vec.iter() { } //~ERROR it is more idiomatic to loop over `&vec`
295-
for _v in vec.iter_mut() { } //~ERROR it is more idiomatic to loop over `&mut vec`
289+
for _v in vec.iter() { }
290+
//~^ ERROR it is more idiomatic to loop over `&vec`
291+
//~| HELP to write this more concisely, try looping over
292+
//~| SUGGESTION for _v in &vec {
296293

294+
for _v in vec.iter_mut() { }
295+
//~^ ERROR it is more idiomatic to loop over `&mut vec`
296+
//~| HELP to write this more concisely, try looping over
297+
//~| SUGGESTION for _v in &mut vec {
297298

298299
let out_vec = vec![1,2,3];
299-
for _v in out_vec.into_iter() { } //~ERROR it is more idiomatic to loop over `out_vec` instead of `out_vec.into_iter()`
300+
for _v in out_vec.into_iter() { }
301+
//~^ ERROR it is more idiomatic to loop over `out_vec` instead of `out_vec.into_iter()`
302+
//~| HELP to write this more concisely, try looping over
303+
//~| SUGGESTION for _v in out_vec {
300304

301305
for _v in &vec { } // these are fine
302306
for _v in &mut vec { } // these are fine
303307

304-
for _v in [1, 2, 3].iter() { } //~ERROR it is more idiomatic to loop over `&[
308+
for _v in [1, 2, 3].iter() { }
309+
//~^ ERROR it is more idiomatic to loop over `&[
310+
//~| HELP to write this more concisely, try looping over
311+
//~| SUGGESTION for _v in &[1, 2, 3] {
312+
305313
for _v in (&mut [1, 2, 3]).iter() { } // no error
306-
for _v in [0; 32].iter() {} //~ERROR it is more idiomatic to loop over `&[
314+
315+
for _v in [0; 32].iter() {}
316+
//~^ ERROR it is more idiomatic to loop over `&[
317+
//~| HELP to write this more concisely, try looping over
318+
//~| SUGGESTION for _v in &[0; 32] {
319+
307320
for _v in [0; 33].iter() {} // no error
321+
308322
let ll: LinkedList<()> = LinkedList::new();
309-
for _v in ll.iter() { } //~ERROR it is more idiomatic to loop over `&ll`
323+
for _v in ll.iter() { }
324+
//~^ ERROR it is more idiomatic to loop over `&ll`
325+
//~| HELP to write this more concisely, try looping over
326+
//~| SUGGESTION for _v in &ll {
327+
310328
let vd: VecDeque<()> = VecDeque::new();
311-
for _v in vd.iter() { } //~ERROR it is more idiomatic to loop over `&vd`
329+
for _v in vd.iter() { }
330+
//~^ ERROR it is more idiomatic to loop over `&vd`
331+
//~| HELP to write this more concisely, try looping over
332+
//~| SUGGESTION for _v in &vd {
333+
312334
let bh: BinaryHeap<()> = BinaryHeap::new();
313-
for _v in bh.iter() { } //~ERROR it is more idiomatic to loop over `&bh`
335+
for _v in bh.iter() { }
336+
//~^ ERROR it is more idiomatic to loop over `&bh`
337+
//~| HELP to write this more concisely, try looping over
338+
//~| SUGGESTION for _v in &bh {
339+
314340
let hm: HashMap<(), ()> = HashMap::new();
315-
for _v in hm.iter() { } //~ERROR it is more idiomatic to loop over `&hm`
341+
for _v in hm.iter() { }
342+
//~^ ERROR it is more idiomatic to loop over `&hm`
343+
//~| HELP to write this more concisely, try looping over
344+
//~| SUGGESTION for _v in &hm {
345+
316346
let bt: BTreeMap<(), ()> = BTreeMap::new();
317-
for _v in bt.iter() { } //~ERROR it is more idiomatic to loop over `&bt`
347+
for _v in bt.iter() { }
348+
//~^ ERROR it is more idiomatic to loop over `&bt`
349+
//~| HELP to write this more concisely, try looping over
350+
//~| SUGGESTION for _v in &bt {
351+
318352
let hs: HashSet<()> = HashSet::new();
319-
for _v in hs.iter() { } //~ERROR it is more idiomatic to loop over `&hs`
353+
for _v in hs.iter() { }
354+
//~^ ERROR it is more idiomatic to loop over `&hs`
355+
//~| HELP to write this more concisely, try looping over
356+
//~| SUGGESTION for _v in &hs {
357+
320358
let bs: BTreeSet<()> = BTreeSet::new();
321-
for _v in bs.iter() { } //~ERROR it is more idiomatic to loop over `&bs`
359+
for _v in bs.iter() { }
360+
//~^ ERROR it is more idiomatic to loop over `&bs`
361+
//~| HELP to write this more concisely, try looping over
362+
//~| SUGGESTION for _v in &bs {
363+
322364

323365
for _v in vec.iter().next() { } //~ERROR you are iterating over `Iterator::next()`
324366

0 commit comments

Comments
 (0)