add a note that some warnings (and/or errors) can be auto-fixed

[Muscraft]

This adds a note that some warnings (and/or errors) can be auto-fixed by running cargo --fix. This only supports cargo check as we can't show the same note for cargo clippy since it is an external subcommand.

The message for the note was taken from #10976 (comment).

Close #10976

[rust-highfive]

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

[ehuss]

I'm reluctant to add something like this for a few reasons:

• 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.
• cargo fix often won't work correctly if there are any errors in the code.
• cargo fix won't work with changes in the user's repo. They have to run with --allow-dirty and/or --allow-staged, which can be awkward. 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). If there is no git repo, then it is doubly more dangerous. Or if the files haven't been added yet, cargo fix may overwrite them without confirmation.
• 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.)
• This doesn't really make it clear to the user which issues will be fixed.

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?

[Muscraft]

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 haven't used rust-analyzer (in a while) but I know Clion (with the rust extension) gives quick fixes so I would assume RA does as well

I'm reluctant to add something like this for a few reasons:

Given the number of reasons, and the number of problems with cargo fix (which appear to be major blockers), it might be a good idea to close this and continue the discussion in #10976. I could see waiting on this until cargo fix has some issues resolved, or there is a new error/warning variant within rustc that would always be "fixable".

[epage]

Let's leave this open but move this to the Issue

[Muscraft]

I've updated this with feedback from #10976. Changes:

• The note is added to the summary line as suggested here
• The command to run now includes the exact command to run i.e.

  warning: `foo` (example "ex1") generated 1 warning (run `cargo fix --example "ex1"` to apply suggestions)
  warning: `foo` (bench "bench") generated 1 warning (run `cargo fix --bench "bench" --tests` to apply suggestions)
  warning: `foo` (bin "foo" test) generated 2 warnings (run `cargo fix --bin "foo" --tests` to apply suggestions)
  

Concerns:

• Will adding this suggestion cause there to be too many issues filed and lots of work to fix things?
• Is the output message too verbose/specific
• Should the message only be shown once?

[weihanglo]

• Is the output message too verbose/specific

Looks fine to me, since Cargo already emits such note.

I am uncertain whether we need to make sure the suggested command is always runnable. A bad suggestion could be worse than no suggestion.

• Should the message only be shown once?

I don't get it. Could you expand a bit?

[Muscraft]

I am uncertain whether we need to make sure the suggested command is always runnable. A bad suggestion could be worse than no suggestion.

I agree that a bad suggestion is much worse than no suggestion and it's something that I tried to make sure wouldn't happen. That being said, I think having a specific command to run is much better. Pros:

• It ensures that cargo fix is only being run on exactly what is giving the warning
• It makes sure that cargo fix is not being run on anything with an error
• Reduces unintended code churn
• Can clearly see what target/package the command is for
• Fixes applied target by target

Cons:

• The lines are very long
• Lots of new output
• Chance to give the wrong command
• Each command would need to be run separately (lots of cargo fix runs)
• Doesn't suggest --allow-dirty so the command would probably always be "wrong"

• Should the message only be shown once?

I don't get it. Could you expand a bit?

There is a chance that the long lines on potentially every summary line could be overwhelming. Having it output once per package(compared to target) would reduce this and make the command less specific. Going further and having it only be suggested once could make it so there is one place command is suggested and not affect every summary line. Having it be on its own line comes with its own problems but might make it easier to suggest a hyper-specific command (check version control status).

[weihanglo]

@bors r+

[bors]

:pushpin: Commit 8ff8eaa49699c36a38ca67ebd34e5e273feb83c5 has been approved by weihanglo

It is now in the queue for this repository.

[bors]

:hourglass: Testing commit 8ff8eaa49699c36a38ca67ebd34e5e273feb83c5 with merge 2d04bcd1b0d6f40001a7b1762f78842851d0b334...

[bors]

:sunny: Test successful - checks-actions Approved by: weihanglo Pushing 2d04bcd1b0d6f40001a7b1762f78842851d0b334 to master...