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.

Sign up for GitHub

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

Jump to bottom

Don't lint manual_non_exhaustive when enum is #[non_exhaustive] #11590

Merged
merged 2 commits into from Oct 1, 2023
Merged

Don't lint manual_non_exhaustive when enum is #[non_exhaustive] #11590

merged 2 commits into from Oct 1, 2023

Conversation

Tyrubias
Copy link
Contributor

@Tyrubias Tyrubias commented Oct 1, 2023

Fixes #11583

changelog: Fix [manual_non_exhaustive] false positive for unit enum variants when enum is explicitly non_exhaustive.

@Tyrubias
Don't lint manual_non_exhaustive when enum explicitly marked as `no…
e683e3e 
…n_exhaustive`

There are cases where users create a unit variant for the purposes
of tracking the number of variants for an nonexhaustive enum.
We should check if an enum is explicitly marked as nonexhaustive
before reporting `manual_non_exhaustive` in these cases. Fixes rust-lang#11583
@rustbot
Copy link
Collaborator

rustbot commented Oct 1, 2023

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

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 1, 2023
@Alexendoo
Copy link
Member

Thanks! Could you also remove the now unneeded check on line 203

@Tyrubias
Copy link
Contributor Author

Tyrubias commented Oct 1, 2023

I've removed the unneeded check, but having seen this I'm a little confused as to why this check didn't catch this false positive. Do you mind explaining?

@Jarcho
Copy link
Contributor

Jarcho commented Oct 1, 2023

IIRC this was linting on purpose here to catch when the attribute was added, but the extra variant wasn't removed. Probably should have been it's own lint rather than reusing manually_non_exhaustive.

@Alexendoo
Copy link
Member

Yeah, the check only omitted part of the suggestion rather than the whole lint

Thanks! @bors r+

@bors
Copy link
Collaborator

bors commented Oct 1, 2023

📌 Commit 9dfd60c has been approved by Alexendoo

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Oct 1, 2023

⌛ Testing commit 9dfd60c with merge cbe67bc...

@bors
Copy link
Collaborator

bors commented Oct 1, 2023

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

@bors bors merged commit cbe67bc into rust-lang:master Oct 1, 2023
5 checks passed
@Tyrubias Tyrubias deleted the non_ex_false_positive branch October 1, 2023 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alexendoo Alexendoo Alexendoo approved these changes

Assignees

@Alexendoo Alexendoo

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

clippy::manual_non_exhaustive triggers on #[non_exhaustive] enum
5 participants
@Tyrubias @rustbot @Alexendoo @Jarcho @bors