new lint: using collect() to just exhaust an iterator

Should use a for loop instead.
This commit is contained in:
Georg Brandl 2015-08-30 13:10:59 +02:00
parent b72ef5a173
commit 16df79a054
4 changed files with 27 additions and 2 deletions

View file

@ -4,7 +4,7 @@
A collection of lints that give helpful tips to newbies and catch oversights.
##Lints
There are 50 lints included in this crate:
There are 51 lints included in this crate:
name | default | meaning
-----------------------------------------------------------------------------------------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@ -56,6 +56,7 @@ name
[toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg) | warn | a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`)
[type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types; recommends factoring out parts into `type` definitions
[unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp) | warn | comparing unit values (which is always `true` or `false`, respectively)
[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
[zero_width_space](https://github.com/Manishearth/rust-clippy/wiki#zero_width_space) | deny | using a zero-width space in a string literal, which is confusing

View file

@ -96,6 +96,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
loops::EXPLICIT_ITER_LOOP,
loops::ITER_NEXT_LOOP,
loops::NEEDLESS_RANGE_LOOP,
loops::UNUSED_COLLECT,
loops::WHILE_LET_LOOP,
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH,

View file

@ -20,13 +20,17 @@ declare_lint!{ pub ITER_NEXT_LOOP, Warn,
declare_lint!{ pub WHILE_LET_LOOP, Warn,
"`loop { if let { ... } else break }` can be written as a `while let` loop" }
declare_lint!{ pub UNUSED_COLLECT, Warn,
"`collect()`ing an iterator without using the result; this is usually better \
written as a for loop" }
#[derive(Copy, Clone)]
pub struct LoopsPass;
impl LintPass for LoopsPass {
fn get_lints(&self) -> LintArray {
lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP,
WHILE_LET_LOOP)
WHILE_LET_LOOP, UNUSED_COLLECT)
}
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
@ -112,6 +116,20 @@ impl LintPass for LoopsPass {
}
}
}
fn check_stmt(&mut self, cx: &Context, stmt: &Stmt) {
if let StmtSemi(ref expr, _) = stmt.node {
if let ExprMethodCall(ref method, _, ref args) = expr.node {
if args.len() == 1 && method.node.name == "collect" {
if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) {
span_lint(cx, UNUSED_COLLECT, expr.span, &format!(
"you are collect()ing an iterator and throwing away the result. \
Consider using an explicit for loop to exhaust the iterator"));
}
}
}
}
}
}
/// Recover the essential nodes of a desugared for loop:

View file

@ -15,6 +15,7 @@ impl Unrelated {
}
#[deny(needless_range_loop, explicit_iter_loop, iter_next_loop)]
#[deny(unused_collect)]
#[allow(linkedlist)]
fn main() {
let mut vec = vec![1, 2, 3, 4];
@ -56,4 +57,8 @@ fn main() {
let u = Unrelated(vec![]);
for _v in u.next() { } // no error
for _v in u.iter() { } // no error
let mut out = vec![];
vec.iter().map(|x| out.push(x)).collect::<Vec<_>>(); //~ERROR you are collect()ing an iterator
let _y = vec.iter().map(|x| out.push(x)).collect::<Vec<_>>(); // this is fine
}