cargo clippy --fix seems to imply --tests which is at least undocumented

[tsdh]

Summary

After reading the rust 1.69 release notes, I wanted to give cargo clippy --fix a try and thereby found out that while cargo clippy had nothing to complain about in my sample project, cargo clippy --fix issued several warnings (which couldn't be fixed automatically). Turns out, all warnings were in #[test] functions. And indeed, cargo clippy --tests issued the same warnings.

So it seems like --fix implies --tests. Not a big deal but at least not quite expected and especially not documented, AFAICT.

❯ cargo clippy --version
clippy 0.1.69 (84c898d 2023-04-16)

Reproducer

I tried this code:

fn main() {
    println!("Hello, world!");
}

#[test]
fn test() {
    let foo = "clippy will complain about placeholder name foo.";
    println!("{foo}");
}

I expected the first two clippy invocations but not the third:

❯ cargo clippy
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s

❯ cargo clippy --tests
warning: use of a disallowed/placeholder name `foo`
 --> src/main.rs:7:9
  |
7 |     let foo = "clippy will complain about placeholder name foo.";
  |         ^^^
  |
  = 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: `clippy-bug` (bin "clippy-bug" test) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s

❯ cargo clippy --fix --allow-dirty
    Checking clippy-bug v0.1.0 (/home/horn/tmp/clippy-bug)
warning: use of a disallowed/placeholder name `foo`
 --> src/main.rs:7:9
  |
7 |     let foo = "clippy will complain about placeholder name foo.";
  |         ^^^
  |
  = 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: `clippy-bug` (bin "clippy-bug" test) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.11s

Version

rustc 1.69.0 (84c898d65 2023-04-16)
binary: rustc
commit-hash: 84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc
commit-date: 2023-04-16
host: x86_64-unknown-linux-gnu
release: 1.69.0
LLVM version: 15.0.7

Additional Labels

No response

[blyxyas]

@rustbot claim

[blyxyas]

Outdated

After some debugging, turns out this bug is Cargo's fault. You don't need to open an issue to Cargo, I'll do it myself with some more information.

I say this just so you know this will take a while, as Cargo's debugging + PR process is lengthier.

Turns out this isn't Cargo's fault, they document that behaviour. It's our fault.

[tsdh]

@blyxyas Hehe, nice positive wording "so it can fix as much as it can". But I'd rather be a bit more honest and mention the technical cause because it's still at least a bit surprising that cargo clippy --fix will try to fix issues which cargo clippy itself did not mention.

[blyxyas]

Are you saying that I'm dishonest?
I didn't say the technical cause because it doesn't matter.
But the technical cause is that Clippy uses Cargo under the hood, and cargo clippy --fix uses specifically cargo fix. cargo fix defaults to --all-targets when no target is provided. Thats why cargo clippy and cargo clippy --fix have different targets.

[tsdh]

@blyxyas No, I didn't mean to discredit you, of course. But as I see it the behavior is basically an implementation detail (just as you've described in your last comment) and not a feature. In fact, it's actually a bit astonishing and inconsistent that cargo clippy --fix tries to fix issues not mentioned by cargo clippy.

It's ok to accept that and document it. Or there are two other possibilities.

• Explicitly pass the clippy default target to cargo fix when piggy-packing on it
• Make cargo clippy also check all targets

But that's not my judgement call to make. :-)