cargo-fix: what kind of things can be fixed?

[eminence]

I'm trying out cargo fix to see what kind of stuff it can fix, and I'm a little surprised that it doesn't fix some trivial stuff that looks easy to handle. Some examples:

warning: unused import: `std::io`
 --> src/lib.rs:4:9
  |
4 |     use std::io;
  |         ^^^^^^^

warning: variable `BAD_NAME` should have a snake case name such as `bad_name`
 --> src/lib.rs:8:9
  |
8 |     let BAD_NAME = v.len();
  |         ^^^^^^^^

warning: variable does not need to be mutable
 --> src/lib.rs:6:9
  |
6 |     let mut v: Vec<u8> = Vec::new();
  |         ----^
  |         |
  |         help: remove this `mut`

warning: unnecessary parentheses around `return` value
 --> src/lib.rs:9:12
  |
9 |     return (BAD_NAME + 1)
  |            ^^^^^^^^^^^^^^ help: remove these parentheses

Do you expect that cargo fix will eventually be able to make these types of fixes automatically?

[alexcrichton]

Thanks for the report!

The cargo fix tool is driven (as you've probably seen) by compiler lints. The compiler is the one that makes suggestions and cargo fix understands them programmatically and then applies them. Currently cargo fix only works with "machine applicable" suggestions and all suggestions in rustc aren't machine applicable by default (this distinction is pretty recent).

In that sense what needs to happen here is that the compiler needs to be updated and audited to have machine applicable suggestions in the situations you linked above, and once that's done it should be good to go to fixup the lints automatically!

[zackmdavis]

(↑ got scooped while I was drafting this comment, posting my version anyway because the links may be useful)

Under the hood, cargo fix is powered by suggestions emitted by rustc.

Making cargo fix work on your unnecessary-parens and unnecessary-mut examples will be very easy: the compiler is already emitting the appropriately structured suggestion (that's where the "help:" labels in the terminal output come from); it's just that cargo fix is conservatively only applying suggestions that have been explicitly marked as machine-applicable and no one has yet gotten around to marking most of the existing suggestions. Patches welcome!—the work here looks like rust-lang/rust#50724, and I fear the reason it hasn't already been done for the rest of the compiler is just that it's slightly tedious (as opposed to being intellectually challenging and glamorous).

The snake-case-name example is much more awkward, because you'd have to replace all appearances of the variable with the new name.

We don't yet have structured suggestions for unused imports, but that's another case of "no one's gotten around to coding it yet" rather than any fundamental technical barrier.

[eminence]

great answers, thank you both!

[rphmeier]

Maybe this is something that cargo-fix already accounts for, but I can imagine code suggestions for unused imports being overly verbose when using rustc as a human. Sometimes when doing refactoring you can end up with a lot of unused imports.

When rustc gets code suggestions for unused imports, are those opt-in for automated tools like cargo-fix?

Edit: Found rust-lang/rust#50723 which seems to discuss an Applicability enum for exactly this purpose. I'll leave this comment up for posterity :)

[zackmdavis]

@rphmeier Thanks for commenting! The suggestions for unused-imports were implemented by @pietroalbini in rust-lang/rust#56645, but actual rustfixability is blocked on rust-lang/rust#53934, which is blocked on me finding a free weekend day to finish researching the potential impacts on RLS &c.. We'll get there soon, I promise!!

[rphmeier]

Thanks Zack, that's helpful!

[sanmai-NL]

@zackmdavis Is there an issue to track rustfixability of unused imports?

[zackmdavis]

rust-lang/rust#47888? (Sorry, it looks like this still (!) points to being blocked on rust-lang/rust#53934, which I had done research on and genuinely intended to write an adequate solution, before life events pulled me away from compiler development for many, many months and I lost all my context; if anyone wants to pick it up, I can respond to questions.)