Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nrc opened this issue Sep 10, 2019 · 4 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@nrc
Copy link
Member

nrc commented Sep 10, 2019

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.

@flip1995 flip1995 added L-suggestion Lint: Improving, adding or fixing lint suggestions C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Sep 10, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Sep 10, 2019

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

@matthiaskrgr
Copy link
Member

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

t-rapp added a commit to t-rapp/rekee that referenced this issue Apr 26, 2021
Apply most clippy code improvement suggestions. Ignore the
unnecessary_unwrap lint where non-beneficial (see rust-lang/rust-clippy#4530).
@yerke
Copy link
Contributor

yerke commented Jul 22, 2022

let_chains is riding the train to stable and is expected to be in Rust 1.64 🚀 : rust-lang/rust#94927

@DanielT
Copy link

DanielT commented Aug 22, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

No branches or pull requests

6 participants