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

fix: Make lint names snake_case #13635

Merged
merged 1 commit into from
Mar 25, 2024
Merged

fix: Make lint names snake_case #13635

merged 1 commit into from
Mar 25, 2024

Conversation

Muscraft
Copy link
Member

@Muscraft Muscraft commented Mar 24, 2024 •
edited
Loading

When working on #13621, I somehow missed that lint names should be snake_case according to the rustc-dev-guide as well as RFC #344.

This PR renames:

  • implicit-features => implicit_featires
  • rust-2024-compatibility => rust_2024_compatibility.

Note: We should probably have some tooling to enforce this, but I was unsure if it belonged to this PR or another one. One solution would be to use a macro to create the const LINT_NAME: Lint = {...}, where LINT_NAME would be the ident as well as the name: &'static str and then have a method on Lint to make it lowercase as needed. This is what rustc does, and it could work well here. It would ensure snake case as const names need to be SCREAMING_SNAKE_CASE, or a warning is shown.

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 24, 2024
@epage
Copy link
Contributor

epage commented Mar 25, 2024

This is strange because every lint and group in rustc -Whelp uses kebab-case, including rust-2024-compatibility which you are mirroring.

@rustbot rustbot added the A-workspaces Area: workspaces label Mar 25, 2024
@Muscraft
Copy link
Member Author

I did some more digging and found that:

  • rustc -Whelp replaces _ with -
  • Lints passed via the CLI, have - replaced with _
  • _ is used internally for all lint names
  • It looks like lints were originally - but used _ for attributes
    • I went back via git blame ~12 years and I found where attributes were added but couldn't find the discussion for lints

So I made it so we use _ internally, but externally, you can use - or _. I added a test to verify this.

epage
epage reviewed Mar 25, 2024
View reviewed changes
src/cargo/core/workspace.rs
Comment on lines +1142 to +1145
let normalized_lints = cargo_lints
.into_iter()
.map(|(name, lint)| (name.replace('-', "_"), lint))
.collect();
Copy link
Contributor

@epage epage Mar 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

non-blocking: this could live in toml/mod.rs when we resolve the manifest.

Please don't do that right now as I have probably 20 commits locally changing that code :)

@epage
Copy link
Contributor

epage commented Mar 25, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 25, 2024

📌 Commit 0a400d5 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 25, 2024
@bors
Copy link
Contributor

bors commented Mar 25, 2024

⌛ Testing commit 0a400d5 with merge bc844af...

@bors
Copy link
Contributor

bors commented Mar 25, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing bc844af to master...

@bors bors merged commit bc844af into rust-lang:master Mar 25, 2024
21 checks passed
@Muscraft Muscraft deleted the lint-names-snake-case branch March 25, 2024 19:41
@Muscraft Muscraft mentioned this pull request Mar 26, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 26, 2024
@bors
Auto merge of rust-lang#123081 - Muscraft:update-cargo, r=Muscraft,wo…
b13a71a 
…rkingjubilee

Update cargo

5 commits in a510712d05c6c98f987af24dd73cdfafee8922e6..499a61ce7a0fc6a72040084862a68b2603e770e8
2024-03-25 03:45:54 +0000 to 2024-03-26 04:17:04 +0000
- fix: do not borrow shell across registry query (rust-lang/cargo#13647)
- Do not strip debuginfo by default for MSVC (rust-lang/cargo#13630)
- chore(deps): update msrv (rust-lang/cargo#13577)
- Fix doc collision for lib/bin with a dash in the inferred name. (rust-lang/cargo#13640)
- refactor: Make lint names snake_case (rust-lang/cargo#13635)

r? ghost
@rustbot rustbot added this to the 1.79.0 milestone Mar 26, 2024
@epage epage changed the title refactor: Make lint names snake_case fix: Make lint names snake_case May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epage epage epage left review comments

Assignees

@weihanglo weihanglo

Labels
A-workspaces Area: workspaces S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Muscraft @rustbot @epage @bors @weihanglo