Proposal: Cargo Lint configuration

tools and infrastructure cargo
1

Written in collaboration with @detrumi.

This is a rough plan for adding lint configuration by config file to Cargo.

It’s a continuation of this Cargo issue from @rantanen. Relevant parts were copied and used as a starting ground for this text. @detrumi opened a Cargo PR when the issue was opened. However, it was decided at the time that the design should go through more discussion first. That’s what this proposal is meant to kick-off.

Summary

Rust currently only allows configuring lint levels on the command line or via crate level attributes. This proposes an additional way to configure lint levels and possibly lint-specific settings in Cargo.toml.

Motivation

Configuring lint levels on the command line or via crate level attributes is fine when the project consists of a single crate, but gets more cumbersome when the project is a workspace with more than a few 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, therefore ensuring consistent lint levels everywhere.
  • 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.

Some real world examples of where this could bring improvements:

Previously this topic came up a few times in the Clippy issue tracker as well:

Guide-level explanation

Lint configuration can also be done through a configuration file. The configuration needs to be able to configure the allow/warn/deny/forbid lint levels for each lint. In order to be more teachable, using toml syntax is probably the best way to configure the lints. The most obvious location is in the Cargo.toml file.

Reference-level explanation

In the most basic version, it would be possible to set the allow/warn/deny/forbid of lints that are registered with the lint registry. These would be grouped in a new [lints] section. Tool-specific lints (like Clippy) are grouped together using the tool name.

That would give us a format like this:

[lints]
dead_code = "allow"

[lints.clippy]
non_snake_case = "warn"

This format would make git history easy to read and would allow you to add configuration options to lints later on. It also allows grouping of lints on more than just the lint level.

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

[lints.clippy]
cyclomatic_complexity = { state = "deny", threshold = 30 }

Another possible format would be:

[lints]
allow = ["dead_code", "non_snake_case"]

This has the benefit of not having to repeat the lint level for every single lint. However, this would probably make diffs more difficult to read.

cargo check and tool lints interaction

If a user has configured a tool lint in Cargo.toml and runs cargo check, the implementation needs to make sure to only pass the rustc lints to rustc, otherwise rustc would complain about unknown lints because it was invoked with check and not clippy, for example.

When cargo clippy is executed, it should include the rustc lints, as it does today.

Lint precedence

The lints can be specified on the workspace level and for individual packages. Anything on the package level will override the workspace setup on a per lint basis. This means that lints configured in the workspace will act as a default for packages within that workspace, and can be overridden by individual packages.

Why Cargo.toml?

The biggest advantage of using Cargo.toml is that it’s already used for configuring project and workspace level settings. Most users would expect to find lint configuration there and pretty much every Rust project uses it already.

However, one downside is that there is currently no way to share a Cargo.toml between separate projects. There’s no concrete solution here, but maybe a new inherit_from or inherit_lints_from key could solve that problem. Prior art includes at least Rubocop’s inherit_from setting and eslint’s extend setting.

Additionally, one might argue that Cargo.toml is part of Cargo, which should just be concerned about package management and not lint configuration. However, Cargo is currently the only tool available that interfaces with rustc and offers a way to configure interfacing with rustc.

For users it may also not be immediately clear that both cargo check and cargo clippy work with the same underlying lint system and they may not expect to be able to configure tool lints through a Cargo configuration file. This is because other programming language ecosystems usually have completely separate tools for their lints. See the Prior art section below for a small discussion of this problem.

If we find a solution to Cargo.toml not being shareable across projects, we consider it to be the best approach mainly because the file is already present in every Cargo project.

Drawbacks

  • Adding lint configuration to Cargo.toml means that there will be more places where lints can be configured, making it more difficult to quickly see what lint levels are used.
  • Another downside of this is that it becomes less clear which lint level takes precedence, if the same lint is set multiple times in different locations. Even if the precedence rules are useful for the majority of use cases, developers still need to be aware of the precedence, or avoid specifying the lint multiple times.

Alternatives

Using package.metadata tables

Another approach, that was proposed on IRLO recently, could be using the metadata configuration:

Cargo would look for a [package.metadata.lints] section instead of a [lints] section. Tools that are not hooking into the lint system, could use their own [package.metadata.toolname] section instead of using a custom file.

One problem with [package.metadata] is that it does not support virtual workspaces as it requires the package table to be present in the Cargo.toml.

Different configuration file location

There are some other plausible locations to configure lints, such as .cargo/config or Lints.toml.

.cargo/config

A more standard location, though less well-known, would be .cargo/config (see Cargo reference). As with Cargo.toml, this file would need a new [lints] section, too.

One big advantage would be that users could add their personal lint preferences in their home directory (both $HOME/.cargo/config and .cargo/config are supported) as .cargo/config already has a hierarchical configuration lookup.

A downside would be that Lint configuration via config file would supposedly be pretty common, while custom Cargo configuration is rarely used. Almost every project would want to make use of a lint configuration, which means that every project would end up having to create the additional .cargo/config file.

Lints.toml

Another option would be a completely separate file. Maybe called Lints.toml. This would be the most flexible solution because the implementation would not have to care about existing code for Cargo.toml or .cargo/config. It is also more similar to the existing Clippy.toml.

However, it would also mean that, like with .cargo/config, users have to add an additional configuration file to the roots of their repositories.

Additionally, we may also want to handle rustfmt configuration, and we would need to find a more general name.

