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

[Rustin170506]

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:

➜  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]

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

[xFrednet]

@epage I believe this is a difference with the cargo check and cargo fix defaults.¹ 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.

Footnotes

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]

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]

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]

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.

[epage]

We discussed this in the Cargo team meeting today.

I propose that we change cargo fix from defaulting to all targets to

• cargo fix defaults to the same targets as cargo check
• If --edition or --edition-idioms is passed in, then you get the current cargo fix behavior

CC @rust-lang/clippy as this would impact cargo clippys behavior,

This issue is fundamentally a clippy issue as they control their CLI. This is a result of their current implementation (of the --fix flag changing them from wrapping cargo check to cargo fix). cargo fix was changed to support this behavior for edition migrations in #5739. As cargo clippy --fix is not related to edition migrations, the behavior doesn't make sense. The implementation could always change to give clippy more control over their default targets. I suspect it could be easier to fix with an idea I have for cargo fix to loop over cargo check. I've suggested making a prototype of that idea a GSoC project.

However, if we change cargo fixs behavior, then cargo clippy should automatically pick that up. The current behavior is motivated by edition migration but that is not the full extend of cargo fixs behavior but is exclusive to --edition and --edition-idioms. cargo fix --edition has a reasonable expectation for what its programmatic behavior is. cargo fix however is less clear as fixable code edits are less likely to be made programmatic and cargo fix may output broken code. Another potential reason for cargo fix --edition to maintain behavior is if we decide to deprecate <build-target>.edition fields in manifests, then edition migration will be on the package level so it makes sense to select the whole package.