#7594: Adding new 'while_let_some_result' linting

[andrewpollack]

Excited for the opportunity to contribute to Clippy! Since the latest Bay Area Rust Meetup, I've been looking for an opportunity and found it 😄

Please let me know how I can improve this PR! Much of it is similar to [`if_let_some_result`], but I followed the description in the issue to create its own linting rules. Looking forward to feedback, and continuing to work on Clippy in the future!

changelog: Renamed Lint: if_let_some_result is now called [match_result_ok]. Now also handles while let case.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

Please see the contribution instructions for more information.

[Manishearth]

Looks really good, thanks!

[bors]

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

[andrewpollack]

@Manishearth @camsteffen Thank you for the suggestion to rename and add functionality to match_result_ok!

Although functioning, I know the syntax can be improved. After some time, I tried to do some clever if statements to pull out the small differences to the top and have them go down the same if path. Unfortunately, I struggled a bit with Rust syntax and was unsuccessful in doing so. Happy to look into fixing the syntax if some tips can be given on syntax around how to merge this statement:

if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr);
and
if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr);

I tried if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) | if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr);, but the compiler did not like this formatting

[Manishearth]

progress, some issues left

[Manishearth]

@andrewpollack yeah, there's no fancy syntax, just use a match there instead, it's the first condition in the if chains. If you need you can factor out the stuff inside it into a different function to make it easier

[bors]

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

[Manishearth]

I pushed a commit making this change, and also rebased it

(for future reference, please rebase instead of merging when there are merge conflicts, it makes it harder to do further rebases if merges are used)

[Manishearth]

@bors r+

[bors]

📌 Commit 17155c8 has been approved by Manishearth

[bors]

⌛ Testing commit 17155c8 with merge 7a4e25c...

[bors]

💔 Test failed - checks-action_test

[Manishearth]

@bors retry

(PR body changelong not in right format)

[bors]

⌛ Testing commit 17155c8 with merge a2a9b1d...

[bors]

💔 Test failed - checks-action_dev_test

[Manishearth]

@bors r+

[bors]

📌 Commit 13834e6 has been approved by Manishearth

[bors]

⌛ Testing commit 13834e6 with merge cbf27d0...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing cbf27d0 to master...

[andrewpollack]

Thank you so much for helping on this — this is incredibly valuable to learn from, and I am grateful for your help! Excited to keep using and learning Rust, and contributing where I can :]

…

On Mon, Sep 27, 2021 at 10:25 PM Manish Goregaokar ***@***.***> wrote: I pushed a commit making this change, and also rebased it (for future reference, please rebase instead of merging when there are merge conflicts, it makes it harder to do further rebases if merges are used) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7608 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AF5XNGJOK7KTFQ3H25ICH5TUEFGTXANCNFSM5DA4CK5A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.