UI for the 2018 edition release

[alexcrichton]

The rustfix tool is going to be a banner feature of the 2018 edition coming up later this year, so I think we'll want to UI to be as good as we can get it to ensure that everyone's got a smooth experience with applying the tool and updating code.

How does rustfix work today?

I've been reading the code as-is to better understand how it works today, and I'll try to summarize it here for the unfamiliar but please correct me if I'm wrong! Today you can either execute rustfix or cargo fix and they'll do the same thing. The CLI takes no arguments and has a few flags that change its operation. Just executing rustfix means that it'll instead run cargo rustc -- --message-format json.

Once Cargo/rustc is executed rustfix slurps up all the output of rustc. The stderr stream is then parsed into a list of suggestions and replacements. All suggestions are then iterated over and by default presented to the user to ask whether the fix should be applied or not. If accepted then it's queued up to be applied later. This then happens for all suggestions found, and there's a flag as well to say "don't query me, just apply everything".

Finally after all this the suggestions will be applied one-by-one, updating the files on the filesystem.

What do we want the UI for the edition to feel like?

I think we're largely undecided on this in the sense that we haven't concretely nailed this down (although I may have missed it!). I'll put forth a strawman though for how I could see this working.

First off before we release the edition we'll want to test out lints, language features, and rustfix. To do that you'll first add #![warn(rust_2018_migration)] to the crate you'd like to update. This will include src/lib.rs, src/main.rs, tests/*.rs, examples/*.rs, etc. They'll all need this annotation. After that's applied you'll execute cargo fix. This'll then apply as many fixes as it can (no user interaction), but also continue to emit normal compiler warnings about things that couldn't be fixed.

The second scenario is when we've actually released the edition itself. The only difference here is that instead of #![warn(..)] all over the place you'll instead just say rust = '2018' in your Cargo.toml.

How to implement this?

Rustfix is shaping up to be a pretty simple tool (yay!). It's largely just taking suggestions from the compiler and actually applying them on the filesystem, assuming the compiler is correctly informing rustfix of changes that need to be made.

The actual integration though is going to be pretty tricky. For a the best-quality experience we'll want to make sure that rustfix gracefully handles things like workspaces, multiple crates/targets in a workspace, cross-compilation, etc. I think that we can get all this done with a "hack" which has worked out quite well for rustbuild historically, override rustc.

A new CLI

I'm envisioning a CLI that looks like this for rustfix:

# Works like `cargo check --all-targets`, fixes all warnings that `cargo check --all-targets`
# would otherwise produce. This involves updating libraries, binaries, examples,
# tests, etc.
$ cargo fix

# Fixes all warning that `cargo build` would otherwise produce
$ cargo fix build

# Fixes all warning that `cargo test` would otherwise produce
$ cargo test

# Fixes all warnings specifically for Windows
$ cargo fix --target x86_64-pc-windows-msvc

# Fixes all warnings for just one example and the library, if any:
$ cargo fix build --example foo

The general idea is that cargo fix is the main entry point, and it otherwise mirrors Cargo's normal invocations. It will operate "as if" some other cargo command is executing, only a bunch of warnings are fixed along the way.

If a subcommand to fix isn't specified it's assumed to be check. If nothing is passed it's the special case check --all-targets to fix as much as possible. Otherwise flags are all forwarded to cargo subcommands as usual to execute various compilations.

This should have the benefit of working on workspaces, working on all targets, and hopefully even being future compatible with future Cargo features!

Overriding rustc

To actually implement the above interface I think we'll want to execute cargo (the CLI) with the RUSTC environment variable set to rustfix's binary itself. That way it'll get reexecuted and have control over the compilation.

In this way we've now provided a hook to all rustc compilations, allowing us to intercept error messages and such to see what's going on. I think the logic of the script will look like:

• Find an argument that looks like a filename. If it doesn't exist exec rustc.
• If the filename looks like it's from a crates.io crate or otherwise not relevant to the workspace we're working in, exec rustc.
• Now that we know this is a local crate, we fix all lints
  • The compiler is executed in --emit metadata mode (injected if not already present). All output is slurped up with --error-format=json as well.
  • All fixes are parsed and applied
    • This can possibly involve IPC back to the main parent process if we want interactivity
  • TODO: what if the compilation here fails?
