cargo: Output which errors are automatically correctable, to improve discoverability of `cargo fix` and `cargo clippy --fix`

This seems interesting, I’d love to work on it if we can figure out what the message should be. I lean towards an explicit callout to cargo --fix but I could see just having how many are fixable.

It was discussed in a recent cargo team meeting to move forward with showing a message that cargo fix can fix some warnings but only on nightly.

Current format:

warning: `foo` (lib) generated 1 warning (run `cargo fix --lib -p foo` to apply 1 suggestion)

Current implementation from #10989:

• Shown on nightly only to allow iterative (potentially breaking) updates driven by get user feedback
• Shown only on warnings as there are problems with cargo fix and errors
• Shown in the summary line to not add extra output lines
• The command is shown on a per target basis
• The exact command to run is shown (minus version control specific flags)

Open questions:

• Should the message stay in the summary line or be on its own line?
  • The summary line can be very long with the current implementation
• Should the command be specific or genera?
• Should the command to run be per target or shown once at the end?
• Should version control specific flags be added to the command to run?

How I have it currently working doesn’t work for clippy as its a subcommand

I was thinking to keep things more targeted

; cargo check
warning: unused import: `::r#unsafe`
 --> src/lib.rs:1:5
  |
1 | use ::r#unsafe;
  |     ^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: `test-cargo` (lib) generated 1 warning
note: to fix 1 warning automatically, run `cargo fix`

• atm cargo doesn’t have help though it could be added, so I’ve been using note: for similar.
• by building the number into the note it makes the message more direct / succinct which makes it easier to scan.

On a related note, I’ve been meaning to bring up if cargo should align with rustc in its error style guide

Running cargo clippy will also emit this message, but as it’s suggesting a plain cargo fix it won’t apply the suggestions

It would be great it if could suggest running cargo clippy --fix, but if a way to do that hasn’t been decided yet it’s probably best to omit the message when either RUSTC_WRAPPER or RUSTC_WORKSPACE_WRAPPER are set

I never run cargo fix. Something like this will just be constant noise for me. I think it is good to be very cautious about being too verbose or too prodding.

@ehuss I want to contrast this with me who nevers use cargo fix because it never crosses my mind. Even if I did, I probably wouldn’t want to do the mental calculus to see if its worth running. Adding this would be a big help to me and, I suspect, to a lot of new users.

As for balancing noise vs help, we intentionally made it so we only emit 1 brief line for the entire run because of this. I personally suspect this one line won’t be an issue

cargo fix often won’t work correctly if there are any errors in the code.

Maybe we should limit the message to only when there are fixable errors warnings?

cargo fix won’t work with changes in the user’s repo

To me, this just means we evaluate the behavior between cargo fix and cargo fix --allow-dirty to see which holistic solution is better for being the default recommended command.

Also, there is a moderate risk that cargo fix will damage or otherwise mess up the user’s code with no way to revert (the reason --allow-dirty is there). I

I think a --allow-dirty flag, whether we suggest it or not, will give users enough of a hint about this. Also, by having increased visibility of the feature, we’ll likely get more bug reports so we’ll improve the behavior.

Even filtering on machine-fixable errors, it is moderately common that the fix fails for one reason or another. I’m not particularly confident in pushing large numbers of people to use it more often. (Minor concern.)

ditto about this just being an opportunity to improve it.

This doesn’t really make it clear to the user which issues will be fixed.

As a user, I don’t care. I’ll run the command and get things fixed and then address the rest. I’ll be elated to not have to manually update 200+ warnings that I sometimes get.

I appreciate it can be difficult to educate people on the existence of tools or options for performing certain tasks. I’m wondering, though, does rust-analyzer not show a quick-fix for fixable suggestions? Perhaps that would be a better route to make it easier for people to apply suggestions, or otherwise have easy access to it?

I at least do not use rust-analyzer and punting to rust-analyzer is a slipper slope (we don’t need cargo add because rust-analyzer should do it, etc).

Seems reasonable. I can’t remember which order the compiler does it but I’m assuming errors would go before warnings.

maybe something like this?

; cargo check
warning: unused import: `::r#unsafe`
 --> src/lib.rs:1:5
  |
1 | use ::r#unsafe;
  |     ^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: `test-cargo` (lib) generated 1 warning (1 automatically fixable)
help: use `cargo fix` to automatically fix warnings

and only display the help message if there’s at least one automatically fixable warning.

Oh, great! I had missed the intent of your previous message

To improve discoverability, we could do something similar to what rubocop does: each error could include info about whether it’s automatically correctable or not.

Another approach is the one I took in cargo upgrade. I report the status of every dependency but if a dependency is held back and needs a flag to be upgraded, I track that and report it at the end.

In this case, it’d be a note like

note: Run `cargo --fix` to automatically address some of the above.

EDIT: The challenge with this is it violates layering / separation of concerns. cargo doesn’t know about these messages to put the suggestion in and the commands don’t know about cargo to do so.

@jyn514 yes, rather than tagging every command that can output warnings, I just tagged the ones that the suggestions would be for.