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

New lints are not backwards compatible #12495

Closed
ehuss opened this issue Aug 14, 2023 · 2 comments
Closed

New lints are not backwards compatible #12495

ehuss opened this issue Aug 14, 2023 · 2 comments
Assignees
Labels
A-lints-table Area: [lints] table C-bug Category: bug S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix

Comments

@ehuss
Copy link
Contributor

ehuss commented Aug 14, 2023

Problem

When a new lint is introduced, a user may want to start using it via the [lints] table. However, if they add it to the table, it will be a hard error for all versions of Rust before the lint was introduced. This can make it difficult to set the level of lints, where compared to using attributes which just generate a warning.

Steps

Set up a project with:

[package]
name = "foo"
version = "0.1.0"

[lints.rust]
some-new-lint = "warn"

Results in:

error[E0602]: unknown lint: `some_new_lint`
  |
  = note: requested on the command line with `-W some_new_lint`

For more information about this error, try `rustc --explain E0602`.
error: could not compile `foo` (lib) due to previous error

With no possibility to allow it (even with capped lints).

Possible Solution(s)

This may require a change in rustc (and thus this may be an issue for rust-lang/rust). I think unknown lints on the command-line should not generate an error to match the behavior of an unknown attribute, and should just generate an unknown_lints warning.

This has been the behavior since the beginning (well beyond 1.0). I don't think there was any motivation other than all CLI flags generated errors (like -C foo also generates an error).

Notes

No response

Version

cargo 1.73.0-nightly (d78bbf4bd 2023-08-03)
release: 1.73.0-nightly
commit-hash: d78bbf4bde3c6b95caca7512f537c6f9721426ff
commit-date: 2023-08-03
host: aarch64-apple-darwin
libgit2: 1.6.4 (sys:0.17.2 vendored)
libcurl: 7.88.1 (sys:0.4.65+curl-8.2.1 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Mac OS 13.4.1 [64-bit]
@ehuss ehuss added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. A-lints-table Area: [lints] table labels Aug 14, 2023
@weihanglo weihanglo added S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix and removed S-triage Status: This issue is waiting on initial triage. labels Aug 20, 2023
@ojeda
Copy link

ojeda commented Aug 21, 2023

This may require a change in rustc (and thus this may be an issue for rust-lang/rust). I think unknown lints on the command-line should not generate an error to match the behavior of an unknown attribute, and should just generate an unknown_lints warning.

The manual way would be having Cargo avoiding to pass the flags for given versions. This is what we do for C in the Linux kernel (either testing if the flag itself is recognized or for the version), and what we would currently need to do for rustc.

That infrastructure for skipping flags is something we would need to do anyway, even if this is in place, since other flags that are not diagnostics are not included in this "ignore if unknown" proposal, right? Thus one may just do it for diagnostics too instead of -Aunknown_lints to ensure we do not have mistakes in lint names.

Is the intention that one has a CI build with the latest compiler and -Dunknown_lints, to ensure there are no such mistakes in lint names, and then in all other builds use -Aunknown_lints?

Also, related discussion on ignoring lints (when compiling with newer versions, unlike here which is about compiling with older versions) and lint stability: rust-lang/rust-clippy#11227

@weihanglo weihanglo self-assigned this Aug 24, 2023
weihanglo added a commit to weihanglo/rust that referenced this issue Aug 24, 2023
…r-errors

refactor(lint): translate `RenamedOrRemovedLint`

I was trying to address <rust-lang/cargo#12495> and found that maybe I should refactor relevant lints a bit.

This PR translates `RenamedOrRemovedLint` into fluent file. To make diagnostic types clearer and easier to organize, this PR splits it into two structs.

The second commit adds lifetime annotations for removing unnecessary clones. If people feel too noisy, we can revert such change.

### Possibly relevant UI tests:

* `tests/ui/lint-removed*`
* `tests/ui/lint-renamed*`
* `tests/ui/rustdoc-renamed.rs`
* `tests/rustdoc-ui/lints/unknown-renamed-lints.rs`
@weihanglo
Copy link
Member

This should be resolved by rust-lang/rust#115387. One could simply add this and smile :)

[lints.rust]
unknown-lints = 'warn'

If it is not, please comment below or reopen the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints-table Area: [lints] table C-bug Category: bug S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix
Projects
None yet
Development

No branches or pull requests

3 participants