Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide rustfix to upgrade code to Rust 2018 automatically #49247

Closed
2 of 8 tasks
killercup opened this issue Mar 21, 2018 · 13 comments
Closed
2 of 8 tasks

Provide rustfix to upgrade code to Rust 2018 automatically #49247

killercup opened this issue Mar 21, 2018 · 13 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@killercup
Copy link
Member

killercup commented Mar 21, 2018

Rustfix is a CLI tool that parses the compiler's suggestions and applies them to the source files. It is the official tool to migrate from Rust 2015 to 2018.

This issue is to track the development of the CLI tool. Other ways to interact with the compiler suggestions include "reading them and changing code manually" as well as "using RLS and clicking on lightbulb icons in VSCode".

What needs to be done

@killercup
Copy link
Member Author

State of rustfix as of 2018-03-21

The rustfix available in killercup/rustfix is pretty bare-bones: It currently is a simple interactive CLI tool that executes cargo (or clippy) and presents the user a choice of "which suggestion to apply"/"skip" for each compiler diagnostic message that include one or more suggestions.

Its test suite works well and exercises some suggestions from rustc as well as clippy. The library itself has some known shortcomings that some of clippy's suggestions show. E.g. problems with applying multiple suggestions changes per suggestions multiple times per file confuses its simplistic "replace line/column ranges" approach.

I propose to only focus on the lints related to the 2018 edition and their suggestions for now (which seem to live in the epoch_2018 for the most part -- cf. #48796). The rustfix library can probably reuse parts of the compiler internals to use the canonical representation of the messages instead of custom structs to deserialize them, and the CLI tool should be rewritten to be non-interactive for now.

@killercup
Copy link
Member Author

My initial suggestion for the user experience of rustfix

As a programmer I want to have a painless way of upgrading my code to the new Rust 2018 edition.

  • When Rust 2018 is released, it includes rustfix as a tool that is distributed by rustup by default, similarly to rustdoc.
  • Running rustfix --edition 2018 foo.rs will compile foo.rs (assuming the default edition, i.e., 2015) and transform the source file so it compiles without edition-warnings (cf. make the epoch Enable All The Things #48790 and Provide a way to distinguish between "breakage" and "idiom" epoch lints? #48796 for discussion on which lints specifically) using the 2018 edition
  • Similarly, running cargo fix --edition 2018 (--edition 2018 is also the default) will fix the current cargo package's source files so they compile in Rust 2018 mode
    • Bikeshed the command name: cargo upgrade-edition? (cargo upgrade conflicts with cargo-edit's subcommand to upgrade dependencies)

(This is the main focus right now. The rustfix tool should in the future be able to apply all machine-applicable compiler suggestions, including those provided by clippy.)

@killercup
Copy link
Member Author

Next steps

I'll meet with @Manishearth and @oli-obk at the All Hands next week to discuss this, and try to get implementation going. One explicit goal will be to create mentor-able issues.

@nikomatsakis
Copy link
Contributor

BTW, should we nab the cargo-fix crate name? It seems clear we'll want to run this like cargo fix, right?

@killercup
Copy link
Member Author

killercup commented Mar 21, 2018

@nikomatsakis good idea! Let me write a crate that does this but… as a crate :)

Update: This should do for now

@kennytm kennytm added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Mar 21, 2018
@mgattozzi
Copy link
Contributor

@killercup I'd be happy to help with this once you get some issues up after next weeks allhands.

@Manishearth
Copy link
Member

@mgattozzi Great! This will be a big help, rustfix is one of the items that needs a bunch of work but everyone is too busy to work on it.

For now, looking into https://github.com/killercup/rustfix/issues/57 may be helpful.

@Manishearth
Copy link
Member

Manishearth commented Mar 22, 2018

Overall I think there's huge potential for fixture tests in rustc, both as a way to test suggestions and to test rustfix.

The way I envision it is that if any compile-fail or UI test foo.rs is accompanied by a foo.rs.fixed file, it's an indication that we must run rustfix on it, and the fixed source should match the .fixed file. Additionally, the fixed file should not emit any of the warning entries that had suggestions.

Once the framework for this exists I can see lots of easy bugs popping out.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 22, 2018

Overall I think there's huge potential for fixture tests in rustc, both as a way to test suggestions and to test rustfix.

Tracked in #45516

@alexcrichton
Copy link
Member

@killercup some thoughts I might have on the initial suggestion. It may actually be pretty cool if we basically just say "run cargo fix" and that's it. The cargo fix mode could basially be a blanket "automatically fix all suggestions for rustc" mode of Cargo (although under the hood it'd actually be rustfix).

This I think would lend itself great to things like:

  • Want to fix warnings for another target? cargo fix --target arm-unknown-linux-gnueabi
  • Want to fix warnings in release mode? cargo fix --release
  • Want to only fix warnings on one test? cargo fix --test foo

I think punting edition flags and such to Cargo and/or the crate may make cargo fix's life (and users' lives) much easier. That way we can ingrain "just run cargo fix and all your warnings will go away" into users minds. That way the edition upgrade experience (at the end of it all) is:

  1. Add rust = '2018' to Cargo.toml
  2. Run cargo fix
  3. Run cargo build, fixing up any remaining warnings

Internally this could all be architected as an elaborate wrapper around cargo check. Basically cargo fix executes the equivalent of RUSTC=myself cargo check and then whenever it's a "local path" for the current crate (TBD) it has all the relevant arguments to pass to rustc so it injects --error-format=json and slurps up the results. In-place it'd then apply all the suggestions, squelch warnings, and if anything else remains basically rerun the compiler with inherited stdin/stdout and let them go naturally to the user.

(the ui here is likely to be fixed over time)

I think something along these lines though is likely to be the quickest and most robust path forward, but I'm curious what you think as well!

@Manishearth
Copy link
Member

Manishearth commented May 3, 2018

comment moved to rust-lang/rustfix#64 (comment)

@killercup
Copy link
Member Author

A lot happend last week thanks to @alexcrichton doing a ton of stuff! I think we have most of the critical features implemented in the CLI tool, and I've opened rust-lang/rustfix#95 which adds the "edition mode" (currently depends on edition lints becoming available).

@mgattozzi, still want to and have time to help out? We have opened some good issues to polish the tooling, and have also left a good number of TODO comments in the code waiting to be picked up :)

@alexcrichton
Copy link
Member

I believe this is largely done so I'm going to close this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants