rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

unnecessary_unwrap: should not trigger if the `is_some` is just part of the conditional

Open nrc opened this issue 4 years ago • 3 comments

unnecessary_unwrap recommends that foo.is_some() in a conditional followed by foo.unwrap() should be replaced by an if let. That is usually good advice, however, if the is_some is just part of the conditional, then the only way to use if let is with nested ifs.

E.g.,

if some_condition && foo.is_some() {
    let foo = foo.unwrap();
    ...
}

can only be rewritten as

if some_condition {
    if let Some(foo) = foo {
        ...
    }
}

which is not much of an improvement (and arguably no improvement at all). (In this case, using match might be better, but in some more complex examples it is not possible).

I would much prefer is unnecessary_unwrap only triggered when it was possible to rewrite exactly using a single if let or match.

nrc avatar Sep 10 '19 09:09 nrc

Since chaining if lets in a condition already works on nightly, we can just wait until that hits stable and suggest the chained condition instead.

But until then this seems like an OK fix

oli-obk avatar Sep 10 '19 15:09 oli-obk

triage: it seems the let_chains feature has not been stabilized yet.

matthiaskrgr avatar Mar 21 '20 13:03 matthiaskrgr

let_chains is riding the train to stable and is expected to be in Rust 1.64 🚀 : https://github.com/rust-lang/rust/pull/94927

yerke avatar Jul 22 '22 05:07 yerke

I just ran into this issue. let-chainsgot reverted from Rust 1.64 and as far as I can tell there is no timeline for stabilization

Since let chains have not landed in 4 years, could this be fixed in clippy after all?

DanielT avatar Aug 22 '23 07:08 DanielT