Suggest for loop instead of while-let when looping over iterators

This commit is contained in:
Florian Hartwig 2015-10-16 20:27:13 +02:00
parent 3fe6ca9def
commit f6163fce61
4 changed files with 37 additions and 3 deletions

View file

@ -6,7 +6,8 @@ A collection of lints to catch common mistakes and improve your Rust code.
[Jump to usage instructions](#usage)
##Lints
There are 68 lints included in this crate:
There are 69 lints included in this crate:
There are 65 lints included in this crate:
name | default | meaning
-------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@ -74,6 +75,7 @@ name
[unstable_as_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice) | warn | as_slice is not stable and can be replaced by & v[..]see https://github.com/rust-lang/rust/issues/27729
[unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect) | warn | `collect()`ing an iterator without using the result; this is usually better written as a for loop
[while_let_loop](https://github.com/Manishearth/rust-clippy/wiki#while_let_loop) | warn | `loop { if let { ... } else break }` can be written as a `while let` loop
[while_let_on_iterator](https://github.com/Manishearth/rust-clippy/wiki#while_let_on_iterator) | warn | using a while-let loop instead of a for loop on an iterator
[wrong_pub_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_pub_self_convention) | allow | defining a public method named with an established prefix (like "into_") that takes `self` with the wrong convention
[wrong_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_self_convention) | warn | defining a method named with an established prefix (like "into_") that takes `self` with the wrong convention
[zero_divided_by_zero](https://github.com/Manishearth/rust-clippy/wiki#zero_divided_by_zero) | warn | usage of `0.0 / 0.0` to obtain NaN instead of std::f32::NaN or std::f64::NaN

View file

@ -138,6 +138,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
loops::REVERSE_RANGE_LOOP,
loops::UNUSED_COLLECT,
loops::WHILE_LET_LOOP,
loops::WHILE_LET_ON_ITERATOR,
matches::MATCH_BOOL,
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH,

View file

@ -38,6 +38,8 @@ declare_lint!{ pub EXPLICIT_COUNTER_LOOP, Warn,
declare_lint!{ pub EMPTY_LOOP, Warn, "empty `loop {}` detected" }
declare_lint!{ pub WHILE_LET_ON_ITERATOR, Warn, "using a while-let loop instead of a for loop on an iterator" }
#[derive(Copy, Clone)]
pub struct LoopsPass;
@ -45,7 +47,8 @@ impl LintPass for LoopsPass {
fn get_lints(&self) -> LintArray {
lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP,
WHILE_LET_LOOP, UNUSED_COLLECT, REVERSE_RANGE_LOOP,
EXPLICIT_COUNTER_LOOP, EMPTY_LOOP)
EXPLICIT_COUNTER_LOOP, EMPTY_LOOP,
WHILE_LET_ON_ITERATOR)
}
}
@ -228,6 +231,17 @@ impl LateLintPass for LoopsPass {
}
}
}
if let ExprMatch(ref expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
let pat = &arms[0].pats[0].node;
if let (&PatEnum(ref path, _), &ExprMethodCall(method_name, _, _)) = (pat, &expr.node) {
if method_name.node.as_str() == "next" &&
match_trait_method(cx, expr, &["core", "iter", "Iterator"]) &&
path.segments.last().unwrap().identifier.name.as_str() == "Some" {
span_lint(cx, WHILE_LET_ON_ITERATOR, expr.span,
"this loop could be written as a `for` loop");
}
}
}
}
fn check_stmt(&mut self, cx: &LateContext, stmt: &Stmt) {

View file

@ -1,7 +1,7 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(while_let_loop, empty_loop)]
#![deny(while_let_loop, empty_loop, while_let_on_iterator)]
#![allow(dead_code, unused)]
fn main() {
@ -53,6 +53,23 @@ fn main() {
while let Some(x) = y { // no error, obviously
println!("{}", x);
}
while let Option::Some(x) = (1..20).next() { //~ERROR this loop could be written as a `for` loop
println!("{}", x);
}
while let Some(x) = (1..20).next() { //~ERROR this loop could be written as a `for` loop
println!("{}", x);
}
while let Some(_) = (1..20).next() {} //~ERROR this loop could be written as a `for` loop
while let None = (1..20).next() {} // this is fine (if nonsensical)
if let Some(x) = (1..20).next() { // also fine
println!("{}", x)
}
}
// regression test (#360)