clippy::if_let_some_result suggests changing while to if, resulting in code with different behaviour

[dllu]

Lint name: if_let_some_result

I tried this code:

struct Wat {
    counter: i32,
}

impl Wat {
    fn next(&mut self) -> Result<i32, &str> {
        self.counter += 1;
        if self.counter < 5 {
            Ok(self.counter)
        } else {
            Err("Oh no")
        }
    }
}

fn main() {
    let mut wat = Wat { counter: 0 };
    while let Some(a) = wat.next().ok() {
        dbg!(&a);
    }
}

When you run the above code, you get:

cargo run
   Compiling whilelet v0.1.0 (/home/dllu/snippets/whilelet)
    Finished dev [unoptimized + debuginfo] target(s) in 0.38s
     Running `target/debug/whilelet`
[src/main.rs:19] &a = 1
[src/main.rs:19] &a = 2
[src/main.rs:19] &a = 3
[src/main.rs:19] &a = 4

I expected to see this happen: while let Ok(a) = wat.next()

Instead, this happened: if let Ok(a) = wat.next()

cargo +nightly clippy -Z unstable-options
    Checking whilelet v0.1.0 (/home/dllu/snippets/whilelet)
warning: matching on `Some` with `ok()` is redundant
  --> src/main.rs:18:5
   |
18 |     while let Some(a) = wat.next().ok() {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(clippy::if_let_some_result)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result
help: consider matching on `Ok(a)` and removing the call to `ok` instead
   |
18 |     if let Ok(a) = wat.next() {
   |     ~~~~~~~~~~~~~~~~~~~~~~~~~

warning: `whilelet` (bin "whilelet") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.18s

This is quite dangerous because if you run --fix, you will get code with different behaviour!!!

> cargo +nightly clippy -Z unstable-options --fix
    Checking whilelet v0.1.0 (/home/dllu/snippets/whilelet)
       Fixed src/main.rs (1 fix)
    Finished dev [unoptimized + debuginfo] target(s) in 0.29s
> cargo run
   Compiling whilelet v0.1.0 (/home/dllu/snippets/whilelet)
    Finished dev [unoptimized + debuginfo] target(s) in 0.37s
     Running `target/debug/whilelet`
[src/main.rs:19] &a = 1

Meta

• cargo clippy -V: clippy 0.1.56 (30a0a9b 2021-08-17)
• rustc -Vv:

rustc 1.56.0-nightly (30a0a9b69 2021-08-17)
binary: rustc
commit-hash: 30a0a9b694cde95cbab863f7ef4d554f0f46b606
commit-date: 2021-08-17
host: x86_64-unknown-linux-gnu
release: 1.56.0-nightly
LLVM version: 12.0.1

[dllu]

haha @JohnTitor I guess you were right when you said #5032 that

This approach may be fragile though...

[dswij]

I suppose the name of the lint should be changed then? Something like let_some_result?

[xFrednet]

@camsteffen Do you think that this is also a regression bug from rust-lang/rust#80357? It looks similar to #7575 that you're fixing in rust-lang/rust#88163

[camsteffen]

Indeed. Let me see if I can throw in a fix for that too. I think we can check for desugaring in higher::IfLet.

[dllu]

Thanks for fixing. However I was hoping that Clippy would correct

    while let Some(a) = wat.next().ok() {

to

    while let Ok(a) = wat.next() {

but now it seems that it simply does not raise any warnings for this code. Would it be possible to add that, or is that out of the scope of if_let_some_result? Should there be a while_let_some_result?

[camsteffen]

I think that could be an enhancement to if_let_some_result. Do you mind opening another issue for that?