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

Fix needless_match false positive for if-let when the else block doesn't match to given expr #8700

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

youknowone
Copy link
Contributor

@youknowone youknowone commented Apr 15, 2022

fix #8695

Please write a short comment explaining your change (or "none" for internal only changes)

changelog: Fixed [`needless_match`] false positive when else block expression differs.

@rust-highfive
Copy link

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

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 15, 2022
@youknowone youknowone force-pushed the needless_match-false-positive branch 3 times, most recently from fbe3b2e to d2f9c61 Compare April 15, 2022 10:55
@youknowone youknowone changed the title (Draft) Failling test for needless_match false positive Fix needless_match false positive for if-let when the else block doesn't match to given expr Apr 15, 2022
@youknowone youknowone marked this pull request as ready for review April 15, 2022 10:58
@xFrednet
Copy link
Member

r? @Alexendoo

@xFrednet xFrednet assigned xFrednet and unassigned camsteffen and xFrednet Apr 17, 2022
Copy link
Member

@Alexendoo Alexendoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @xFrednet


// Don't trigger
let _ = if let Some(a) = Some(1) {
do_something();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though actually one small nit, the do_something should go as this statement doesn't trigger the FP

Suggested change
do_something();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alexendoo thank you! I fixed it

@youknowone youknowone force-pushed the needless_match-false-positive branch 2 times, most recently from b3ef94b to fa19572 Compare April 18, 2022 05:24
when the else block doesn't match to given expr
@youknowone youknowone force-pushed the needless_match-false-positive branch from fa19572 to b94e24e Compare April 18, 2022 05:33
@xFrednet
Copy link
Member

This version looks good to me, I'm taking the previous approval from @Alexendoo also as an okay. Thank you very much for the contribution and quick update @youknowone. 👍

Also thank you to @Alexendoo for taking over the review! 🎉

@bors r+

@bors
Copy link
Contributor

bors commented Apr 20, 2022

📌 Commit b94e24e has been approved by xFrednet

@bors
Copy link
Contributor

bors commented Apr 20, 2022

⌛ Testing commit b94e24e with merge f99ad82...

@youknowone
Copy link
Contributor Author

Thank you for everyone. I am excited to make my first contribution to clippy, finally. I want to bring more patches in near future.

@bors
Copy link
Contributor

bors commented Apr 20, 2022

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

@bors bors merged commit f99ad82 into rust-lang:master Apr 20, 2022
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.

needless_match doesn't consider the else of if let
7 participants