Tracking Issue for `[lints]` table RFC 3389 #12115

ehuss · 2023-05-09T15:14:02Z

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.

Testing instructions

Requires

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

Steps:

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

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.

The text was updated successfully, but these errors were encountered:

kornelski · 2023-05-12T20:24:12Z

Currently there's no mechanism for controlling Cargo's warnings. Could the [lints] be used to silence Cargo's own warnings too? Like this one?

epage · 2023-05-12T21:45:07Z

Yes, one of the future possibilities listed in the RFC is for cargo to have configurable lints.

feat: `lints` feature ### What does this PR try to resolve? Implement rust-lang/rfcs#3389 which shifts a subset of `.cargo/config.toml` functionality to `Cargo.toml` by adding a `[lints]` table. This **should** cover all of the user-facing aspects of the RFC - This doesn't reduce what flags we fingerprint - This will fail if any lint name as `::` in it. What to do in this case was in the RFC discussion but I couldn't find the thread to see what all was involved in that discussion - This does not fail if a `[lints]` table is present or malformed unless nightly with the `lints` feature enabled - The idea is this will act like a `[lints]` table is present in an existing version of rust, ignore it - The intent is to not force an MSRV bump to use it. - When disabled, it will be a warning - When disabled, it will be stripped so we don't publish it Tracking issue for this is #12115. ### How should we test and review this PR? 1. Look at this commit by commit to see it gradually build up 2. Look through the final set of test cases to make sure everything in the RFC is covered I tried to write this in a way that will make it easy to strip out the special handling of this unstable feature, both in code and commit history ### Additional information I'd love to bypass the need for `cargo-features = ["lints"]` so users today can test it on their existing projects but hesitated for now. We can re-evaluate that later. I broke out the `warn_for_feature` as an experiment towards us systemitizing this stabilization approach which we also used with #9732. This works well when we can ignore the new information which isn't too often but sometimes happens. This does involve a subtle change to `profile.rustflags` precedence but its nightly and most likely people won't notice it? The benefit is its in a location more like the rest of the rustflags.

In rust-lang#12115, we explored how we can let stable projects experiment with `[lints]` to provide feedback. What we settled on is switching from the `cargo-features` manifest key to the `-Z` flag as `cargo-features` always requires nightly while `-Z` only requires it when being passed in. This means a project can have a `[lints]` table and have CI / contributors run `cargo nightly check -Zlints` when they care about warnings.

fix(lints): Switch to -Zlints so stable projects can experiment ### What does this PR try to resolve? In #12115, we explored how we can let stable projects experiment with `[lints]` to provide feedback. What we settled on is switching from the `cargo-features` manifest key to the `-Z` flag as `cargo-features` always requires nightly while `-Z` only requires it when being passed in. This means a project can have a `[lints]` table and have CI / contributors run `cargo nightly check -Zlints` when they care about warnings. ### How should we test and review this PR? Demonstrate how you test this change and guide reviewers through your PR. With a smooth review process, a pull request usually gets reviewed quicker. If you don't know how to write and run your tests, please read the guide: https://doc.crates.io/contrib/tests ### Additional information I considered reworking the code to show the user the errors they would encounter once the feature is stable but held off. I wasn't quite sure what language to use and most likely a user would have something doing error reporting, like CI, so it should be fine.

weihanglo · 2023-09-11T15:11:31Z

This is now stabilized on nightly from nightly-2023-09-10 and later!

extrawurst · 2023-10-18T10:34:31Z

@weihanglo what is the expected timeline to stabilize it?

jhpratt · 2023-10-18T10:39:35Z

what is the expected timeline to stabilize it?

It is stable on nightly. It will reach the stable compiler in 1.75.

mcandre · 2023-10-22T23:08:05Z

Dumb question, is the lints section in a Rust crate inherited by downstream projects?

I love linting, but I am wary of accidentally forcing a particular lint configuration onto my users.

weihanglo · 2023-10-23T00:40:26Z

If my memory hasn't rusty, then yes. Dependencies will compile with their lint configurations. However, users are not expected to see any compilation failure since Cargo uses --cap-lints to stop dependency warnings from propogating to users. See relevant code below:

cargo/src/cargo/core/compiler/mod.rs