Prior art

In other programming language ecosystems, the concerns of dependency management and things such as lint configuration are handled by completely separate tools. This is usually because the language itself does not come with any lints like Rust. For example, in Javascript, you have eslint and the package.json, which don’t really interact. In Ruby, you have Rubocop for lints and bundler/Gemfile for dependencies.

Rust is different from these examples because it already comes with built-in lints and offers an interface for external tools to make use of the same lint system.

If another language exists that provides external tools with hooks into its lint system, it would be good to take a look. Specifically how the interaction between external and internals lints is handled.

Future possibilities

  • It might be desirable to also be able to configure the settings of specifics lints, such as in the cyclomatic_complexity shown earlier. This could also replace the clippy.toml file, which currently allows configuring lints.
  • It would make sense to include other settings related to lints, such as output verbosity or --cap-lints in the lint configuration.
  • Other tools could also add lints to the same lints section, not just Clippy.
  • rustfmt configuration via Cargo.toml might be something that’s desired, too.

Diagnostics for lint level definition location reporting

With the current approach, we pass all lint level definitions from the config file to rustc via the command line arguments. Consequently, rustc will tell the user the lint level has been defined on the command-line. This is the opposite of helpful if the user wants to change the level of the lint:

error: any use of this value will cause an error
  --> $DIR/const-err-multi.rs:13:1
   |
LL | pub const A: i8 = -std::i8::MIN;
   | ^^^^^^^^^^^^^^^^^^-------------^
   |                   |
   |                   attempt to negate with overflow
   |
   = note: requested on the command line with `-D const_err`

Ideally, the user would get a note like this:

note: lint level defined here
  --> $DIR/Cargo.toml:511:1
   |
LL | const_err = { state = "deny" }
   | ^^^^^^^^^

Adding a new variant to LintSource is the easy part, but how is this information passed from Cargo to rustc? Should we pass a json file with lint level definition locations to rustc every time cargo invokes rustc?

Diagnostics for unknown lints reporting

There is currently no good way to detect whether a lint is known or not: all configured lints are passed to rustc directly, which throws an error if the lint isn’t known. Ideally, Cargo would issue a warning instead that includes the line number in the configuration file.

13 Likes
2

For future possibilities, I’d like to see “being able to force lints on dependencies” added.

3

I personally prefer having lint information directly in the source file or files, as that keeps it together with the source itself rather than as a compiler flag. It affects the interpretation of the source itself.

1 Like
4

I see that lint settings get repetitive for multiple-crate projects/workspaces, but putting them in Cargo.toml feels like using it because it already exists, rather than using it because it’s the right place for lints.

Clippy is now a part of the Rust project, so maybe Clippy.toml could be adopted as the place for all lints?

I also have mixed feelings about storing lint settings outside the crate. It’s convenient for the author, as the author may even set all the lints system wide from one file in their home dir, but that makes lints separated from the crate, so other contributors won’t know what lints to adhere to.

Maybe having to copy and paste a few lines once a in a while is not that bad, and solutions aren’t worth the added complexity…

5

I think the problem here is that there aren't currently any workspace-wide configurations except Cargo.toml (and sort of .cargo). A workspace is a cargo concept, and a separate Lints.toml or Clippy.toml file doesn't obviously map to a workspace (as a random developer, I would assume it would apply to the crate, not the whole workspace).

Another way of looking at solving this problem is by looking at how else code can be easily shared between the crates in a workspace. As an example, if clippy config were all in one file as code (using today's syntax), one could just include! that file if include! macro fails if included file has top-level inner attributes · Issue #752 · rust-lang/rfcs · GitHub were implemented - one line of boilerplate per crate, and the logic lives in one place. Another (slightly weirder) idea could be workspace-scoped macros, where a macro could be auto-visible to the containing workspace, for a similar effect.

6

We discussed this in the Cargo meeting today. We all felt broadly in favour of the general idea (i.e., there is a problem that needs solving), but felt there were a lot of open questions (which might be fine to work out on a proper RFC). We spent a lot of our time discussing the merits of Cargo.toml vs .config vs Lints.toml, etc. (and whether we should do something like this at all). We agree that if we support lint configuration in Cargo, then Cargo.toml is the best place for that data. An alternative is some kind of include mechanism for including a lint policy (i.e., not just individual lints) from an outside source. I believe the former will be simpler to use and implement. (Our reasoning is that there is no real benefit to having multiple config files rather than one large config file, and there are some drawbacks. .cargo does not seem appropriate since it makes sense to commit this information to version control).

I think it is worth considering how this might work with configuration for other tools, e.g., rustfmt - we support both rustfmt::skip and allow(clippy::foo) in the compiler, and I would expect that users would want to support generic tool configuration, not just lints in the fashion proposed.

Finally, while we do think this is a good idea, it is not likely to be on this years roadmap and we don’t consider it high priority for the Cargo team, so it would be good to think about who would implement this feature.

4 Likes
7

Just to clarify, you're saying that the team is generally in favour of something like this (pending an RFC to nail down the exact details), and would accept this as a change to Cargo by anyone who is willing to put in the work, even if it's not part of the 2019 roadmap, and the Cargo team won't pick this up themselves. Ccorrect?

If so, that's great to hear :sunny:

8

Yep, exactly that. There is a bit of an issue that if the PR is large, then finding review bandwidth might be a problem, but I think it would probably be OK.

9

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.