Skip to content
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

Deprecate unused_collect lint #4348

Merged
merged 1 commit into from
Aug 15, 2019
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 310 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 309 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
8 changes: 8 additions & 0 deletions clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,11 @@ declare_deprecated_lint! {
pub INVALID_REF,
"superseded by rustc lint `invalid_value`"
}

/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** This lint has been superseded by #[must_use] in rustc.
declare_deprecated_lint! {
pub UNUSED_COLLECT,
"`collect` has been marked as #[must_use] in rustc and that covers all cases of this lint"
}
6 changes: 4 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
"clippy::invalid_ref",
"superseded by rustc lint `invalid_value`",
);
store.register_removed(
"clippy::unused_collect",
"`collect` has been marked as #[must_use] in rustc and that covers all cases of this lint",
);
// end deprecated lints, do not remove this comment, it’s used in `update_lints`

reg.register_late_lint_pass(box serde_api::SerdeAPI);
Expand Down Expand Up @@ -761,7 +765,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
loops::NEEDLESS_RANGE_LOOP,
loops::NEVER_LOOP,
loops::REVERSE_RANGE_LOOP,
loops::UNUSED_COLLECT,
loops::WHILE_IMMUTABLE_CONDITION,
loops::WHILE_LET_LOOP,
loops::WHILE_LET_ON_ITERATOR,
Expand Down Expand Up @@ -1142,7 +1145,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
large_enum_variant::LARGE_ENUM_VARIANT,
loops::MANUAL_MEMCPY,
loops::NEEDLESS_COLLECT,
loops::UNUSED_COLLECT,
methods::EXPECT_FUN_CALL,
methods::ITER_NTH,
methods::OR_FUN_CALL,
Expand Down
38 changes: 0 additions & 38 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,24 +242,6 @@ declare_clippy_lint! {
"`loop { if let { ... } else break }`, which can be written as a `while let` loop"
}

declare_clippy_lint! {
/// **What it does:** Checks for using `collect()` on an iterator without using
/// the result.
///
/// **Why is this bad?** It is more idiomatic to use a `for` loop over the
/// iterator instead.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```ignore
/// vec.iter().map(|x| /* some operation returning () */).collect::<Vec<_>>();
/// ```
pub UNUSED_COLLECT,
perf,
"`collect()`ing an iterator without using the result; this is usually better written as a for loop"
}

