false positive on while_immutable_condition

[kpp]

Code:

pub fn skip_while<St, F, Fut>(stream: St, f: F) -> impl Stream<Item = St::Item>
where
    St: Stream,
    F: FnMut(&St::Item) -> Fut,
    Fut: Future<Output = bool>,
{
    let stream = Box::pin(stream);
    let should_skip = true;
    crate::stream::unfold(
        (stream, f, should_skip),
        |(mut stream, mut f, should_skip)| async move {
            while should_skip {
                if let Some(item) = next(&mut stream).await {
                    if f(&item).await {
                        continue;
                    } else {
                        return Some((item, (stream, f, /* should_skip */ false)));
                    }
                } else {
                    return None;
                }
            }
            if let Some(item) = next(&mut stream).await {
                Some((item, (stream, f, /* should_skip */ false)))
            } else {
                None
            }
        },
    )
}

Error:

error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop.
   --> src/stream.rs:717:19
    |
717 |             while should_skip {
    |                   ^^^^^^^^^^^
    |

https://github.com/kpp/futures-async-combinators/blob/d8da69c9f3b4587a99ddf1e0e5545bcff81cf57c/src/stream.rs#L718

[flip1995]

I don't see the FP. As long as next(&mut stream).await is Some and f(&item).await is true this is an infinite loop, or am I missing something?

[kpp]

There are two termination conditions in the loop, so it is not always an infinite loop.

[sinkuu]

I would like to use if cond { loop { } } in this case for more clarity. while's condition should be part of its termination condition.

[flip1995]

There are two termination conditions in the loop, so it is not always an infinite loop.

Ah I see. I think the lint triggering here is correct. We should at least reword the lint to ...may lead to..., though. Best case would be to split up this lint in a correctness part, that triggers, when there are no return or break statements in the loop body, and a style part, that suggests if cond { loop { } } otherwise.

[kpp]

Yep, if cond { loop { } } looks better, however this FP should not be an error but a suggestion.

[yerke]

Can I claim this issue?

[flip1995]

Yeah sure, if you have any questions just ask here or open a WIP PR.