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(lints): Remove ability to specify - in lint name #13837

Merged
merged 2 commits into from
May 1, 2024

Conversation

Muscraft
Copy link
Member

@Muscraft Muscraft commented May 1, 2024

In a recent Cargo Team meeting, it was discussed whether our lint should use - or _ and if we should rewrite the wrong form to the correct one. It was decided that Cargo would use _ for lint names and would not convert - to _ automatically; instead, we would warn about an "unknown lint" and mention the similarly named lint with _, if found.

The decision to ise _ was made because it is the canonical representation, as well as RFC #344 specifies:

Use snake case in the same way you would for function names.

This PR implements these changes.

Note: This adds an unknown_lints lint, that tries to mirror the lint rustc has with the same name.

@rustbot
Copy link
Collaborator

rustbot commented May 1, 2024

r? @ehuss

rustbot has assigned @ehuss.
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 A-workspaces Area: workspaces S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2024
@Muscraft Muscraft force-pushed the only-underscores-lint-names branch from 49bf098 to 6c08e58 Compare May 1, 2024 16:46
@Muscraft Muscraft mentioned this pull request May 1, 2024
@epage
Copy link
Contributor

epage commented May 1, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented May 1, 2024

📌 Commit 6c08e58 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 May 1, 2024
@bors
Copy link
Collaborator

bors commented May 1, 2024

⌛ Testing commit 6c08e58 with merge bbea437...

let dash_feature_name = feature_gate.name().replace("_", "-");
let title = format!("use of unstable lint `{}`", dash_name);
let title = format!("use of unstable lint `{}`", lint_name);
Copy link
Member

Choose a reason for hiding this comment

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

nit: implicit arg capture (should we force that in clippy?)

Suggested change
let title = format!("use of unstable lint `{}`", lint_name);
let title = format!("use of unstable lint `{lint_name}`");

Copy link
Contributor

Choose a reason for hiding this comment

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

If its something we bring up in code review but could be automated, then yes

with the caveat on how often we don't want it which I believe is pretty rare for this one.

@@ -573,6 +601,120 @@ pub fn check_implicit_features(
Ok(())
}