declare_clippy_lint! {
/// **What it does:** Checks for functions collecting an iterator when collect
/// is not needed.
Expand Down Expand Up @@ -467,7 +449,6 @@ declare_lint_pass!(Loops => [
FOR_LOOP_OVER_RESULT,
FOR_LOOP_OVER_OPTION,
WHILE_LET_LOOP,
UNUSED_COLLECT,
NEEDLESS_COLLECT,
REVERSE_RANGE_LOOP,
EXPLICIT_COUNTER_LOOP,
Expand Down Expand Up @@ -602,25 +583,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {

check_needless_collect(expr, cx);
}

fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) {
if let StmtKind::Semi(ref expr) = stmt.node {
if let ExprKind::MethodCall(ref method, _, ref args) = expr.node {
if args.len() == 1
&& method.ident.name == sym!(collect)
&& match_trait_method(cx, expr, &paths::ITERATOR)
{
span_lint(
cx,
UNUSED_COLLECT,
expr.span,
"you are collect()ing an iterator and throwing away the result. \
Consider using an explicit for loop to exhaust the iterator",
);
}
}
}
}
}

enum NeverLoopResult {
Expand Down
9 changes: 1 addition & 8 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 310] = [
pub const ALL_LINTS: [Lint; 309] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -1967,13 +1967,6 @@ pub const ALL_LINTS: [Lint; 310] = [
deprecation: None,
module: "misc_early",
},
Lint {
name: "unused_collect",
group: "perf",
desc: "`collect()`ing an iterator without using the result; this is usually better written as a for loop",
deprecation: None,
module: "loops",
},
Lint {
name: "unused_io_amount",
group: "correctness",
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/deprecated.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ error: lint `clippy::misaligned_transmute` has been removed: `this lint has been
LL | #[warn(clippy::misaligned_transmute)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: lint `clippy::unused_collect` has been removed: ``collect` has been marked as #[must_use] in rustc and that covers all cases of this lint`
--> $DIR/deprecated.rs:6:8
|
LL | #[warn(clippy::unused_collect)]
| ^^^^^^^^^^^^^^^^^^^^^^

error: lint `clippy::invalid_ref` has been removed: `superseded by rustc lint `invalid_value``
--> $DIR/deprecated.rs:7:8
|
Expand All @@ -42,5 +48,5 @@ error: lint `clippy::str_to_string` has been removed: `using `str::to_string` is
LL | #[warn(clippy::str_to_string)]
| ^^^^^^^^^^^^^^^^^^^^^

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

1 change: 0 additions & 1 deletion tests/ui/deprecated_old.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
#[warn(unstable_as_slice)]
#[warn(unstable_as_mut_slice)]
#[warn(misaligned_transmute)]
#[warn(unused_collect)]

fn main() {}
8 changes: 1 addition & 7 deletions tests/ui/deprecated_old.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,11 @@ error: lint `misaligned_transmute` has been removed: `this lint has been split i
LL | #[warn(misaligned_transmute)]
| ^^^^^^^^^^^^^^^^^^^^

error: lint name `unused_collect` is deprecated and may not have an effect in the future. Also `cfg_attr(cargo-clippy)` won't be necessary anymore
--> $DIR/deprecated_old.rs:6:8
|
LL | #[warn(unused_collect)]
| ^^^^^^^^^^^^^^ help: change it to: `clippy::unused_collect`

error: lint `str_to_string` has been removed: `using `str::to_string` is common even today and specialization will likely happen soon`
--> $DIR/deprecated_old.rs:1:8
|
LL | #[warn(str_to_string)]
| ^^^^^^^^^^^^^

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

1 change: 0 additions & 1 deletion tests/ui/for_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ impl Unrelated {
clippy::reverse_range_loop,
clippy::for_kv_map
)]
#[warn(clippy::unused_collect)]
#[allow(
clippy::linkedlist,
clippy::shadow_unrelated,
Expand Down
52 changes: 22 additions & 30 deletions tests/ui/for_loop.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this range is empty so this for loop will never run
--> $DIR/for_loop.rs:40:14
--> $DIR/for_loop.rs:39:14
|
LL | for i in 10..0 {
| ^^^^^
Expand All @@ -11,7 +11,7 @@ LL | for i in (0..10).rev() {
| ^^^^^^^^^^^^^

error: this range is empty so this for loop will never run
--> $DIR/for_loop.rs:44:14
--> $DIR/for_loop.rs:43:14
|
LL | for i in 10..=0 {
| ^^^^^^
Expand All @@ -21,7 +21,7 @@ LL | for i in (0...10).rev() {
| ^^^^^^^^^^^^^^

error: this range is empty so this for loop will never run
--> $DIR/for_loop.rs:48:14
--> $DIR/for_loop.rs:47:14
|
LL | for i in MAX_LEN..0 {
| ^^^^^^^^^^
Expand All @@ -31,13 +31,13 @@ LL | for i in (0..MAX_LEN).rev() {
| ^^^^^^^^^^^^^^^^^^

error: this range is empty so this for loop will never run
--> $DIR/for_loop.rs:52:14
--> $DIR/for_loop.rs:51:14
|
LL | for i in 5..5 {
| ^^^^

error: this range is empty so this for loop will never run
--> $DIR/for_loop.rs:77:14
--> $DIR/for_loop.rs:76:14
|
LL | for i in 10..5 + 4 {
| ^^^^^^^^^
Expand All @@ -47,7 +47,7 @@ LL | for i in (5 + 4..10).rev() {
| ^^^^^^^^^^^^^^^^^

error: this range is empty so this for loop will never run
--> $DIR/for_loop.rs:81:14
--> $DIR/for_loop.rs:80:14
|
LL | for i in (5 + 2)..(3 - 1) {
| ^^^^^^^^^^^^^^^^
Expand All @@ -57,108 +57,100 @@ LL | for i in ((3 - 1)..(5 + 2)).rev() {
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: this range is empty so this for loop will never run
--> $DIR/for_loop.rs:85:14
--> $DIR/for_loop.rs:84:14
|
LL | for i in (5 + 2)..(8 - 1) {
| ^^^^^^^^^^^^^^^^

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

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

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

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

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

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

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

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

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

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

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

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

error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:147:15
--> $DIR/for_loop.rs:146:15
|
LL | 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
--> $DIR/for_loop.rs:149:15
--> $DIR/for_loop.rs:148:15
|
LL | for _v in vec.iter().next() {}
| ^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::iter-next-loop` implied by `-D warnings`

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

error: aborting due to 22 previous errors
error: aborting due to 21 previous errors

10 changes: 1 addition & 9 deletions tests/ui/infinite_iter.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
error: you are collect()ing an iterator and throwing away the result. Consider using an explicit for loop to exhaust the iterator
--> $DIR/infinite_iter.rs:10:5
|
LL | repeat(0_u8).collect::<Vec<_>>(); // infinite iter
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unused-collect` implied by `-D warnings`

error: infinite iteration detected
--> $DIR/infinite_iter.rs:10:5
|
Expand Down Expand Up @@ -113,5 +105,5 @@ LL | let _: HashSet<i32> = (0..).collect(); // Infinite iter
|
= note: `#[deny(clippy::infinite_iter)]` on by default

error: aborting due to 15 previous errors
error: aborting due to 14 previous errors