I'm trying out Rust 1.74's support for [lints] in Cargo.toml, and there's one use case I want that I can't figure out how to implement. I want clippy and cargo check to deny all of rustc's warn-by-default lints, and I also want some clippy lints (e.g., clippy::todo) to be at "warn" level. Unfortunately, these two things seem to be in conflict.
A simple linty src/main.rs:
fn main() {
todo!();
let x = 1;
}
linted with this in Cargo.toml:
[lints.rust]
warnings = "deny"
[lints.clippy]
todo = "warn"
correctly errors for the unused let x = 1 line, but the todo!() is marked as an error rather than a warning.
Setting the priorities for the lint levels — regardless of which one is higher — doesn't make a difference.
Removing the warnings = "deny" line correctly warns about the todo!(), but now the dead code is only warned about.
I get the same behavior running cargo clippy -- -D warnings -W clippy::todo (also with the options swapped), so this isn't specific to the [lints] table.
Is there a way to get what I want without having to manually list & deny each & every rustc warning lint?
How does using -D warnings only in CI deal better with new lints in newer Rusts? If a new lint breaks my code, the build will break, and I'll have to fix it. If I deny warnings locally, the build will break locally, and I'll have to fix it.
In a "perfect" world, you deny warnings in CI on a pinned toolchain version, which is then updated regularly and promptly, along with any necessary updates to fix the new warnings.
Even without this pinning, though, denying warnings on CI is preferable to denying them locally, because either way the lines block a PR merge, but if they only break CI, you can incrementally fix warnings with the local build without preventing running tests locally.
You know warnings are forbidden, so you don't need the compiler to refuse to run tests in order to know there are still necessary fixes to be applied. In local development, deny lints should only be for lints which have reason to prevent a local build. In CI, deny lints should be for lints which have reason to prevent a merge.
This calculus is based on a PR workflow, even for solo projects. ("HEAD is always green" is not rocket science. "Merge when green" is now a native feature of GitHub, so you no longer need a separate bot for it.) If you push directly to remote HEAD there's less of a distinction to make.
And if I believe that those lints include everything that rustc warns-by-default on, then what? Am I supposed to individually list all 115 lints in my Cargo.toml? I'd much rather be able to do list.rust.warning = "deny" without it affecting non-rustc lints, which is what this topic is about.
The warnings group is including "[a]ll lints that are set to issue warnings", which also includes external lints which are set to issue warnings. There is no "rustc-warn-by-default" lint group, and, indeed, it’s not possible to do what you want without denying manually all the 115 lints. For that, a new, separate lint group which includes all the 115 lints is required. To be honest, I’m not sure how useful this actually is, given that there is pretty much no reason (EDIT: or rather, I fail at finding a good reason) to prevent a local build because of warnings. The compile errors and deny-by-default lints of rustc are generally already more than enough, and as mentioned by others, the general good practice is to deny / forbid the remaining of the lints in CI.
You could open a thread in the internals forum and discuss adding such a group if you have a compelling argument.
I am genuinely curious what is the rationale for that.
Imagine OP has accepted the recommendation to only do this in CI. How would you go about that?
If you -D warnings that includes all of the clippy warnings that you want to stay warnings. The only way I have found to get the right behaviour is to do the entire build twice with different RUSTFLAGS, which means clippy can't reuse any of the build graph from the check part, and hence the entire thing takes double the time.
I understand the recommendation given previously was also about denying all the warnings, including the ones from clippy. However, I think you bring up a good point. I’ve seen GitHub actions for running clippy on modified code only, adding annotations like this:
Denying rustc warn-by-default warnings while keeping clippy lints as simple warnings this way, effectively as "automatic FYI review comments", addressing them on a best-effort basis could be useful to some folks, especially for big old codebases. I also know that some projects like rust-analyzer are not keen on enforcing clippy in CI, but applying the suggestions is still welcomed as long as it clearly improves the code:
Of course, applying Clippy suggestions is welcome as long as they indeed improve the code.
I doubt that adding a new lint group is very difficult, and making this use case easier and less slow may be a good reason enough to make it happen?
I'm not 100% sure you need to set all the clippy groups back to warn, but I'm guessing you do. I'm also not 100% sure you need to set the priority; maybe setting the clippy groups overrides the rustc one. But it shouldn't hurt.
You might also want to set warnings to -2 and the clippy groups to -1.