const UNKNOWN_LINTS: Lint = Lint {
Copy link
Member

Choose a reason for hiding this comment

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

You know. I'll request for more doc comments like what IMPLICIT_FEATURES has :)

} else if let Some(group) = LINT_GROUPS.iter().find(|g| g.name == underscore_lint_name) {
Some((group.name, "group"))
} else {
None
Copy link
Member

Choose a reason for hiding this comment

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

Besides dash-replaced, should we also leverage util::edit_distance::closest to find one more possible suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should. I saw that as a future improvement. The minimum we needed in this case was to help with dropping support for -.

@bors
Copy link
Collaborator

bors commented May 1, 2024

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

@bors bors merged commit bbea437 into rust-lang:master May 1, 2024
21 checks passed
@Muscraft Muscraft deleted the only-underscores-lint-names branch May 1, 2024 21:39
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
Update cargo

18 commits in 6087566b3fa73bfda29702632493e938b12d19e5..05364cb2f61a2c2b091e061c1f42b207dfb5f81f
2024-04-30 20:45:20 +0000 to 2024-05-03 16:48:59 +0000
- chore(deps): update msrv (3 versions) to v1.76 (rust-lang/cargo#13857)
- Stabilize `-Zcheck-cfg` as always enabled (rust-lang/cargo#13571)
- fix(lints): Prevent inheritance from bring exposed for published packages (rust-lang/cargo#13852)
- refactor: remove unnecessary branch for link binary on macOS (rust-lang/cargo#13851)
- perf(toml): Avoid inferring when targets are known (rust-lang/cargo#13849)
- Update continuous-integration.md: Include CircleCI reference (rust-lang/cargo#13850)
- chore(deps): update msrv (1 version) to v1.78 (rust-lang/cargo#13848)
- Workaround copying file returning EAGAIN on ZFS on mac OS (rust-lang/cargo#13845)
- Clean package perf improvements (rust-lang/cargo#13818)
- fix(toml): Validate crates_types/proc-macro for bin like others (rust-lang/cargo#13841)
- fix(toml): On 2024 Edition, disallow ignored `default-features` when inheriting (rust-lang/cargo#13839)
- chore(ci): Ignore openssl deps (rust-lang/cargo#13840)
- fix(lints): Remove ability to specify `-` in lint name (rust-lang/cargo#13837)
- fix(resolver): Treat unset MSRV as compatible (rust-lang/cargo#13791)
- fix(toml): Don't lose 'public' when inheriting a dep (rust-lang/cargo#13836)
- chore(deps): update compatible (rust-lang/cargo#13834)
- Error when unstable lints are specified but not enabled (rust-lang/cargo#13805)
- fix(lint): Warn not Error on unsupported lint tool (rust-lang/cargo#13833)

r? ghost

Note: this includes the fix that was beta backported in rust-lang#124647
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2024
Update cargo

18 commits in 6087566b3fa73bfda29702632493e938b12d19e5..05364cb2f61a2c2b091e061c1f42b207dfb5f81f
2024-04-30 20:45:20 +0000 to 2024-05-03 16:48:59 +0000
- chore(deps): update msrv (3 versions) to v1.76 (rust-lang/cargo#13857)
- Stabilize `-Zcheck-cfg` as always enabled (rust-lang/cargo#13571)
- fix(lints): Prevent inheritance from bring exposed for published packages (rust-lang/cargo#13852)
- refactor: remove unnecessary branch for link binary on macOS (rust-lang/cargo#13851)
- perf(toml): Avoid inferring when targets are known (rust-lang/cargo#13849)
- Update continuous-integration.md: Include CircleCI reference (rust-lang/cargo#13850)
- chore(deps): update msrv (1 version) to v1.78 (rust-lang/cargo#13848)
- Workaround copying file returning EAGAIN on ZFS on mac OS (rust-lang/cargo#13845)
- Clean package perf improvements (rust-lang/cargo#13818)
- fix(toml): Validate crates_types/proc-macro for bin like others (rust-lang/cargo#13841)
- fix(toml): On 2024 Edition, disallow ignored `default-features` when inheriting (rust-lang/cargo#13839)
- chore(ci): Ignore openssl deps (rust-lang/cargo#13840)
- fix(lints): Remove ability to specify `-` in lint name (rust-lang/cargo#13837)
- fix(resolver): Treat unset MSRV as compatible (rust-lang/cargo#13791)
- fix(toml): Don't lose 'public' when inheriting a dep (rust-lang/cargo#13836)
- chore(deps): update compatible (rust-lang/cargo#13834)
- Error when unstable lints are specified but not enabled (rust-lang/cargo#13805)
- fix(lint): Warn not Error on unsupported lint tool (rust-lang/cargo#13833)

r? ghost

Note: this includes the fix that was beta backported in rust-lang#124647
@ehuss ehuss added this to the 1.80.0 milestone May 8, 2024
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 24, 2024
Update cargo

18 commits in 6087566b3fa73bfda29702632493e938b12d19e5..05364cb2f61a2c2b091e061c1f42b207dfb5f81f
2024-04-30 20:45:20 +0000 to 2024-05-03 16:48:59 +0000
- chore(deps): update msrv (3 versions) to v1.76 (rust-lang/cargo#13857)
- Stabilize `-Zcheck-cfg` as always enabled (rust-lang/cargo#13571)
- fix(lints): Prevent inheritance from bring exposed for published packages (rust-lang/cargo#13852)
- refactor: remove unnecessary branch for link binary on macOS (rust-lang/cargo#13851)
- perf(toml): Avoid inferring when targets are known (rust-lang/cargo#13849)
- Update continuous-integration.md: Include CircleCI reference (rust-lang/cargo#13850)
- chore(deps): update msrv (1 version) to v1.78 (rust-lang/cargo#13848)
- Workaround copying file returning EAGAIN on ZFS on mac OS (rust-lang/cargo#13845)
- Clean package perf improvements (rust-lang/cargo#13818)
- fix(toml): Validate crates_types/proc-macro for bin like others (rust-lang/cargo#13841)
- fix(toml): On 2024 Edition, disallow ignored `default-features` when inheriting (rust-lang/cargo#13839)
- chore(ci): Ignore openssl deps (rust-lang/cargo#13840)
- fix(lints): Remove ability to specify `-` in lint name (rust-lang/cargo#13837)
- fix(resolver): Treat unset MSRV as compatible (rust-lang/cargo#13791)
- fix(toml): Don't lose 'public' when inheriting a dep (rust-lang/cargo#13836)
- chore(deps): update compatible (rust-lang/cargo#13834)
- Error when unstable lints are specified but not enabled (rust-lang/cargo#13805)
- fix(lint): Warn not Error on unsupported lint tool (rust-lang/cargo#13833)

r? ghost

Note: this includes the fix that was beta backported in #124647
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

6 participants