Allow/deny lints in clippy.toml

[BenjaminGill-Metaswitch]

I'm currently trying to use Clippy for a large multi-crate project. An issue I've found is enabling additional lints: if I want to enable missing_docs_in_private_items, I've got to add #![allow(unknown_lints)] and #![warn(missing_docs_in_private_items)] to every main.rs and lib.rs (i.e. >10 places).

It would make clippy more usable for large projects if one could allow/deny/warn lints in clippy.toml, e.g.

warn = ["missing_docs_in_private_items"]

[oli-obk]

For cargo clippy this is easy. I'm not sure what to do in the plugin case though

[BenjaminGill-Metaswitch]

For comparison, Rustc allows one to set warning levels via environment variable: RUSTFLAGS="-W missing_docs".

Cargo then has a config file (.cargo/config), in which you could put the following:

[build]
rustflags = ["-W", "missing_docs"]

As I understand the way Cargo works, Cargo uses this config to construct the RUSTFLAGS environment variable and pass it into Rustc.

Might it be possible to do something similar for Clippy?

[Manishearth]

We can do the same for cargo clippy, but not the clippy plugin.

[casey]

@BenjaminGill-Metaswitch: Would my suggestion in #1340 be good enough? This would also make it clear that this can be done with cargo clippy, but not the plugin, since only cargo clippy parses command line options.

[mcarton]

