Deprecate unused_collect lint

I found this because we only had two test cases in total for this lint. It turns out the functionality is fully covered by rustc these days. [Playground Examples](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=eb8ee6db389c77180c9fb152d3c608f4) changelog: Deprecate `unused_collect` lint. This is fully covered by rustc's `#[must_use]` on `collect` cc rust-lang#2846
phansch · Aug 7, 2019 · 2aa71a8 · 2aa71a8
1 parent ea26a95
commit 2aa71a8
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 90 deletions.
diff --git a/README.md b/README.md
@@ -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:
 

diff --git a/clippy_lints/src/deprecated_lints.rs b/clippy_lints/src/deprecated_lints.rs
@@ -113,3 +113,8 @@ declare_deprecated_lint! {
     pub UNSAFE_VECTOR_INITIALIZATION,
     "the replacement suggested by this lint had substantially different behavior"
 }
+
+declare_deprecated_lint! {
+    pub UNUSED_COLLECT,
+    "`collect` has been marked as #[must_use] in rustc and that covers all cases of this lint"
+}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -430,6 +430,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         "unsafe_vector_initialization",
         "the replacement suggested by this lint had substantially different behavior",
     );
+    store.register_removed(
+        "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);
@@ -758,7 +762,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,
@@ -1138,7 +1141,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,

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
@@ -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.
@@ -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,
@@ -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 {

diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
@@ -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",
@@ -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",

diff --git a/tests/ui/deprecated.rs b/tests/ui/deprecated.rs
@@ -3,5 +3,6 @@
 #[warn(unstable_as_slice)]
 #[warn(unstable_as_mut_slice)]
 #[warn(misaligned_transmute)]
+#[warn(unused_collect)]
 
 fn main() {}
diff --git a/tests/ui/deprecated.stderr b/tests/ui/deprecated.stderr
@@ -30,11 +30,17 @@ error: lint `misaligned_transmute` has been removed: `this lint has been split i
 LL | #[warn(misaligned_transmute)]
    |        ^^^^^^^^^^^^^^^^^^^^
 
+error: lint `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(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.rs:1:8
    |
 LL | #[warn(str_to_string)]
    |        ^^^^^^^^^^^^^
 
-error: aborting due to 6 previous errors
+error: aborting due to 7 previous errors
 
diff --git a/tests/ui/for_loop.rs b/tests/ui/for_loop.rs
@@ -24,7 +24,6 @@ impl Unrelated {
     clippy::reverse_range_loop,
     clippy::for_kv_map
 )]
-#[warn(clippy::unused_collect)]
 #[allow(
     clippy::linkedlist,
     clippy::shadow_unrelated,

diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr
@@ -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 {
    |              ^^^^^
@@ -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 {
    |              ^^^^^^
@@ -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 {
    |              ^^^^^^^^^^
@@ -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 {
    |              ^^^^^^^^^
@@ -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) {
    |              ^^^^^^^^^^^^^^^^
@@ -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
 
diff --git a/tests/ui/infinite_iter.stderr b/tests/ui/infinite_iter.stderr
@@ -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
    |
@@ -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