rust-lang / rust-lang/rust-clippy

needless_collect warning, when it's actually needed

Open
#7,900 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

C-bug I-false-positive I-suggestion-causes-error L-nursery
Dominant language
Rust
Stars
13.5k
Forks
2.2k
Avg merge
2d 10h
Merged PRs (30d)
32

Description

Lint name: needless_collect

I tried this code:

pub fn extract<T: IntoIterator>(
    into_iter: T,
    index: usize,
) -> (Option<T::Item>, impl Iterator<Item = T::Item>) {
    let mut iter = into_iter.into_iter();
    let head = iter.by_ref().take(index).collect::<Vec<_>>();
    (iter.next(), head.into_iter().chain(iter))
}

I expected to see no warnings:
Because If I want to read element as index, I need to first read all elements before and store them somewhere.
So I think collect is appropriate here.

Instead, this happened:

warning: avoid using `collect()` when not needed
  --> itertools/src/lib.rs:47:42
   |
47 |     let head = iter.by_ref().take(index).collect::<Vec<_>>();
   |                                          ^^^^^^^
48 |     (iter.next(), head.into_iter().chain(iter))
   |                   ---------------- the iterator could be used here instead
   |
   = note: `#[warn(clippy::needless_collect)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
help: use the original Iterator instead of collecting it and then producing a new one
   |
47 ~     
48 ~     (iter.next(), iter.by_ref().take(index).chain(iter))
   |

What is more, suggested changes do not compile.
@rustbot label +I-suggestion-causes-error

If I write like this

pub fn extract<T: IntoIterator>(
    into_iter: T,
    index: usize,
) -> (Option<T::Item>, impl Iterator<Item = T::Item>) {
    let mut iter = into_iter.into_iter();
    (iter.next(), iter.by_ref().take(index).chain(iter))
}

I, expectedly, get error from rust compiler

   |
45 | ) -> (Option<T::Item>, impl Iterator<Item = T::Item>) {
   |      ------------------------------------------------ opaque type requires that `iter` is borrowed for `'static`
46 |     let mut iter = into_iter.into_iter();
47 |     (iter.next(), iter.by_ref().take(index).chain(iter))
   |                   ^^^^ borrowed value does not live long enough
48 | }
   | - `iter` dropped here while still borrowed
   |
Meta

Rust version (rustc -Vv):

rustc 1.56.0 (09c42c458 2021-10-18)
binary: rustc
commit-hash: 09c42c45858d5f3aedfa670698275303a3d19afa
commit-date: 2021-10-18
host: x86_64-apple-darwin
release: 1.56.0
LLVM version: 13.0.0

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the needless_collect warning using the example in the issue and inspect the lint behavior around the reported itertools/src/lib.rs location. Check why the suggested rewrite fails to compile, then add or update a regression test covering this iterator-returning case and verify the suggestion no longer produces invalid code.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
devtools, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.