rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.24k stars 1.51k forks source link

needless_collect warning, when it's actually needed #7900

Open pornopatsan opened 2 years ago

pornopatsan commented 2 years ago

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
rustbot commented 2 years ago

Error: Parsing relabel command in comment failed: ...'ot label +' | error: empty label at >| ' I-suggest'...

Please let @rust-lang/release know if you're having trouble with this bot.

pornopatsan commented 2 years ago

Oh, just created issue, and then found, that it is duplicate, sorry for that https://github.com/rust-lang/rust-clippy/issues/7512 https://github.com/rust-lang/rust-clippy/issues/7512 https://github.com/rust-lang/rust-clippy/issues/6066