• Next we actually compile the crate
  • The compiler is invoked with the original arguments passed to the script (no output capture)
  • This is now executing with fixed code, and the user never saw warnings that ended up being fixed
  • Warnings that weren't automatically fixed will still be emitted
  • TODO: what if compilation fails here?

And I think that's enough to basically get the feeling of "automatically fix all the code" while also assuming as little as possible about Cargo itself.

Some possible caveats are:

• There won't be an easy way to roll back fixes once we've gone past a crate. For example if a crate successfully compiles but fails other downstream compilations, it'll be hard to go back and revert the original crate.
• If the fixed code fails to compile, we may have a bad UI presented here. We can go back and revert all the fixes we applied and tell the user what happened, but they run the risk of being confused.
• We want to force Cargo to not actually cache anything and recompile local crates, so somehow we'll want to make repeated invocations of cargo fix actually run over code that may need fixing.

---

If we were to do this it's a pretty major rewrite/rethinking of the current CLI, so I'd like to get others' opinions on this! I'm curious if we can improve various aspects or otherwise make us better suited for the edition release!

[alexcrichton]

I'm also not entirely sure where a rustfix standalone CLI tool would fit into this, although it may arguably be useful for things like rustfix foo.rs which behaves like rustc foo.rs

[Manishearth]

One thing I want to note is that I'm currently maintaining a breakage vs idiom distinction for epoch lints. Breakage lints are the bare minimum you must fix to upgrade. A good feature of this is that they're backwards compatible changes; if you fix these you can still work on the old edition (great for testing). Idiom lints contain changes that may be more expansive but not required (but highly recommended).

It would be nice if we had a cargo fix --minimal vs cargo fix mode for this.

[killercup]

Sounds like a very solid plan to me, @alexcrichton! I've previously tried to rely on cargo to give us all the JSON output we desire (i.e., just call cargo … --message-format=json … internally) but I can see the advantages of wrapping rustc -- mainly being able to re-execute rustc after fixing.

I like the idea to pass cargo-like flags to cargo fix, but I'd try to omit the build in cargo fix build. Super tiny nit in the big picture, of course.

I'm a bit unsure if we need the "find out if this is a file we care about" step, though. If our wrapper gets called by cargo, we should get --cap-lint flags for all truly external crates, right?

Also, do we actually need the user to add #![warn(rust_2018_migration)] to their code? I'd adding -W rust-2018-migration to the rustc call would pretty much to the same thing. In fact, do you think it'd make sense to always override the lints when running in rustfix mode (not in the second compile after fixing) so we can reduce the warnings we get to only the lints we know we can fix? This can in the future of course be parametrized.

[alexcrichton]

@killercup oh hm interesting! Using --message-format=json is also possible in that we could pretty easily basically execute cargo once, slurp up everything, and then afterwards run cargo check again which will only execute over modified sources.

I'm not actually sure if there's a benefit to shadowing rustc and injecting arguments "just in time" over the --message-format=json approach, so that seems plausible to me!

I think that's something we'll want to talk through today about whether users need to add the attribute to their code, I don't personally have too many thoughts but would be fine either way!

[killercup]

cf. #66

[alexcrichton]

Ok with #66 landed now I'm going to close this and we can continue to iterate in-tree and on future issues

[Ixrec]

One thing I want to note is that I'm currently maintaining a breakage vs idiom distinction for epoch lints. Breakage lints are the bare minimum you must fix to upgrade. A good feature of this is that they're backwards compatible changes; if you fix these you can still work on the old edition (great for testing). Idiom lints contain changes that may be more expansive but not required (but highly recommended).

It would be nice if we had a cargo fix --minimal vs cargo fix mode for this.

My naive expectation was that if my crate is currently on Rust 2015, cargo fix only does the fixes that don't break Rust 2015, and only if my crate is on Rust 2018 would cargo fix also do all the fixes that require 2018 features. So I don't think we need a new --minimal flag for this; the edition already is that flag.