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

clippy --fix -- -Dwarnings does not fail on warnings #13502

Open
ilyagr opened this issue Oct 5, 2024 · 2 comments
Open

clippy --fix -- -Dwarnings does not fail on warnings #13502

ilyagr opened this issue Oct 5, 2024 · 2 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@ilyagr
Copy link

ilyagr commented Oct 5, 2024

Summary

I have cargo clippy -- -Dwarnings correctly fail on warnings.

However, cargo clippy --fix -- -Dwarnings succeeds on warnings (that is, exists with 0 error code).

I'm not sure whether #8424 will help here. Cc #1209 where I got the idea for -Dwarnings.

I'm not sure whether this is a bug or a surprising behavior, but if it is intentional, it should be
mentioned in cargo clippy --help or somewhere, as well as somewhere in https://doc.rust-lang.org/nightly/clippy/usage.html#command-line.

Reproducer

Any code with warnings, e.g.

#![allow(clippy::style)]

#[warn(clippy::double_neg)]
fn main() {
    let x = 1;
    let y = --x;
}

Version

rustc 1.83.0-nightly (7042c269c 2024-09-23)
binary: rustc
commit-hash: 7042c269c166191cd5d8daf0409890903df7af57
commit-date: 2024-09-23
host: aarch64-apple-darwin
release: 1.83.0-nightly
LLVM version: 19.1.0

Additional Labels

No response

@ilyagr ilyagr added the C-bug Category: Clippy is not doing the correct thing label Oct 5, 2024
@Coekjan
Copy link

Coekjan commented Oct 5, 2024

I think this might be an expected behavior. The exit code indicates whether the process works successfully. Here is my thought:

  1. When there is no --fix, clippy is a checker and raises errors if it finds something bad.
  2. When there is --fix, clippy modifies the code and raises errors only if it fails to fix.

Correct me if I am wrong.

@ilyagr
Copy link
Author

ilyagr commented Oct 5, 2024

It's a good point, it seems that clippy --fix does not report an error code even if there are errors (e.g. replace warn(..) with deny(...) in my example). In fact, it seems to convert errors to warnings, so it's doing whatever the opposite of -Dwarnings is.

However, I don't think this always makes sense. If clippy --fix reports something, it is by definition something it couldn't fix and probably didn't even try. I expected this to be considered a problem, especially if I explicitly added -Dwarnings.

If the default behavior is considered correct, it'd be nice to have a well-documented option to change it. In fact, if -Dwarnings was respected and this was documented, this could be the option (together with whatever option is needed to treat errors as errors as well).

My usecase, by the way, was to use this with git branchless test fix. I was hoping for it to stop on the first error clippy --fix couldn't fix so that I could fix it manually. I can imagine others wanting clippy --fix to keep going and fix as much as possible, but I think my use-case is also reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

No branches or pull requests

2 participants