rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.25k stars 1.52k forks source link

pre-commit hook not aborting on unfixable warnings #13297

Open eismont21 opened 3 weeks ago

eismont21 commented 3 weeks ago

Description

I'm trying to integrate cargo clippy as part of my pre-commit hooks to enforce code quality in my project. The command I'm using is: cargo clippy --all-features --fix -- -D warnings

Here's the behavior I'm aiming for:

  1. If clippy identifies an issue, it attempts to fix it automatically.
  2. If the fix is successful, the changes are applied, and the pre-commit hook aborts the push.
  3. If clippy is unable to fix the warnings, I expect to receive errors, and the push should be aborted.

The first scenario works as expected—when clippy successfully fixes issues, the push is aborted due to new changes in the codebase, which is great. However, in the second scenario, if clippy can't fix the warnings, I don't receive any errors, and the push goes through as if everything is fine, even though it isn't. I do see these warnings in the log, but the check finishes successfully without any errors, so the push isn’t aborted as it should be.

How to ensure that the pre-commit hook catches when clippy can't automatically fix warnings?

Version

rustc 1.80.1 (3f5fd8dd4 2024-08-06)
binary: rustc
commit-hash: 3f5fd8dd41153bc5fdca9427e9e05be2c767ba23
commit-date: 2024-08-06
host: aarch64-apple-darwin
release: 1.80.1
LLVM version: 18.1.7
pre-commit 3.6.2

Additional Labels

@rustbot label C-question

xFrednet commented 3 weeks ago

There is some weirdness with rustfix, that it ignores lint levels set via console arguments. I believe you can go around that by using the RUSTFLAGS environment value instead. Though, I would suggest to only run the normal clippy. Running it in the background and updating source files could be unexpected for users in your project.

eismont21 commented 3 weeks ago

Thank you for the suggestion. I implemented it by updating the command as follows: RUSTFLAGS="-D warnings" cargo clippy --all-features --fix Unfortunately, the hook still finishes successfully even when warnings are logged, allowing the push to proceed.