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

check for if-some-or-ok-else-none-or-err #8696

Merged
merged 3 commits into from
Aug 21, 2022
Merged

Conversation

J-ZhengLi
Copy link
Member

@J-ZhengLi J-ZhengLi commented Apr 14, 2022

fixes: #8492


changelog: make [option_if_let_else] to check for match expression with both Option and Result; TODO: Change lint name? Add new lint with similar functionality?

@rust-highfive
Copy link

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 14, 2022
@bors
Copy link
Collaborator

bors commented Apr 15, 2022

☔ The latest upstream changes (presumably #8705) made this pull request unmergeable. Please resolve the merge conflicts.

@J-ZhengLi J-ZhengLi changed the title [WIP] check for if-some-or-ok-else-none-or-err check for if-some-or-ok-else-none-or-err Apr 24, 2022
@J-ZhengLi
Copy link
Member Author

Well... I forgor that I did for this PR already, may I request a feedback anyway? @flip1995

@Serial-ATA
Copy link
Contributor

Serial-ATA commented Jun 23, 2022

Should the lint be renamed if it's going to cover match now as well? Just saw you already asked that 😄.

@Serial-ATA
Copy link
Contributor

Serial-ATA commented Jun 23, 2022

I think this should only check for arms that span a single line.

I don't think going from

let _ = match res {
    Err(_) => 1,
    Ok(mut a) => {
        a += 1;
        println!("{a}");
        a -= 1;
        let b = 5;
        a += b;
        a
    }
};

to

let _ = res.map_or(1, |mut a| {
        a += 1;
        println!("{a}");
        a -= 1;
        let b = 5;
        a += b;
        a
    });

makes much of a difference.

@flip1995
Copy link
Member

Thanks! Sorry for taking so long. I rebased it and did a minor cleanup. About the lint name: Yes it will need to be changed. But since this lint is currently in nursery there's no rush to decide this now. I added a FIXME comment for it.

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 21, 2022

📌 Commit 1f75845 has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 21, 2022

⌛ Testing commit 1f75845 with merge 87b3afc...

@bors
Copy link
Collaborator

bors commented Aug 21, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 87b3afc to master...

@bors bors merged commit 87b3afc into rust-lang:master Aug 21, 2022
@J-ZhengLi J-ZhengLi deleted the issue8492 branch February 8, 2023 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

suggest map_or for simple None substitutions
5 participants