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(lint): Warn not Error on unsupported lint tool #13833

Merged
merged 1 commit into from
May 1, 2024

Conversation

Muscraft
Copy link
Member

In a recent Cargo Team meeting, it was decided to lessen the error on an unsupported tool in [lints] to a warning. This PR implements that change.

@rustbot
Copy link
Collaborator

rustbot commented Apr 30, 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-manifest Area: Cargo.toml issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 30, 2024
src/cargo/util/toml/mod.rs Outdated Show resolved Hide resolved
@Muscraft Muscraft force-pushed the warn-unknown-tool-lints-table branch from 836111e to e4e3278 Compare April 30, 2024 23:06
Comment on lines 2237 to 2239
specifying an unrecognized lint tool could lead to future breakage, as `cargo`
reserves all of the `lints` namespace for itself and provides no compatibility
guarantees when encountering unrecognized tools"
Copy link
Member Author

Choose a reason for hiding this comment

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

I also considered:

`cargo` reserves all of the `lints` namespace for itself and provides no
future compatibility guarantees when encountering unrecognized tools, as they
may be supported in the future.

Copy link
Member

Choose a reason for hiding this comment

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

I prefer this one.

Should we also suggest using package.metadata?

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this makes it sound like the lints table is special and this doesn't apply elsewhere.

Something to keep in mind is people might also be here from a typo.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this should be the top level message:

unrecognized lint tool `{}`, must be one of {supported}

What about:

no compatibility guarantees are provided when specifying unrecognized manifest
key(s), as the entire namespace is reserved for future use. Tools that wish to
store package configuration should use `package.metadata`

or

no compatibility guarantees are provided when specifying unrecognized manifest
key(s) as `cargo` reserves the entire namespace for future use. `package.metadata`
should be used for tools which would like to store package configuration

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm good with the now updated wording

@Muscraft Muscraft force-pushed the warn-unknown-tool-lints-table branch from e4e3278 to 72f265b Compare May 1, 2024 02:21
@epage epage changed the title refactor: Warn not Error on unsupported lint tool fix(lint): Warn not Error on unsupported lint tool May 1, 2024
@epage
Copy link
Contributor

epage commented May 1, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented May 1, 2024

📌 Commit 72f265b 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 72f265b with merge 1c92c1e...

@bors
Copy link
Collaborator

bors commented May 1, 2024

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

@bors bors merged commit 1c92c1e into rust-lang:master May 1, 2024
21 checks passed
@Muscraft Muscraft deleted the warn-unknown-tool-lints-table branch May 1, 2024 03:05
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-manifest Area: Cargo.toml issues 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.

None yet

6 participants