Skip to content

Fix the bug remaining in #1586. #1871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ fn contains_continue_decl(decl: &Decl, dest: &NodeId) -> bool {

fn contains_continue_expr(expr: &Expr, dest: &NodeId) -> bool {
match expr.node {
ExprRet(Some(ref e)) |
ExprBox(ref e) |
ExprUnary(_, ref e) |
ExprCast(ref e, _) |
Expand Down
6 changes: 1 addition & 5 deletions clippy_tests/examples/for_loop.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(plugin, step_by, inclusive_range_syntax)]
#![feature(plugin, inclusive_range_syntax)]
#![plugin(clippy)]

use std::collections::*;
Expand Down Expand Up @@ -189,10 +189,6 @@ fn main() {
println!("{}", i);
}

for i in (10..8).step_by(-1) {
println!("{}", i);
}

let x = 42;
for i in x..10 { // no error, not constant-foldable
println!("{}", i);
Expand Down
122 changes: 57 additions & 65 deletions clippy_tests/examples/for_loop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -292,183 +292,175 @@ error: this range is empty so this for loop will never run
186 | | }
| |_____^

error: use of deprecated item: replaced by `Iterator::step_by`
--> for_loop.rs:192:22
|
192 | for i in (10..8).step_by(-1) {
| ^^^^^^^
|
= note: `-D deprecated` implied by `-D warnings`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:207:15
--> for_loop.rs:203:15
|
207 | for _v in vec.iter() { }
203 | for _v in vec.iter() { }
| ^^^^^^^^^^ help: to write this more concisely, try `&vec`
|
= note: `-D explicit-iter-loop` implied by `-D warnings`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:209:15
--> for_loop.rs:205:15
|
209 | for _v in vec.iter_mut() { }
205 | for _v in vec.iter_mut() { }
| ^^^^^^^^^^^^^^ help: to write this more concisely, try `&mut vec`

error: it is more idiomatic to loop over containers instead of using explicit iteration methods`
--> for_loop.rs:212:15
--> for_loop.rs:208:15
|
212 | for _v in out_vec.into_iter() { }
208 | for _v in out_vec.into_iter() { }
| ^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try `out_vec`
|
= note: `-D explicit-into-iter-loop` implied by `-D warnings`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:215:15
--> for_loop.rs:211:15
|
215 | for _v in array.into_iter() {}
211 | for _v in array.into_iter() {}
| ^^^^^^^^^^^^^^^^^ help: to write this more concisely, try `&array`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:220:15
--> for_loop.rs:216:15
|
220 | for _v in [1, 2, 3].iter() { }
216 | for _v in [1, 2, 3].iter() { }
| ^^^^^^^^^^^^^^^^ help: to write this more concisely, try `&[1, 2, 3]`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:224:15
--> for_loop.rs:220:15
|
224 | for _v in [0; 32].iter() {}
220 | for _v in [0; 32].iter() {}
| ^^^^^^^^^^^^^^ help: to write this more concisely, try `&[0; 32]`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:229:15
--> for_loop.rs:225:15
|
229 | for _v in ll.iter() { }
225 | for _v in ll.iter() { }
| ^^^^^^^^^ help: to write this more concisely, try `&ll`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:232:15
--> for_loop.rs:228:15
|
232 | for _v in vd.iter() { }
228 | for _v in vd.iter() { }
| ^^^^^^^^^ help: to write this more concisely, try `&vd`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:235:15
--> for_loop.rs:231:15
|
235 | for _v in bh.iter() { }
231 | for _v in bh.iter() { }
| ^^^^^^^^^ help: to write this more concisely, try `&bh`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:238:15
--> for_loop.rs:234:15
|
238 | for _v in hm.iter() { }
234 | for _v in hm.iter() { }
| ^^^^^^^^^ help: to write this more concisely, try `&hm`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:241:15
--> for_loop.rs:237:15
|
241 | for _v in bt.iter() { }
237 | for _v in bt.iter() { }
| ^^^^^^^^^ help: to write this more concisely, try `&bt`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:244:15
--> for_loop.rs:240:15
|
244 | for _v in hs.iter() { }
240 | for _v in hs.iter() { }
| ^^^^^^^^^ help: to write this more concisely, try `&hs`

error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
--> for_loop.rs:247:15
--> for_loop.rs:243:15
|
247 | for _v in bs.iter() { }
243 | for _v in bs.iter() { }
| ^^^^^^^^^ help: to write this more concisely, try `&bs`

error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want
--> for_loop.rs:249:5
--> for_loop.rs:245:5
|
249 | for _v in vec.iter().next() { }
245 | for _v in vec.iter().next() { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you are collect()ing an iterator and throwing away the result. Consider using an explicit for loop to exhaust the iterator
--> for_loop.rs:256:5
--> for_loop.rs:252:5
|
256 | vec.iter().map(|x| out.push(x)).collect::<Vec<_>>();
252 | vec.iter().map(|x| out.push(x)).collect::<Vec<_>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D unused-collect` implied by `-D warnings`

error: the variable `_index` is used as a loop counter. Consider using `for (_index, item) in &vec.enumerate()` or similar iterators
--> for_loop.rs:261:5
--> for_loop.rs:257:5
|
261 | for _v in &vec { _index += 1 }
257 | for _v in &vec { _index += 1 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D explicit-counter-loop` implied by `-D warnings`

error: the variable `_index` is used as a loop counter. Consider using `for (_index, item) in &vec.enumerate()` or similar iterators
--> for_loop.rs:265:5
--> for_loop.rs:261:5
|
265 | for _v in &vec { _index += 1 }
261 | for _v in &vec { _index += 1 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: you seem to want to iterate on a map's values
--> for_loop.rs:325:5
--> for_loop.rs:321:5
|
325 | / for (_, v) in &m {
326 | | let _v = v;
327 | | }
321 | / for (_, v) in &m {
322 | | let _v = v;
323 | | }
| |_____^
|
= note: `-D for-kv-map` implied by `-D warnings`
help: use the corresponding method
| for v in m.values() {

error: you seem to want to iterate on a map's values
--> for_loop.rs:330:5
--> for_loop.rs:326:5
|
330 | / for (_, v) in &*m {
331 | | let _v = v;
332 | | // Here the `*` is not actually necesarry, but the test tests that we don't suggest
333 | | // `in *m.values()` as we used to
334 | | }
326 | / for (_, v) in &*m {
327 | | let _v = v;
328 | | // Here the `*` is not actually necesarry, but the test tests that we don't suggest
329 | | // `in *m.values()` as we used to
330 | | }
| |_____^
|
help: use the corresponding method
| for v in (*m).values() {

error: you seem to want to iterate on a map's values
--> for_loop.rs:337:5
--> for_loop.rs:333:5
|
337 | / for (_, v) in &mut m {
338 | | let _v = v;
339 | | }
333 | / for (_, v) in &mut m {
334 | | let _v = v;
335 | | }
| |_____^
|
help: use the corresponding method
| for v in m.values_mut() {

error: you seem to want to iterate on a map's values
--> for_loop.rs:342:5
--> for_loop.rs:338:5
|
342 | / for (_, v) in &mut *m {
343 | | let _v = v;
344 | | }
338 | / for (_, v) in &mut *m {
339 | | let _v = v;
340 | | }
| |_____^
|
help: use the corresponding method
| for v in (*m).values_mut() {

error: you seem to want to iterate on a map's keys
--> for_loop.rs:348:5
--> for_loop.rs:344:5
|
348 | / for (k, _value) in rm {
349 | | let _k = k;
350 | | }
344 | / for (k, _value) in rm {
345 | | let _k = k;
346 | | }
| |_____^
|
help: use the corresponding method
| for k in rm.keys() {

error: aborting due to 52 previous errors
error: aborting due to 51 previous errors


To learn more, run the command again with --verbose.
10 changes: 10 additions & 0 deletions clippy_tests/examples/never_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ fn test10() {
}
}

fn test11<F: FnMut() -> i32>(mut f: F) {
loop {
return match f() {
1 => continue,
_ => (),
}
}
}

fn main() {
test1();
test2();
Expand All @@ -114,5 +123,6 @@ fn main() {
test8();
test9();
test10();
test11(|| 0);
}

11 changes: 5 additions & 6 deletions clippy_tests/examples/range.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![feature(iterator_step_by)]
#![feature(step_by)]
#![feature(inclusive_range_syntax)]
#![feature(plugin)]
#![plugin(clippy)]
Expand All @@ -11,15 +10,15 @@ impl NotARange {

#[warn(iterator_step_by_zero, range_zip_with_len)]
fn main() {
(0..1).step_by(0);
let _ = (0..1).step_by(0);
// No warning for non-zero step
(0..1).step_by(1);
let _ = (0..1).step_by(1);

(1..).step_by(0);
(1...2).step_by(0);
let _ = (1..).step_by(0);
let _ = (1...2).step_by(0);

let x = 0..1;
x.step_by(0);
let _ = x.step_by(0);

// No error, not a range.
let y = NotARange;
Expand Down
52 changes: 22 additions & 30 deletions clippy_tests/examples/range.stderr
Original file line number Diff line number Diff line change
@@ -1,52 +1,44 @@
error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:14:12
|
14 | (0..1).step_by(0);
| ^^^^^^^
error: Iterator::step_by(0) will panic at runtime
--> range.rs:13:13
|
= note: `-D deprecated` implied by `-D warnings`

error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:16:12
13 | let _ = (0..1).step_by(0);
| ^^^^^^^^^^^^^^^^^
|
16 | (0..1).step_by(1);
| ^^^^^^^
= note: `-D iterator-step-by-zero` implied by `-D warnings`

error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:18:11
error: Iterator::step_by(0) will panic at runtime
--> range.rs:17:13
|
18 | (1..).step_by(0);
| ^^^^^^^
17 | let _ = (1..).step_by(0);
| ^^^^^^^^^^^^^^^^

error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:19:13
error: Iterator::step_by(0) will panic at runtime
--> range.rs:18:13
|
19 | (1...2).step_by(0);
| ^^^^^^^
18 | let _ = (1...2).step_by(0);
| ^^^^^^^^^^^^^^^^^^

error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:22:7
error: Iterator::step_by(0) will panic at runtime
--> range.rs:21:13
|
22 | x.step_by(0);
| ^^^^^^^
21 | let _ = x.step_by(0);
| ^^^^^^^^^^^^

error: It is more idiomatic to use v1.iter().enumerate()
--> range.rs:30:14
--> range.rs:29:14
|
30 | let _x = v1.iter().zip(0..v1.len());
29 | let _x = v1.iter().zip(0..v1.len());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D range-zip-with-len` implied by `-D warnings`

error: Iterator::step_by(0) will panic at runtime
--> range.rs:34:13
--> range.rs:33:13
|
34 | let _ = v1.iter().step_by(2/3);
33 | let _ = v1.iter().step_by(2/3);
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D iterator-step-by-zero` implied by `-D warnings`

error: aborting due to 7 previous errors
error: aborting due to 6 previous errors


To learn more, run the command again with --verbose.