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: Empty enum never type suggested only if the feature is enabled #6513

Merged
merged 5 commits into from
Jan 5, 2021

Conversation

nahuakang
Copy link
Contributor

@nahuakang nahuakang commented Dec 28, 2020

This PR addresses Issue 6422. Instead of always recommending never type for empty enums, Clippy would only recommend the lint if LatePass.TyCtxt has features().never_type enabled.

  • Followed [lint naming conventions][lint_naming]
  • Added passing UI tests (including committed .stderr file)
  • cargo test passes locally
  • Executed cargo dev update_lints
  • Added lint documentation
  • Run cargo dev fmt

Please write a short comment explaining your change (or "none" for internal only changes)
changelog: Only trigger [empty_enum] lint if never_type feature is enabled.

@rust-highfive
Copy link

r? @Manishearth

(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 Dec 28, 2020
@nahuakang
Copy link
Contributor Author

r? @flip1995 After our discussion via email, I added #![feature(never_type)] to tests/ui/empty_enum.rs and then ran cargo dev bless, which updated empty_enum.stderr (discarded all other changes for other tests).

@nahuakang nahuakang marked this pull request as ready for review December 28, 2020 22:15
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Thanks! Just some NITS and a test missing.

clippy_lints/src/empty_enum.rs Outdated Show resolved Hide resolved
clippy_lints/src/empty_enum.rs Outdated Show resolved Hide resolved
tests/ui/empty_enum.rs Show resolved Hide resolved
@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jan 4, 2021
@nahuakang
Copy link
Contributor Author

@flip1995 Made changes according to your feedback. Thanks so much 🙏

@flip1995
Copy link
Member

flip1995 commented Jan 5, 2021

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Jan 5, 2021

📌 Commit a8d47b4 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Jan 5, 2021

⌛ Testing commit a8d47b4 with merge f4e6966...

bors added a commit that referenced this pull request Jan 5, 2021
…p1995

Fix: Empty enum never type suggested only if the feature is enabled

This PR addresses [Issue 6422](#6422). Instead of always recommending `never type` for empty enums, Clippy would only recommend [the lint](https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum) if [LatePass.TyCtxt](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html) has `features().never_type` enabled.

- \[ ] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
---

*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog:
@bors
Copy link
Collaborator

bors commented Jan 5, 2021

💔 Test failed - checks-action_test

@flip1995
Copy link
Member

flip1995 commented Jan 5, 2021

@bors retry

@bors
Copy link
Collaborator

bors commented Jan 5, 2021

⌛ Testing commit a8d47b4 with merge 311186b...

@bors
Copy link
Collaborator

bors commented Jan 5, 2021

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

@bors bors merged commit 311186b into rust-lang:master Jan 5, 2021
@nahuakang nahuakang deleted the fix/empty_enum_lint_never_type branch January 5, 2021 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants