Support defining enabled and disabled lints in a configuration file

[Rantanen]

This issue started its life in clippy (rust-lang/rust-clippy#1313). Once it was realized this should apply to built-in lints as well, it migrated to rust (rust-lang/rust#45832). There @kennytm suggested that it should be Cargo's responsibility to pass the -D/-W/-A flags to rustc.

Motivation

Currently project-wide lint configuration needs to be included in the crate sources. This is okay when the project consists of a single crate, but gets more cumbersome when the project is a workspace with dozen crates.

Being able to define the lints in an external file that will be used when building the crates would have several benefits:

• Sharing one configuration file for all the crates in a workspace.
• A canonical place to check for lint configuration when contributing to a new project.
• An easy way to examine the version history for lint changes.

Proposal

• A configuration lints.toml (or [lints] in Cargo.toml?) file that specifies the lints.
• The lints can be specified on the workspace level and for individual packages. Anything on the package level will override the workspace setup on per lint basis.
• The configuration file is cfg/feature-aware and can specify different lints depending on features. Specifying clippy-lints will result in clippy complaining about unknown lints if clippy isn't used.

Also... unlike what @oli-obk suggested in the rust issue, personally I'd prefer the following format:

[lints]
allow_unused = "allow"
non_snake_case = "allow"

While this is more verbose than allow = [ "allow_unused", "non_snake_case" ], I think the diff benefits and the ability for the user to group logically similar lints together even if some of them are deny and some allow outweighs the increased verbosity.

Also if Cargo/rustc ever support lint configurations, this would be more future proof:

cyclomatic_complexity = { state = "allow", threshold = 30 }

Example configuration

[lints]
allow_unused = "deny"
non_snake_case = "allow"

[lints.'cfg(feature = "clippy")']
cyclomatic_compexity = "deny"

The feature syntax is lifted from Cargo's dependency syntax - at least that was the intention. Personally I'm not a fan of it, but I guess that's a limitation of toml. Don't really have a better proposal for it, given I don't really know what's available int he toml syntax. :<

[Mark-Simulacrum]

I am strongly in favor. We should also probably specify that unknown lints should be a warning, not an error, for the purposes of backwards compatibility.

[Rantanen]

Forgot to mention: I'm also interested in contributing code for the implementation, if there is consensus on what should happen. Also do Cargo changes, such as this, go through the RFC process?

[Mark-Simulacrum]

This seems potentially worthy of an RFC, but I could see it going either way. There doesn't seem to be much design space. @alexcrichton can probably say more.

[alexcrichton]

This seems plausible to me! I think it'd be fine to add this to Cargo.toml on an unstable basis (behind a nightly feature) and we can decide to stabilize at a later date.

[lukaslueg]

Do we have a chance for rustc to report where the linter settings are coming from? Given the increasing amount of knobs and switches, it would be very nice to have rustc report that allow_unused was denied and that deny was specified by the workspace's root Cargo.toml or the commandline.

As rustc knows nothing about these, we could introduce a new switch to rustc that specifies the source of the following linter settings (e.g. --LS "/home/myproject/foobar/Cargo.toml" -D ... --LS "/home/myproject/Cargo.toml" -D ...).

Is this a thing?

[oli-obk]

I'm not sure if these should be in the Cargo.toml, as we many want to be able to specify them globally for a group of projects.

[oli-obk]

Is this a thing?

rustc already reports whether something was

• defined by command line
• defined by default settings
• defined by code

so we just add another variant for config file there.

[phansch]

I'm really interested in this, too.

Also if Cargo/rustc ever support lint configurations, this would be more future proof:
cyclomatic_complexity = { state = "allow", threshold = 30 }

It could be cyclomatic_complexity = { state = "allow" } from the start, then we would have the option to add further options in a second step.

[phansch]

This Rollup PR from rust demonstrates how rust itself could benefit from a shared lint config, too: rust-lang/rust#52268 (note all the 'deny bare trait object' PRs; and there's a few more)

[detrumi]

I'd like to start working on this, unless @phansch or @Rantanen started on this already.

Seems like the lints.toml name is the one I've seen used most, so let's go with that.
As for the syntax, allow_unused = "allow" seems reasonable and more ergonomic than requiring the cyclomatic_complexity = { state = "allow" } syntax.

I'm not sure if these should be in the Cargo.toml, as we many want to be able to specify them globally for a group of projects.

A Cargo.toml file can already be used for a workspace, unless you meant a group of workspaces?

[Mark-Simulacrum]

I'd prefer that we centralize this into Cargo.toml rather than adding an additional file, but that can likely be decided in further detail at a later point in time.

[oli-obk]

The argument against Cargo.toml is that you can have a single lints.toml for multiple projects. Or even hierarchical lints.tomls

[phansch]

@detrumi I didn't start any work on this, and won't have that much time either. From my side, feel free to start 👍

[Mark-Simulacrum]

@oli-obk Right, which is what workspaces are for IMO. I'd prefer to avoid "one more" file in every Rust directory.

[oli-obk]

That would mean that companies need one enormous workspace for all the crates in their company if they want to have a consistent lint setup. This is the very use case that started the discussion about lint configuration files. A company-wide uniform linting experience. Maybe specializeable for crates, but they can already do that via #![allow(foo)] in their lib.rs. If we leave the door open for importing lint configurations in a dependency-style manner, doing it in Cargo.toml seems fine though.

[Luthaf]

Why not use .cargo/config here ? This seems like the obvious candidate for a file shared across repositories/workspaces/crates. Plus, one can commit a .cargo/config file per repository in OSS scenarios.

[detrumi]

Alright, placing it in Cargo.toml seems like a better place to start then.

That would mean that companies need one enormous workspace for all the crates in their company if they want to have a consistent lint setup.

Hierarchical Cargo.toml files might be a solution to the enormous workspaces, but that's something that can be added later.

[Rantanen]

I'd like to start working on this, unless @phansch or @Rantanen started on this already.

Feel free to! I have been waiting for some kind of a decision on how to proceed, so haven't done anything for this myself.

[sanmai-NL]

@oli-obk, @Mark-Simulacrum: My company has one enormous workspace indeed. And we are highly in need of DRY linting/warnings/etc. configuration with local exceptions (preferably down to the statement-level). So if industrial use cases/productivity are the motivation, perhaps we can survey a bit to come up with requirements in a more deliberate process?

[oli-obk]

down to the statement-level

that part is no problem, you can configure lints in source code just fine.

And if you have one enormous workspace, then the Cargo.toml solution will work just fine for you I think.

[sanmai-NL]

Suppose we settle on configuring this in Cargo.toml, then at a later stage we can design a more general feature, namely supporting Cargo.toml override files. Such override files could e.g. patch sections such as the lints and warnings configuration. We can then circumvent the discussion about the need for a separate configuration file. The name of the section is more important, since compiler warnings and clippy lints may be merged in some form later.

[flip1995]

I started writing up a RFC with the MVP @joshtriplett suggests. I never got to finishing it due to limited time. I plan to work on this in the near future though. However, I'm aware about the feature freeze of cargo. But I think we should get the ball rolling with making a community accepted design choice (should it be a config file, included in Cargo.toml, a mix of both, ...?) through an RFC.

[Muscraft]

if Cargo.toml is chosen it might be a good idea to extend workspace inheritance. It would make sharing lints easier.

If that is the case I think allow = [ "allow_unused", "non_snake_case" ] would be better and make toml parsing easier. Not having to make a field for each lint would be much better overall. Ideally, the Cargo.toml would look like below

# [PROJECT_DIR]/Cargo.toml
[workspace.lints]
allow = [ "allow_unused", "non_snake_case" ]

# [PROGJCT_DIR]/bar/Cargo.toml
[lints]
workspace = true
deny = [ "allow_unused" ]

I would have to think more about how it would fit with MaybeWorkspace but I think it gives a general idea of how it would work

[CobaltCause]

I don't know if this has been brought up before, but what about something a little more general like Python's pyproject.toml [tool] section from PEP 518? After having worked with Python for a little over a year, I found this rather pleasant to use since it means all tools (that support it) use the same format, in the same file, which helps with uniformity, ease-of-use, and reduces file clutter. Cargo already has [package.metadata] and [workspace.metadata] too which sound pretty similar.

[ParkMyCar]

Hey @flip1995, you mentioned you started on an RFC? I'd be more than happy to help, or pickup the work if you've become too busy?

[flip1995]

Yes, I started one some time ago, but can't find it anymore. 🤦 There is this attempt at an RFC: https://hackmd.io/@flip1995/HJteAAPgO But it is really sparse and not at a point where I would continue with it, but rather start fresh.

[emilk]

For anyone looking for a workaround for this issue, I have been using cargo cranky for the past six months, and I'm really liking it: https://github.com/ericseppanen/cargo-cranky

[Manishearth]

I'm also wondering if at this point clippy should just support it in clippy.toml.

We can then do fancy stuff like "disable lints that were introduced in version foo" (or "enable all lints up to version foo").

Ideally this can be done with an additional state for Rust's lint emission UI where it mentions that the lint level comes from a file instead.

[epage]

I've created an RFC for this: rust-lang/rfcs#3389

[epage]

Since #12115 is merged, I'm closing this as resolved.