Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

WIP: Edition mode #95

Closed
wants to merge 6 commits into from
Closed

Conversation

killercup
Copy link
Member

@killercup killercup commented May 6, 2018

This adds a first test for edition lints (cf. #62). I'll open another PR for filtering by machine applicability in a minute.

Note: Requires custom rustc built with rust-lang/rust#50454

cc @Manishearth

@killercup
Copy link
Member Author

rewrote the branches a bit, so this is now based on #96

killercup added 4 commits May 6, 2018 21:57
To allow us to add more fixture tests modes soon.
Also refactors the test setup to finally run as many tests as possible
and give useful errors.
Basically the same as the one for rustfix proper, but with the required
cargo setup in place, too. We should replace this one with a 'phase 1'
lint (that makes required changes to migrate to the 2018 edition) once
one is available.
We should late on pass `-W rust_2018_migration` automatically when there
are lints present in this lint group.
@alexcrichton
Copy link
Member

Nice! I think we're still thinking of adding an --edition flag, right? One to auto-turn-on the "phase 1" lints I think?

@killercup
Copy link
Member Author

killercup commented May 7, 2018

@alexcrichton yes! This PR is just about having tests for each of the three modes (migration-to-2018, on-2018, yolo).

We should add an --edition flag that adds -W rust_2018_migration and disables all other lints (but maybe not in this PR).

@killercup
Copy link
Member Author

While it's nice to have this in rustfix' test suite, once we have a rustfix with machine-applicable-only flag, we should really upgrade rustc's compiletest to use that and make sure to add rustfix tests for each of the lints in rust-lang/rust#50068 in the compiler itself.

@alexcrichton
Copy link
Member

Ok cool makes sense to me. I'm also less certain myself about what those three modes are, could you detail them a bit more?

Also, are you thinking that this should land ahead of other PRs to get it into rust-lang/rust sooner?

@killercup
Copy link
Member Author

killercup commented May 7, 2018

Sure. The "three modes" are basically the same as the "groups" we defined here, plus "fix all suggestions":

  1. Migration to 2018
    • "Group 1: lints within 2015"
    • lints that change your 2015 code so it also works in 2018.
    • I'm not sure if the lint group for this exists in rustc yet.
  2. Edition mode
    • "Group 2: lints within 2018"
    • lints that change your code so it becomes idiomatic 2018 code.
    • This is what you get when you add #[warn(rust_2018_migration)]
      • which means my naming is bad (this lint group was previously known as rust_2018_idioms so it didn't conflict with me calling the above group "migration"… oh well
  3. "Everything" (the good old "yolo" mode) is just an internal group for us
    • all the other lints that are edition unrelated

This is only an internal grouping for rustfix' tests! I want to ensure that we have integration tests covering what is pretty essential to rustfix -- doing edition related fixes :)

@alexcrichton
Copy link
Member

@killercup ah ok, interesting! I figured we were just gonna always leave rustfix in "fix all warnings you can" mode and otherwise the flags would simply indicate what lints the tool explicitly enables.

For example I figured that when you switch to the 2018 edition via edition = '2018' we'd probably have a lot of lints on by default, so you'd run cargo fix and be done with it. Before you do that though you may want to make sure you can do it in the first place, so you'd run cargo fix --edition 2018 which would enable the rust_2018_migration lint explicitly (which currently is thought to be turned off for the near future.

Are you thinking we'd explicitly ignore some lints though and not apply fixes that we automatically look at?

@killercup
Copy link
Member Author

Sorry for the confusion! This PR might not actually make much sense -- I just wanted to have a quick way of testing the edition lints. But we should do that in rustc directly.

Let's close this PR. I'll open a new one that adds the cargo fix --prepare-for 2018 mode once we have a lint in nightly that we can use there.

We should add one end-to-end test soon, though: One that goes from "compiles on 2015" through cargo fix --prepare-for 2018 and then adding edition = "2018" to "nice looking and working 2018 code".

Are you thinking we'd explicitly ignore some lints though and not apply fixes that we automatically look at?

Nah, we should trust what the compiler tells us. Forget everything you saw here.

@killercup killercup closed this May 7, 2018
@alexcrichton
Copy link
Member

Ok that all sounds great to me!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants