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

[jgpaiva]

Problem

cargo fix and cargo clippy --fix can be great tools for sparing developer cycles, especially when a new version of cargo is released. However, the discoverability of these tools is a problem: not everyone knows they exist.

Proposed Solution

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. This may prompt users who don't know about this feature to go find it. It also helps users who do know about the feature: they can avoid running the fix command in situations where it'll be useless because the tool actually can't automatically fix those particular errors.

For reference, here's a screenshot of rubocop's output:

Notes

No response

[jyn514]

This information is already available in the JSON output:

// foo.rs
#[derive(Debug(x))]
struct Foo;

; rustc foo.rs --crate-type lib
error: traits in `#[derive(...)]` don't accept arguments
 --> foo.rs:1:15
  |
1 | #[derive(Debug(x))]
  |               ^^^ help: remove the arguments
; rustc foo.rs --crate-type lib --error-format json 2>&1 | jq .children[].spans[].suggestion_applicability
"MachineApplicable"

[jyn514]

@epage this isn't unique to clippy, it's useful for normal check lints and errors too.

[epage]

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

[epage]

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]

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.

I don't understand the problem? Cargo already knows about these suggestions because they're present in the JSON output from rustc. Cargo already parses those messages to know how many errors were present.

[jyn514]

Here's an example of a similar change in the past: #9655

[epage]

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

[Muscraft]

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.

[jyn514]

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.

[epage]

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

[jyn514]

@epage what do you think should happen for errors that can be fixed automatically? maybe something like this?

note: to fix 1 warning and 1 error automatically, run `cargo fix`