rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.57k stars 2.38k forks source link

`cargo clippy` and `cargo clippy --fix --allow-dirty` have different reports #13527

Open Rustin170506 opened 6 months ago

Rustin170506 commented 6 months ago

Problem

I ran cargo clippy and cargo clippy --fix --allow-dirty in the same project but got different reports.

  1. cargo clippy
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.16s
  2. cargo clippy --fix --allow-dirty
    
    Checking cargo-information v0.4.2 (/Volumes/t7/code/cargo-information)
       Fixed tests/testsuite/cargo_information/git_dependency/mod.rs (1 fix)
    warning: use of a disallowed/placeholder name `baz`
    --> tests/testsuite/cargo_information/git_dependency/mod.rs:9:9
    |
    9 |     let baz = git::new("baz", |project| {
    |         ^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names
    = note: `#[warn(clippy::disallowed_names)]` on by default
warning: use of a disallowed/placeholder name foo --> tests/testsuite/cargo_information/git_dependency/mod.rs:15:9 15 let foo = project() ^^^

= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names

warning: cargo-information (test "testsuite") generated 2 warnings Finished dev profile [unoptimized + debuginfo] target(s) in 0.78s


### Steps

1. Clone https://github.com/hi-rustin/cargo-information
2. Rustup: `rustup toolchain install nightly-2024-02-21`
3. `cargo clippy` and check the result
4. `cargo clippy --fix --allow-dirty` and check the result

### Possible Solution(s)

None

### Notes

I am willing to investigate and fix this issue.

### Version

Cargo:
```console

➜  workspace git:(draft/mystifying-neco) ✗ cargo --version
cargo 1.78.0-nightly (7b7af3077 2024-02-17)

Toolchain:

active toolchain
----------------

nightly-2024-02-21-x86_64-unknown-linux-gnu (default)
rustc 1.78.0-nightly (bb594538f 2024-02-20)

Clippy:

➜  workspace git:(draft/mystifying-neco) ✗ cargo clippy --version
clippy 0.1.78 (bb59453 2024-02-20)
epage commented 6 months ago

I'm assuming this is more of a clippy issue but I do not have permission to transfer this issue to their repo.

xFrednet commented 1 month ago

@epage I believe this is a difference with the cargo check and cargo fix defaults.[^1^] I'm doing some testing on a similar issue, and it seems like cargo fix includes test files, while a normal cargo check run doesn't. The issue also lists additional warnings from the test files.

Thus far I've sadly been unable to confirm this suspicion, as I'm not quite sure where to check for the difference in Cargo's code.

One thing I can say, is that in my testing, the two comments cargo fix/cargo check behaved differently until I specified the --lib flag to only build the library crate.


Edit: Looking at cargo fix command I'm guessing that it selects all members in the workspace by default.

[^1^]: My understanding is, that cargo clippy --fix internally calls cargo fix with clippy set as the driver. cargo clippy is similar, that it internally calls cargo check with clippy as a driver

weihanglo commented 1 month ago

I believe this is a difference with the cargo check and cargo fix defaults.

That's a correct guess. cargo fix by default runs cargo check --all-targets. The cargo-fix help manual has mentioned that difference:

if you’d like to apply all fixes to the current package, you can run:

cargo fix

which behaves the same as cargo check --all-targets.

While it is expected, however still confusing. Unsure how we should address this.

xFrednet commented 1 month ago

Is there a reason why it uses --all-targets by default? For my use case, I'm currently trying to make it behave like the normal cargo check without the flag, but the only solution I found was to use --libs respectively --bins, which is sadly not generic for all kind of crates

epage commented 1 month ago

My best guess is that the UI was optimized for edition migrations which has been the primary focus of cargo fix until we started advertising it on warnings.