Lines 883 to 894 in d2f6a04

 fn add_cap_lints(bcx: &BuildContext<'_, '_>, unit: &Unit, cmd: &mut ProcessBuilder) { 

 // If this is an upstream dep we don't want warnings from, turn off all 

 // lints. 

 if !unit.show_warnings(bcx.config) { 

 cmd.arg("--cap-lints").arg("allow"); 

 // If this is an upstream dep but we *do* want warnings, make sure that they 

 // don't fail compilation. 

 } else if !unit.is_local() { 

 cmd.arg("--cap-lints").arg("warn"); 

 } 

 }

Depends on a newly stabilized feature which will be in 1.75 and available on nightly today rust-lang/cargo#12115 (comment)

jwodder · 2023-11-01T14:57:34Z

(I'm not sure if this issue is the best place to ask this; if there's a better location, please let me know.)

Does the syntax for the [lints] tables only allow per-lint settings, or can it also be used to, e.g., deny all warn-by-default lints — perhaps something like lints.clippy.warn = "deny"?

epage · 2023-11-01T15:29:38Z

warn is a lint group and lint groups are supported. You need to make sure the priority is set correctly so it is correctly applied.

However, I would recommend against it. In general, #![deny(warnings)] (and by extension doing it in the lints table) is generally frowned upon because

whether you see warnings is dependent on the rust version used so someone using a different Rust version than you might not be able to build your project
it means your code always has to be "perfect" even when you are experimenting on something

I would instead recommend looking at #8424.

flip1995 · 2023-11-02T11:24:34Z

warn is a lint group and lint groups are supported.

Small correction: warn is not a lint group in Clippy. But all is, which contains all warn-by-default lints. What epage wrote still applies though: #![deny(warnings)] and by extension #![deny(clippy::all)] is an anti-pattern. So to deny warn-by-default Clippy lints, you should use the same alternatives as listed in the linked documentation.

jwodder · 2023-11-16T14:48:34Z

whether you see warnings is dependent on the rust version used so someone using a different Rust version than you might not be able to build your project

Just to be clear on this point: Does the [lints] table have an effect when another user builds a crate that uses mine or installs my binary crate? Or does it only take effect when building my crate directly (e.g., in a clone of the source repository)? Does this differ between the rustc lints and the clippy lints?

MarijnS95 · 2023-11-16T15:31:32Z

cargo new will automatically inherit the lints table

