Rustfix not fixing easy error

[3tilley]

Hello,

Apologies if this isn't the right place to put this, it seemed to be rustfix functionality rather than cargo. The snippet below is a fairly oft-occurring error, and one with an obvious solution. Is there a reason rustfix doesn't add the derive attribute?

Sample code:

enum MyEnum {
    A,
    B
}

#[derive(Debug)]
struct MyStruct{
    a: u8,
    e: MyEnum
}

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

Output:

$ cargo fix --broken-code
    Checking fix-test v0.1.0 (...\fix-test)
error[E0277]: `MyEnum` doesn't implement `Debug`
  --> src\main.rs:10:5
   |
7  | #[derive(Debug)]
   |          ----- in this derive macro expansion
...
10 |     e: MyEnum
   |     ^^^^^^^^^ `MyEnum` cannot be formatted using `{:?}`
   |
   = help: the trait `Debug` is not implemented for `MyEnum`
   = note: add `#[derive(Debug)]` to `MyEnum` or manually `impl Debug for MyEnum`
   = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `MyEnum` with `#[derive(Debug)]`
   |
2  | #[derive(Debug)]
   |

For more information about this error, try `rustc --explain E0277`.
error: could not compile `fix-test` due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `fix-test` due to previous error

[Manishearth]

These fixes come from rustc, not rustfix. Rustfix simply applies them.

The reason here is that that suggestion is probably marked MaybeIncorrect in rustc, since it's not necessary the right solution there. It's the most likely but not always correct.

We do want to eventually support opting into applying all suggestions but nobody's had the time to try designing the CLI for that.

[3tilley]

Thanks for the swift reply, that makes sense. MaybeIncorrect seems like a sensible category for this error, and not implemented seems sensible default behaviour.

As well as --apply-maybe-incorrect or similar, I think this would be a lot easier to work with if there was interactive application of fixes. I've seen a couple of references to that on the various repos.

1. Which repo would the CLI changes to into, is there a cargo-fix repo? I assume some changes would be required here too
2. Ditto for --interactive
3. More tangential, but is there something I can read about the difference between rustfix and clippy? Both in terms of functionality and the philosophy of the tools?

[Manishearth]

I think it would live in this repo.

Clippy vs rustfix are apples and oranges.

Basically, rustfix is a lower level tool that parses suggestion diagnostics and applies them to code. Technically it could be used for e.g. go as well if you could get a decent json diagnostics output.

cargo is the general build orchestration tool, it knows when to call which binary. It glues together a build pipeline. When you run cargo fix it's running rustc then rustfix.

rustc and clippy are actual compiler binaries, clippy has a heavily augmented set of lint passes. cargo clippy does approximately the same thing as cargo check but with a different compiler binary.

[3tilley]

Ok that's a great explanation, thanks! So rustfix takes input from somewhere, and has a mechanism to edit the source files to implement those changes. It's concerned with file edits, handling VCS etc. The input for those suggestions could come from lots of different tools.

Does clippy --fix use rustfix? If not, could/should it? I can't see it in the dependencies, but then I'm not sure I'm looking in the right place.

[Manishearth]

Also there's a preexisting issue for this with some discussion, fwiw: #200

[Manishearth]

And yes, clippy --fix uses rustfix.

Once rustfix gets an interactive mode / apply-all mode we'd have to pipe these flags up through cargo.

I think to start with it would be cool to be able to ask rustfix to apply-all a "minimum applicability level" (since beyond MaybeIncorrect there's also HasPlaceholders or something), and then we can try to build something interactive.

Iirc early prototypes of rustfix actually had the interactive thing (maybe that code still exists in tree? Maybe it's just not exposed to cargo)

[Manishearth]

So it seems like rustfix already has support for this, with "fix mode". You can override it with __CARGO_FIX_YOLO=1 in cargo, which does the following:

https://github.com/rust-lang/cargo/blob/65535890dbaa2b186633aedc11e550c3990fc5e9/src/cargo/ops/fix.rs#L587-L589

So really I'd say the first step is exposing it as a flag in cargo. Might be worth filing an issue there to talk about the UX. Ideally we can also have it say something like "rustfix declined to apply some suggestions because it's not sure if they are correct. Pass --flag to apply them too".