Tracking Issue for `[lints]` table RFC 3389

[ehuss]

Summary

RFC: #3389
Implementation: #12148, #12168, #12174, #12174
Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints
Issues: A-lints-table Area: [lints] table

This feature adds a [lints] table to Cargo.toml to configure reporting levels for rustc and other tool lints.

See also #3591, #5034

Testing instructions

Requires

• nightly: nightly-2023-05-25 or newer
• beta
• stable

Steps:

1. Switch from attributes to [lints] table per the unstable reference

• -Dwarnings should likely still be passed in explicitly so local experimentation doesn't get blocked on perfection
• CI jobs passing in -Adeprecations likely should still do so since users should see deprecation warnings locally to decide whether to address them

2. Verify lints in CI with a supported compiler by adding the -Zlints flag

Debugging

• Build with --verbose to see what order [lints] and other RUSTFLAGS are being passed to the underlying tool

Example: #12178

Updates Since RFC

• cargo new will automatically inherit the lints table (Automatically inherit workspace lints when running cargo new/init #12174)
• profile.rustflags precedence with [lints] was unspecified. We have put made it a higher precedence.
• rustflags derived from [lints] table are not included in rustc -C metadata hash.
• [lints] should only apply to doctests as much as a diagnostic attributes (#[warn(...)]) in an .rs file applies to doctests
• Per-target or per-target-kind (bin vs lib) [lints] tables is deferred (not in original RFC)
  • Workaround: using diagnostic attributes (e.g. #[warn] in the target)
  • This is helpful when production code (a subset of [lib], [bin]s in a workspace) want to be held to a higher standard than development code (especially tests)
    • e.g. clippy has an unwrap_used lint but people might want that in tests. clippy works around it today by having a allow-unwrap-in-tests setting
  • Since workspaces can have non-production libs and bins, it almost seems like this is more of user-defined lint settings groups and inheriting specific groups. Or put another way, more design work is needed.
• Workspace-wide lints are deferred until they are needed. See Tracking Issue for [lints] table RFC 3389 #12115 (comment) for ideas
  • See also User control over cargo warnings #12235
• We fingerprint all lints; we can optimize the fingerprinting later
  • See also Add a [lints] table to Cargo.toml rfcs#3389 (comment)

Unresolved Issues

• Are we still comfortable with our schema choice? Add a [lints] table to Cargo.toml rfcs#3389 (comment)
• Are we still comfortable with our precedence choice? Add a [lints] table to Cargo.toml rfcs#3389 (comment)

Feedback

Cases that don't work well:

• Some lints are crate specific, like only wanting missing_docs and missing_debug_implementations for [lib]s (comment)
• Some lints are package and maybe crate specific, like disallowed_methods in chore: dogfood Cargo -Zlints table feature #12178
• Explicit inheritance doesn't work well when you want to catch lints for example packages but want the example package to standalone from the workspace (comment)

Future Extensions

No response

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

[epage]

What do people think about having the unstable [lints] table only warn if its present and not enabled, rather than error?

This would make the behavior more as if we insta-stablized this feature and and a user then ran using an old version of cargo, giving an "unused field" warning.

I feel this would allow more people to try this feature out, increasing the amount of feedback we receive. A project could configure their [lints] table and have their CI warnings job use a nightly.

The biggest downside is that normally Cargo.toml features require the unstable feature to be enabled via the cargo-features field which wouldn't work here because that would also require a nightly compiler. Instead, this would be enabled via -Zlints. We would need to strip the [lints] table on publish. This is a large enough break from convention that I hesitate.

As an anecdote, I only used sparse registry once while its been stable but off-by-default because I regularly enough run using older versions of cargo and didn't want to figure out how to workaround the "this isn't stable" error.

[flip1995]

For future discussions, here are the links to the unresolved questions and the previous discussions:

• Are we still comfortable with our schema choice? Add a [lints] table to Cargo.toml rfcs#3389 (comment)
• Are we still comfortable with our precedence choice? Add a [lints] table to Cargo.toml rfcs#3389 (comment)
• Can we fingerprint only the lints for the tool being run? Add a [lints] table to Cargo.toml rfcs#3389 (comment)

(Feel free to include this in the issue body and hide/delete this comment)

[epage]

Thanks for adding in the links!

[thomaseizinger]

I feel this would allow more people to try this feature out, increasing the amount of feedback we receive. A project could configure their [lints] table and have their CI warnings job use a nightly.

One thing to note here is that new lints added to clippy nightly can then suddenly break your CI (unless you pin the nightly).

For this reason, we pin our clippy job to a stable Rust version and have a non-required beta clippy job to be made aware of new lints early.

I am really looking forward to this feature and I want to help get it stabilized but I wouldn't want to compromise on CI reproducibility because of it.

Perhaps pinning the nightly version is a good enough workaround though.

[oxalica]

The biggest downside is that normally Cargo.toml features require the unstable feature to be enabled via the cargo-features field which wouldn't work here because that would also require a nightly compiler. Instead, this would be enabled via -Zlints. We would need to strip the [lints] table on publish. This is a large enough break from convention that I hesitate.

I think we should ignored [lints] defined in dependency crates by default anyway, for both stable and nightly, just like the current --cap-lints=allow default. Warnings inside dependencies may not be solvable by end-users.

[roblabla]

A problem with erroring upon encountering the lint table without the associated feature flag is that it'll prevent users targeting a low MSRV from using the feature as soon as it gets stabilised.

[zblach]

One thing to note here is that new lints added to clippy nightly can then suddenly break your CI (unless you pin the nightly).
[...]
Perhaps pinning the nightly version is a good enough workaround though.

If you're not pinning versions in CI, you're asking for this potential trouble anyway.

[epage]

Looking at #9732, it appears we have precedence for treating these as warnings, rather than errors.

[epage]

The motivation for the unstable package.rust-version using a warning instead of an error: #8037 (comment)

[Nemo157]

It makes a lot of sense that when an unstable feature is not enabled then Cargo should mostly act as if it did not exist (other than maybe adding a note that there's a feature not enabled), I think the times when there had been errors added specifically for new features that were previously only warnings were a mistake. (Other prior art for this: unstable lints without their feature active warn that the lint is unknown, with a note about the feature).

[epage]

I think the times when there had been errors added specifically for new features that were previously only warnings were a mistake.

If its something like weak/namespaced features, there isn't always a way to map it to the old behavior, so an error makes sense.