`needless_collect` doesn't understand local borrows

[jpdoyle]

When running clippy (0.0.212 (2020-09-17 f3c923a)) on https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=5b4bbd5f3b653c7d9d8a84d2a63903fa I get the suggestion:

    Checking playground v0.0.1 (/playground)
warning: avoid using `collect()` when not needed
 --> src/main.rs:4:5
  |
4 | /     let a_filt = a.into_iter().filter(|x| !b.contains(x)).collect::<Vec<_>>();
5 | |     
6 | |     for v in b.into_iter().chain(a_filt.into_iter()) {
  | |_________________________________^
  |
  = note: `#[warn(clippy::needless_collect)]` on by default
help: Use the original Iterator instead of collecting it and then producing a new one
  |
4 |     
5 |     
6 |     for v in b.into_iter().chain(a.into_iter().filter(|x| !b.contains(x))) {
  |

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.33s

However, the original Iterator cannot be used, as demonstrated by https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=65b782dbf6cd1731a6906afcd9704e99

   Compiling playground v0.0.1 (/playground)
error[E0505]: cannot move out of `b` because it is borrowed
 --> src/main.rs:6:14
  |
4 |     let a_filt = a.into_iter().filter(|x| !b.contains(x));
  |                                       ---  - borrow occurs due to use in closure
  |                                       |
  |                                       borrow of `b` occurs here
5 |     
6 |     for v in b.into_iter().chain(a_filt) {
  |              ^                   ------ borrow later used here
  |              |
  |              move out of `b` occurs here

error: aborting due to previous error

For more information about this error, try `rustc --explain E0505`.
error: could not compile `playground`

To learn more, run the command again with --verbose.

[flip1995]

So to generalize the problem: If something is borrowed in the collect expression, but later moved in the Iterator expression, this lint should not trigger. When attempting to fix this, a look at the let_and_return lint might help to find out how to check for borrows in an expression (potentially the "borrow check" can be shared).

[SKyletoft]

Might be the same issue, might not be:

fn variations(n: u64, mask: &str) -> Vec<u64> {
    let positions = mask
        .char_indices()
        .filter(|&(_, d)| d == 'X')
        .map(|(idx, _)| 35 - idx as u64)
        .collect::<Vec<u64>>();
    (0..2u64.pow(positions.len() as u32))
        .rev()
        .map(|m| {
            positions.iter().enumerate().fold(n, |acc, (i, &curr)| {
                let to_set = (((1 << i) & m) << curr) >> i;
                let with_hole = (!(1 << curr)) & acc;
                with_hole | to_set
            })
        })
        .collect()
}

warning: avoid using `collect()` when not needed
  --> src/main.rs:94:2
   |
94 |       let positions = mask
   |  _____^
95 | |         .char_indices()
96 | |         .filter(|&(_, d)| d == 'X')
97 | |         .map(|(idx, _)| 35 - idx as u64)
98 | |         .collect::<Vec<u64>>();
99 | |     (0..2u64.pow(positions.len() as u32))
   | |_________________^
   |
   = note: `#[warn(clippy::needless_collect)]` on by default
help: Take the original Iterator's count instead of collecting it and finding the length
   |
94 |     
95 |     (0..2u64.pow(mask
96 |         .char_indices()
97 |         .filter(|&(_, d)| d == 'X')
98 |         .map(|(idx, _)| 35 - idx as u64).count() as u32))
   |

warning: 1 warning emitted

Here the first statement can be replaced with the .count() version, but it doesn't recognise that the collected version is used again later in the statement

[jpdoyle]

something is wrong with tracking where iterators are used, for sure. A small variation on @SKyletoft's example passes without warning: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=f9a04a7d9f7d4409cb91caa73deba23f

[jpdoyle]

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=c9664102f28e54faf426599eace8443a also works...

[djc]

This has a pretty similar problem:

fn foo(iter: impl Iterator<Item = usize>) {
    let bar = iter.collect::<Vec<_>>();
    let range = 0..bar.len();
    (0..100)
        .map(|mut i| {
            for &foo in &bar {
                i += foo;
            }
            i
        })
        .for_each(|i| println!("{}", i));
}

[giraffate]

https://github.com/rust-lang/rust-clippy/issues/6066#issuecomment-747059434

This has a pretty similar problem: (snip)

The lint wasn't emitted at rustc 1.50.0-nightly (b32e6e6ac 2020-12-16). That case seems to be fixed at #6313.

[djc]

Awesome, thanks for fixing it!