Expect `lints.workspace = true` for crates in a workspace with a `[workspace.lints]` section

[JarredAllen]

What it does

If your workspace has a [lints] table specifying lints, then this lint checks that the lints.workspace = true value is set in the Cargo.toml of all packages in the workspace, to ensure that those lints are actually applied.

We also might want to check for the other workspace-inheritable things, but those should be separate lints since people might want to only enforce this for some of them, and I don't personally use any of them, so I'm less qualified to talk about that.

Advantage

Specifying lint levels once in the package workspace is nice, but it's easy to forget to add the corresponding field to the Cargo.toml for each package member, which can result in extra lints specified in the package workspace silently not being applied. Having this lint will allow me to be confident that those lints are picked up in all of my packages.

See my post on URLO asking about how to check for this for the problem it solves.

Drawbacks

Some people might deliberately not inherit the workspace table for all lints, enough that I feel like allow-by-default is probably the play (idk if this belongs in cargo or pedantic, but one of those feels right).

Example

If the workspace Cargo.toml has:

...
[workspace.lints.clippy]
# Unimportant what's here, just that something is set
# Also should apply if it's not clippy lints but some other linter
pedantic = "warn"
...

And the package Cargo.toml is missing a

[lints]
workspace = true

Then we can add it for them.

[torokati44]

Oh my gosh, I thought/hoped that this is implicitly on without having to opt in for every workspace member crate. 😱

[emilk]

I 100% want this lint, but with one small modification

If your workspace has a [lints] table specifying lints, then this lint checks that the lints.workspace = true value is set in the Cargo.toml of all packages in the workspace

I think the lint should enforce that all crates in the workspace has lint.workspace set to something., That is: it is fine to explicitly opt-out using lints.workspace = false, but NOT ok to omit lint.workspace completely

[Kriskras99]

@emilk that would be great, but currently setting lints.workspace to false is not allowed.

[epage]

So one one about this, you do not inherit from [lints] but [workspace.lints].

One way to reframe an opt-out to not require workspace = false is to check if the [lints] table, even if empty, is present. That still requires an explicit action.

[Walther]

Adding some thoughts as a casual user of clippy and cargo workspaces - as someone who has not been involved in the development and internals of either tool. In the past, I have configured clippy using the #![deny(clippy::)] attributes in lib.rs and similar relevant places where needed.

Recently, I heard that support has been added for configuring lints on the package and workspace level. For a new repository, I decided to give this a try, in order to avoid repeating myself in the workspace member packages. Figuring out how to do this turned out to be non-obvious.

First I opened the documentation for configuring clippy. Based on the instructions at the top of the page, I tried to use a clippy.toml file at the workspace root, as it seemed like the primary recommended method. I tried to add some group rules there like all = "deny" and pedantic = "warn" as a starting point. This got me errors like error reading Clippy's configuration file: unknown field `all`, expected one of with a list of direct lint names, and no other hints of what was going on. After a bit of poking around I thought maybe this file cannot be used for general settings¹. When writing this issue and verifying some details, I noticed that the clippy repository README has a mention "clippy.toml or .clippy.toml cannot be used to allow/deny lints", but this was missing in the official documentation.

Second, I tried to figure out how the configuration would work from a workspace Cargo.toml. The documentation for Lints Section in Cargo.toml is very brief, and only mentions adding a [lints.clippy] table to the file. Again I faced error messages: error: failed to parse manifest, this virtual manifest specifies a `lints` section, which is not allowed. After a bit of searching online I found out that for the workspace Cargo.toml, you have to use the [workspace.lints.clippy] table name instead. This was not mentioned anywhere in the clippy documentation, and was non-obvious to figure out / find.

Third, I tried to verify that the workspace configuration for the lints would work. I deliberately added some code that clippy would point out as needing improvement. Unfortunately, with all = "deny" and even pedantic = "deny", I seemed to be unable to trigger hard errors with cargo clippy at the workspace root, only warnings. Eventually I started browsing through the GitHub issues here, and found this one. Apparently the final missing bit was that each crate in the workspace has to manually opt in² to "inheriting" the workspace-level lint configuration. This was again non-obvious to figure out / find. Extra emphasis: to the user, this feels like a silent failure, and is thus extremely difficult to debug!

These stumbling blocks are perhaps "papercut bugs" - not very serious, and possible to learn your way out of with half an hour of searching and reading. However, I feel like maybe some user experience improvements could be possible.

Finally, I hope reporting these negative experiences is not taken too harshly - clippy is a wonderful tool, and I love seeing it improve over time 🧡 Thank you for all the great work!

Footnotes

1. Which is a bit of a shame perhaps? Compare to other language ecosystems and e.g. .eslintrc, the config file is often the one place for adjusting everything linter related for the project. As it currently stands, the user has to learn and understand what goes into Cargo.toml vs clippy.toml and why, and possibly look in both when trying to find something. Perhaps the clippy.toml config should gain the ability to allow/deny lints and groups as well in addition to it being supported in Cargo.toml? Difficult to say what the best way forward here would be. ↩

2. Although to be fair: apparently if you have first added the workspace-level lint configuration and then run cargo new --bin, the new package's toml will be created with the workspace = true pre-filled. That is nice - although I'm unsure if sufficient? Wouldn't the inheritance be nicer as an opt-out? ↩

[epage]

FYI I've created rust-lang/cargo#15579. There is still some steps we need to take before its ready to implement.

[epage]

@rustbot label +T-cargo