@casey The problem with command line options is that you need to retype them every time (or find them in you shell's history), and you need to document them for other users. Having this persistent in a conf file would be more convenient.

[rajatgoel]

It would be great if clippy.toml supports overriding levels too (as suggested in the original in this issue). We are in the same boat, tons of crates (100+ at the moment) and we would like to share same setting across them.

cargo clippy solution doesn't work for us because we directly use rustc (via bazel) to build these crates.

[pravic]

+1.

It would be nice to have all lint settings in clippy.toml. But I would place each lint at separate line:

absurd_extreme_comparisons = "warn"
almost_swapped = "warn"
assign_ops = "allow"
needless_return = "allow"

instead of

warn = ["absurd_extreme_comparisons", "almost_swapped"]
allow = ["assign_ops", "needless_return"]

Because it would be hard to maintain (especially via VCS) long lists of settings.

[laumann]

I would like to add my ➕1️⃣ here as well. I used clippy here and with a few example programs I had to add individual cfg_attr in a few places (see this commit).

I would like to have both options from @pravic's suggestion. For my current use case, I would go for the latter form with:

allow = ["ptr_arg", "type_complexity", "needless_pass_by_value", "many_single_char_names"]

but I can see how both forms could be useful (resolving the use of both forms might be more complex though).

EDIT I'll also add my ➕💯 for clippy being a great tool - and easy to use!

[gnzlbg]

Ideally cargo clippy would try to find the clippy.toml file located in the closest parent directory of the file being processes. That way one can have a clippy.toml in the root of a project that would apply to all modules and crates in that project, while still allowing using a different clippy.toml for a particular sub-crate / sub-module, for example, for the examples/ directory in a library crate.

The clippy.toml could include an option to tell cargo clippy to find the "parent" clippy.toml file to that one as base, and apply the current options as an override.

---

EDIT: sed s/clippy/cargo clippy/

[oli-obk]

Isn't this something rustc/cargo should provide? Lints aren't something clippy-specific. Allowing/denying them is currently only possible with attributes, I don't see why we couldn't have a --lint-file=foo.toml option on rustc which is auto-filled by cargo if a lints.toml exists.

[gnzlbg]

Sorry! By clippy above I meant cargo clippy, I never use clippy (nor the clippy plugin) directly.

[oli-obk]

@gnzlbg I wasn't specifically targeting your comment. This entire issue feels, after re-reading it, like it should be part of rustc, not clippy, not even cargo clippy. There are probably rustc lints that one would like to enable across all projects.

[gnzlbg]

Makes sense, is there an issue filled about this in rustc?

[oli-obk]

There is now: rust-lang/rust#45832

[phansch]

This is now tracked in cargo: rust-lang/cargo#5034

[phansch]

Going to close this as there's nothing to be done on the Clippy side currently.

[dansgithubuser]

This was closed two years ago and the descendent issue (rust-lang/cargo#5034) is inconclusive and hasn't had an update since November.

Am I underestimating how easy it is to get this in to clippy? Can we save everyone the pain of waiting for this feature somewhere else by writing some temporary code?

Edit: writing a custom cargo clippy invocation into a custom script solves for my use case.

Edit: allow flags not working, not sure why...

[flip1995]

It would be easier to implement this in cargo, instead of in Clippy. There's not really a reason, why this should first be implemented in Clippy, instead of directly in cargo. Clippy is not a temporary-test-code repository ;)

[dansgithubuser]

It's a piece of software with features that users use. Right now it doesn't have a feature because it's punted to someone else and they are moving slow (and cargo should move slow because it's so core).

I completely accept that time is limited, but seemed to me like this had been forgotten, or maybe more sensible workarounds had been found but not collected here.

[flip1995]

What I wanted to say is, that it wouldn't make sense for someone who wants to implement this, to start here in Clippy. If someone wants to put time into this, then it should be done directly in cargo.

IIUC the feature was accepted in the cargo repo. Now someone has to step forward and implement this. There's no point in writing throwaway code for an accepted feature. It wouldn't be easier to implement this in Clippy either.

[pravic]

We already have clippy.toml with some configuration, why move it to the Cargo.toml (which is complicated enough already)? AFAIK, rustfmt.toml is still a separate file - and rustfmt and clippy are separate tools also.

[flip1995]

clippy.toml is for configuration of Clippy lints, not enabling/disabling lints during compilation (decided in the Clippy 1.0 RFC). The lint configuration should not be included in the Cargo.toml, but in a lints.toml (or similar). This requires a toml parser for the lint enabling/disabling syntax. For this, it doesn't matter if the parser is implemented in Clippy or cargo. By implementing it in cargo we can control lints not only from Clippy, but also rustc (and possibly other lint tools in the future).

[ghost]

Please consider re-opening this issue.

(in what follows, I've used "clippy" regardless of whether I mean cargo clippy)

According to this issue, having lint deny/allow config done at the cargo-level is non-trivial because it's possible that some lints are "deny by default". Therefore, anybody downloading the crate would also need the lint config because (if you explicitly allowed a deny-by-default lint) otherwise rustc might refuse to compile your crate.

However, this is not a problem clippy has. Clippy is separate, optional, and purely a developement/CI aid. Even if some clippy lint is deny-by-default it cannot possibly break publishing of crates.

More generally, this is a huge PiTA every time it comes up for me. I just want to flexibly configure how my development tool behaves so that it's consistent across (a) me manually running it (b) my IDE integration running it and (c) my CI (d) other developers machines. I often want to do this across several crates at once (all having the same config, but not necessarily in the same workspace). Right now, there are exactly two ways to deny/warn/allow clippy lints: command line arguments or in the source of the crate. Both are woefully inadequate compared to the excellent tooling Rust is known for (and even most other lint tools for other languages I've used).

Apologies that this is a bit of a rant, but honestly this is a bigger problem than you seem to think. I also don't see why clippy shouldn't have a separate way of configuring this, even if cargo sorts out a way of doing it. It's a separate, optional, tool and therefore has an opportunity to be so much more flexible (and already has its own config file).

[Manishearth]

We understand the need for this very well, no need to repeat that.

architecturally speaking it's tricky for Clippy to be in charge of this due to Clippy being invoked by Cargo. That's one of the reasons why we're reluctant to do this, but also in general we'd like all lints to be able to use this.

If it's hard from the cargo side, too, I'm open to doing this. But:

Therefore, anybody downloading the crate would also need the lint config because (if you explicitly allowed a deny-by-default lint) otherwise rustc might refuse to compile your crate.

I don't understand, cap-lints seems to solve this problem completely?

I would rather this be pursued on the Cargo side as much as possible. If it's really not possible, then yeah, perhaps we should figure out how to do it on the Clippy side.

[flip1995]

This is tracked in #6625. We want this, but currently don't have the resources to do it (or noone picked it up). To do this in cargo/rustc it will require a RFC, figuring out where this configuration file should live, what the syntax should be and so on. Doing this in Clippy would bypass this process and wouldn't make it possible to configure rustc lints.

I don't think it is hard implementing it in cargo technically speaking. But the design process is really involved, since it will have to go through the RFC process and many people have many different opinions on this.

I wanted to write up a draft of this RFC for a long time, but just didn't get to it yet...

Therefore, anybody downloading the crate would also need the lint config because (if you explicitly allowed a deny-by-default lint) otherwise rustc might refuse to compile your crate.

I also don't see why this is a problem. Dependencies are compiled with --cap-lints=allow and if you compile a crate from source, the config file should be included.

Also saying, that not shipping the config file would break the build because you only keep one locally, is kind of like saying not shipping the Cargo.toml breaks the build, because you only keep that locally.

[ghost]

Apologies. This was the issue that came up when I searched and reading through it seemed to be rejected and dormant. But nonetheless, #6625 is again about having Cargo take on this configuration role (with all of the aforementioned really involved process, making it difficult to get anything moving).

As for why it's non-trivial in cargo, I was just repeating what I understood from here and I don't think I'd really understood that --cap-lints=allow was used when compiling crates you depend on.

I'll take your word for it that it's architecturally tricky for clippy to implement this, but given that clippy already has its own configuration file this isn't obvious at all. Clippy already has its own way to configure how a link is applied, extending that to whether it's applied would seem relatively simple (a priori, again I trust you know about details that I don't).

[Manishearth]

The tricky part is not in clippy being configurable, the tricky part is in the clippy codebase controlling lint levels, due to the architecture of clippy and lints in general

[jmaargh]

I have a proof-of-concept branch here: https://github.com/jmaargh/rust-clippy/tree/poc/lint-config

Obviously it's far from ready to be merged (both in terms of code quality and features), but shows how and where clippy could implement this feature independent from cargo.

[flip1995]

We know that it is possible to do it in Clippy by passing down command line flags to rustc. But we don't think we should do it. Not only is this just a hacky way of doing it, but it also introduces a new configuration file, that'll probably be removed once this is done properly in cargo. This will add the burden of us going through an endless deprecation process. In addition to that it already implicitly specifies how this configuration file should look and there isn't any agreement on that yet. In addition to that this file will only be picked up if Clippy is run and will probably cause confusion if users try to specify lint levels for rustc. The procedural and practical downsides are just to big to just hack this into Clippy.

The clippy.toml file already only exists, because there is no way to also do this properly on a cargo level. This file was never stabilized, but people just expect that it is stable and we can't really deprecate it anymore (not that we want to do this now, but we want to include this in a general lint configuration file at some point), which results in Clippy always having code for handling this in its codebase for backwards compatibility.

So: We know that is technically possible. But this bypasses the RFC process that is necessary to do this properly.

[camsteffen]

This feature would be more useful as a Cargo feature since you could enable non-clippy lints.