Is there (going to be) a way to force workspace lints on all member crates? Having to add [lints] workspace = true to every Cargo.toml in my workspace is not only verbose, it also turns this into an opt-in (that we'd have to watch out for very carefully in PRs that add new crates) rather than being a set of rules that are unconditionally enforced on all crates in our repo / workspace.

epage · 2023-11-16T15:48:08Z

#12976 was just merged which added the following to the documentation

Generally, these will only affect local development of the current package.
Cargo only applies these to the current package and not to dependencies.
As for dependents, Cargo suppresses lints from non-path dependencies with features like
--cap-lints.

epage · 2023-11-16T15:49:57Z

cargo new will automatically inherit the lints table

Is there (going to be) a way to force workspace lints on all member crates? Having to add [lints] workspace = true to every Cargo.toml in my workspace is not only verbose, it also turns this into an opt-in (that we'd have to watch out for very carefully in PRs that add new crates) rather than being a set of rules that are unconditionally enforced on all crates in our repo / workspace.

#12208 is our general issue for considering implicit inheritance. If we should consider piece-meal implicit inheritance, I think that should also be discussed there.

ehuss added the C-tracking-issue Category: A tracking issue for something unstable. label May 9, 2023

ehuss mentioned this issue May 9, 2023

Add a [lints] table to Cargo.toml rust-lang/rfcs#3389

Merged

ehuss added S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. A-lints-table Area: [lints] table labels May 9, 2023

This comment was marked as resolved.

Sign in to view

This was referenced May 15, 2023

Even with resolver=2 features are unified when starting from project root #12139

Closed

feat: lints feature #12148

Merged

This comment was marked as resolved.

Sign in to view

This was referenced May 22, 2023

fix(lints): Switch to -Zlints so stable projects can experiment #12168

Merged

Automatically inherit workspace fields when running cargo new/init #12069

Merged

bors closed this as completed in e539380 Sep 9, 2023

thomaseizinger mentioned this issue Sep 11, 2023

replace custom-clippy alias with workspace-wide [lints] configuration libp2p/rust-libp2p#4484

Closed

JonasWanke mentioned this issue Sep 21, 2023

Stricter lints candy-lang/candy#636

Open

Veykril mentioned this issue Sep 22, 2023

Refactor/fix clippy lints rust-lang/rust-analyzer#15615

Merged

mkeeter mentioned this issue Sep 23, 2023

Remove unused async oxidecomputer/crucible#943

Merged

This was referenced Sep 26, 2023

Support defining enabled and disabled lints in a configuration file #5034

Closed

Tracking Issue: Exposing RUSTFLAGS in cargo #12739

Open

Enforce crate naming rules of rustc #2708

Closed

jayvdb mentioned this issue Sep 29, 2023

Integrating configuration into Cargo.toml ericseppanen/cargo-cranky#10

Open

est31 mentioned this issue Oct 4, 2023

Stabilize --json=unused-externs(-silent) rust-lang/compiler-team#674

Closed

3 tasks

FirelightFlagboy mentioned this issue Oct 24, 2023

Simplify cargo-clippy step Scille/parsec-cloud#5616

Closed

dignifiedquire added a commit to n0-computer/iroh that referenced this issue Oct 26, 2023

lints: use [workspace.lints]

d6cb289

Depends on a newly stabilized feature which will be in 1.75 and available on nightly today rust-lang/cargo#12115 (comment)

dignifiedquire added a commit to n0-computer/iroh that referenced this issue Oct 26, 2023

lints: use [workspace.lints]

915daf4

Depends on a newly stabilized feature which will be in 1.75 and available on nightly today rust-lang/cargo#12115 (comment)

ianrrees mentioned this issue Nov 16, 2023

Unstable lints in v0.30.1 rust-embedded/svd2rust#760

Closed

bhavyagosai mentioned this issue Nov 20, 2023

dev: run cargo clippy --all -- -W clippy::all -W clippy::pedantic -W clippy::restriction -W clippy::nursery -D warnings and fix the repo, then add this command in the CI kkrt-labs/kakarot-rpc#584

Closed

torokati44 mentioned this issue Jan 4, 2024

chore: Move lint configuration from .cargo/config.toml to Cargo.toml ruffle-rs/ruffle#14614

Merged

joshlf mentioned this issue May 3, 2024

Someday Maybe google/zerocopy#1120

Open

38 tasks

laralove143 mentioned this issue Jun 3, 2024

Update lints used in Twilight twilight-rs/twilight#2350

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tracking Issue for `[lints]` table RFC 3389 #12115

Tracking Issue for `[lints]` table RFC 3389 #12115

ehuss commented May 9, 2023 •

edited by weihanglo

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

kornelski commented May 12, 2023

epage commented May 12, 2023

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

weihanglo commented Sep 11, 2023

extrawurst commented Oct 18, 2023

jhpratt commented Oct 18, 2023

mcandre commented Oct 22, 2023

weihanglo commented Oct 23, 2023

jwodder commented Nov 1, 2023

epage commented Nov 1, 2023 •

edited

Loading

flip1995 commented Nov 2, 2023 •

edited

Loading

jwodder commented Nov 16, 2023

MarijnS95 commented Nov 16, 2023

epage commented Nov 16, 2023

epage commented Nov 16, 2023

Tracking Issue for [lints] table RFC 3389 #12115

Tracking Issue for [lints] table RFC 3389 #12115

Comments

ehuss commented May 9, 2023 • edited by weihanglo Loading

Summary

Testing instructions

Updates Since RFC

Unresolved Issues

Feedback

Future Extensions

About tracking issues

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

kornelski commented May 12, 2023

epage commented May 12, 2023

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

weihanglo commented Sep 11, 2023

extrawurst commented Oct 18, 2023

jhpratt commented Oct 18, 2023

mcandre commented Oct 22, 2023

weihanglo commented Oct 23, 2023

jwodder commented Nov 1, 2023

epage commented Nov 1, 2023 • edited Loading

flip1995 commented Nov 2, 2023 • edited Loading

jwodder commented Nov 16, 2023

MarijnS95 commented Nov 16, 2023

epage commented Nov 16, 2023

epage commented Nov 16, 2023

Tracking Issue for `[lints]` table RFC 3389 #12115

Tracking Issue for `[lints]` table RFC 3389 #12115

ehuss commented May 9, 2023 •

edited by weihanglo

Loading

epage commented Nov 1, 2023 •

edited

Loading

flip1995 commented Nov 2, 2023 •

edited

Loading