needless_collect - false positive?

[MrMuetze]

Hey everyone, I stumbled over this issue while updating my version of clippy. I was able to reproduce the issue with a simplification of the original code in a rustlang playground and it looks something like this.

fn main() {
    let vec_a = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
    let vec_b = vec_a
        .iter()
        .filter_map(|item| if item < &6 { Some(item) } else { None })
        .collect::<Vec<_>>();

    if vec_b.len() > 3 {
        println!("we got numbers");
    }

    let other_vec = vec![1, 3, 12, 4, 16, 2];

    let we_got_the_same_numbers = other_vec
        .iter()
        .filter(|item| vec_b.contains(item))
        .collect::<Vec<_>>();
        
    dbg!(we_got_the_same_numbers);
}

Please ignore the filter_map warning. It's due to my simplification of the code and does not appear in the original code base.

Here, clippy throws the needless_collect warning, but I am not sure how I should proceed in that case. It seems to me that the collect is actually required for using the .contains() later on and getting rid of it would make things more complicated. I was able to clean up code in a couple of other places where needless_collect warnings popped up, but I'm not sure about this bit.

The playground uses rust version 1.46.0-stable and clippy version 0.0.212. A direct link to the playground can be found here:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=9fd8c2407aefab646fa97e13088f47c0

I'm not sure how to create a backtrace from the playground, but if desired I can add this to the issue by running it locally as well.

Thanks for working on clippy!
Bjoern

[yaahc]

Adding to this, I've also encountered a needless_collect false positive today here: https://github.com/ZcashFoundation/zebra/pull/1016/files/409a2289b57aa71cacbf19b17f6f5839890092e3#diff-94ae6030e39498920ff1ee31c52222efR319-R320

[therocode]

Also another false positive here:

fn reverse_path<N, V, F>(parents: &IndexMap<N, V>, mut parent: F, start: usize) -> Vec<N>
where
    N: Eq + Hash + Clone,
    F: FnMut(&V) -> usize,
{
    let path = itertools::unfold(start, |i| {
        parents.get_index(*i).map(|(node, value)| {
            *i = parent(value);
            node
        })
    })
    .collect::<Vec<&N>>();

    path.into_iter().rev().cloned().collect()
}

clippy doesn't see that the double collect is needed to get the iterator to be a double ended iterator as required by .rev()

[olson-sean-k]

+1. I think I've also encountered a false positive here (see this CI failure log).