incorrect suggestion for while_let_on_iterator

[tspiteri]

For the snippet below, clippy suggests changing while let Some(i) = self.s.next() into for i in self.s { .. }, but that results in a "cannot move out of borrowed content" error. In this case, for i in &mut self.s { .. } would work.

pub struct S {
    i: i32,
}
impl Iterator for S {
    type Item = i32;
    fn next(&mut self) -> Option<i32> {
        if self.i > 0 {
            self.i -= 1;
            Some(self.i)
        } else {
            None
        }
    }
}

pub struct T {
    s: S,
}
impl Iterator for T {
    type Item = i32;
    fn next(&mut self) -> Option<i32> {
        while let Some(i) = self.s.next() {
            if i < 3 || i > 7 {
                return Some(i);
            }
        }
        None
    }
}

#[test]
fn test() {
    let s = S { i: 9 };
    let mut t = T { s };
    assert_eq!(t.next(), Some(8));
    assert_eq!(t.next(), Some(2));
    assert_eq!(t.next(), Some(1));
    assert_eq!(t.next(), Some(0));
    assert_eq!(t.next(), None);
}

[oli-obk]

Hmm... I can't repro on master. @llogiq was this fixed by your PR? Should we extend it to support this case and suggest a for loop over &mut self.s?

Imo the while let loop is better here, since it clearly shows the intent that not all elements of the iterator are consumed.

[oli-obk]

Actually... We should probably detect this very pattern and suggest self.s.filter(|i| *i < 3 || *i > 7).next()

[tspiteri]

About the filter suggestion:

1. It would need to be (&mut self.s).filter rather than self.s.filter, otherwise you get "cannot move out of borrowed content".
2. While that would work in this reduced test case, it would not work if for example the filter expression uses another member of self, as self would be borrowed mutably and the filter body would try to borrow self again.