Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a flag to exit with a non-zero status code on warnings #12300

Closed
SamirTalwar opened this issue Feb 15, 2024 · 3 comments
Closed

Add a flag to exit with a non-zero status code on warnings #12300

SamirTalwar opened this issue Feb 15, 2024 · 3 comments

Comments

@SamirTalwar
Copy link

Description

We can now configure the various levels of each lint option in Cargo.toml. I currently have one that looks like this:

[workspace.lints.clippy]
all = { level = "warn", priority = 0 }
pedantic = { level = "warn", priority = 0 }
# disable some pedantic warnings which feel a little too pedantic
doc_markdown = { level = "allow", priority = 1 }
missing_errors_doc = { level = "allow", priority = 1 }
missing_panics_doc = { level = "allow", priority = 1 }
module_name_repetitions = { level = "allow", priority = 1 }
must_use_candidate = { level = "allow", priority = 1 }
similar_names = { level = "allow", priority = 1 }
wildcard_imports = { level = "allow", priority = 1 }
# these should probably be enabled
default_trait_access = { level = "allow", priority = 2 }
from_iter_instead_of_collect = { level = "allow", priority = 2 }
large_futures = { level = "allow", priority = 2 }
needless_pass_by_value = { level = "allow", priority = 2 }
unnecessary_wraps = { level = "allow", priority = 2 }
unused_async = { level = "allow", priority = 2 }

I want to run Clippy in pre-commit checks and CI too, and in these situations, I want it to fail (exit with a non-zero status code) if any warnings are encountered.

Currently I have two ways of doing this:

  1. using something like sed to rewrite Cargo.toml, replacing "warn" with "deny", or
  2. using flags to override the lints.

Neither of these seem great. Using sed is error-prone, especially when running it locally in a pre-commit hook, and if I use flags, I have to re-specify all of the "allow" lints, which means I probably have to generate the command-line arguments programmatically.

I would like a command-line argument that tells Clippy to fail on warnings instead. I suggest --exit-with-failure-on-warnings, though you may prefer something more terse.

I am happy to attempt to work on this feature myself, but I would like to discuss it to ensure that the maintainers of Clippy agree in principle first.

If I am missing an obvious way to achieve this, please let me know!

Version

rustc 1.76.0 (07dca489a 2024-02-04)
binary: rustc
commit-hash: 07dca489ac2d933c78d3c5158e3f43beefeb02ce
commit-date: 2024-02-04
host: aarch64-apple-darwin
release: 1.76.0
LLVM version: 17.0.6

Additional Labels

No response

@Alexendoo
Copy link
Member

Using --deny warnings is the current recommendation for this, e.g. through RUSTFLAGS

There is a proposal on the cargo side to add a flag for this - rust-lang/cargo#8424

@SamirTalwar
Copy link
Author

@Alexendoo, I didn't know that would work with Clippy warnings. Thanks!

This is a good solution for CI but as it's a change to the rustc flags, it requires recompiling all dependencies, which is kind of unworkable for a pre-commit check.

It sounds like the Cargo flag is the proposed solution already though, and reading through it, I get the impression it's intended to only apply to local crates, not dependencies, so I think it will address the above issue.

Do you think this issue should be closed, then, and converted to a comment on the Cargo issue?

@Alexendoo
Copy link
Member

You could try cargo clippy -- -D warnings for such a hook, I believe that would avoid recompiling dependencies.

Yeah the cargo issue would be the place to look at for a solution that works everywhere in cargo, but I think that should suit your needs on the clippy side of things.

As an aside priority in [lints] can be negative so you only have to set it for groups, e.g.

[workspace.lints.clippy]
pedantic = { level = "warn", priority = -1 }
# disable some pedantic warnings which feel a little too pedantic
doc_markdown = "allow"
missing_errors_doc = "allow"
# ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants