Skip to content

/cargo

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

cargo fix: Prevent fixing in mixed up transition steps #5778

Closed
alexcrichton opened this Issue Jul 23, 2018 · 4 comments

Comments

Assignees

@alexcrichton alexcrichton

Labels
Command-fix
Projects
None yet
Milestone
  Edition Preview 2
2 participants
@alexcrichton @johnthagen
@alexcrichton
Copy link
Member

alexcrichton commented Jul 23, 2018

A common problem we're seeing in the 2018 transition is that steps are performed out of order, rendering cargo fix not too useful any more! I think we should implement two important fixes in cargo fix to remedy the situation:

  • First, Cargo should be able to deduce a set of "interesting crates" which are actually being fixed. This is driven by the -p arguments on the command line.
  • Next, if these crates have the 2018 edition enabled and --prepare-for is passed, then an error is generated. You can't prepare for an edition if you're already in the 2018 edition!
  • And finally, if the crate is compiled but doesn't have #![feature(rust_2018_preview)] inside it then we should either warn or fail compilation.

This should hopefully fix the bugs of "you enabled the edition too early" or "you forgot to enable the feature gate"!

@alexcrichton alexcrichton added the Command-fix label Jul 23, 2018

@alexcrichton alexcrichton added this to the Edition Preview 2 milestone Jul 23, 2018

@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member Author

alexcrichton commented Jul 23, 2018

cc @killercup

@alexcrichton alexcrichton referenced this issue Jul 26, 2018

Closed

Should the rust_2018_idioms lint group be mentioned in documentation? #52679

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 28, 2018

@alexcrichton
Add more diagnostics to smooth edition transition 
This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes rust-lang#5778
eccc731

@alexcrichton alexcrichton referenced this issue Jul 28, 2018

Merged

Add more diagnostics to smooth edition transition #5824

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 28, 2018

@alexcrichton
Add more diagnostics to smooth edition transition 
This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes rust-lang#5778
30488b1
@johnthagen

This comment has been minimized.

Sign in to view
Copy link
Contributor

johnthagen commented Jul 28, 2018

Next, if these crates have the 2018 edition enabled and --prepare-for is passed, then an error is generated. You can't prepare for an edition if you're already in the 2018 edition!

Wanted to share anecdotal support for this feature. When I was doing a transition of a crate, it was very natural to up-arrow in the terminal and pull down my first cargo fix --prepare-for 2018 ... command even after I added the edition to my Cargo.toml. I'd expect others to make this small mistake as well.

@johnthagen johnthagen referenced this issue Jul 28, 2018

Merged

Explicitly mention not passing prepare flag after switching to new edition #69

@alexcrichton

This comment has been minimized.

Sign in to view
Copy link
Member Author

alexcrichton commented Jul 28, 2018

Yes currently I'm not sure if we've got the best ergonomics. Another option would be to issue a warning and keep going (without the lint group), but that runs more risk of causing hundreds of errors I think than the annoyance of tweaking arguments.
@johnthagen

This comment has been minimized.

Sign in to view
Copy link
Contributor

johnthagen commented Jul 28, 2018

Yeah, I feel like shooting for a "fail-fast" error in this case with a good message would be the best IMO.

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 30, 2018

@alexcrichton
Add more diagnostics to smooth edition transition 
This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes rust-lang#5778
e4f447b

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 30, 2018

@alexcrichton
Add more diagnostics to smooth edition transition 
This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes rust-lang#5778
438e240

bors added a commit that referenced this issue Jul 30, 2018

@bors
Auto merge of #5824 - alexcrichton:careful-transition, r=alexcrichton 
Add more diagnostics to smooth edition transition

This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes #5778
Loading status checks…
47829c0

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 30, 2018

@alexcrichton
Add more diagnostics to smooth edition transition 
This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes rust-lang#5778
d7f091e

bors added a commit that referenced this issue Jul 30, 2018

@bors
Auto merge of #5824 - alexcrichton:careful-transition, r=alexcrichton 
Add more diagnostics to smooth edition transition

This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes #5778
Loading status checks…
53c351e

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 31, 2018

@alexcrichton
Add more diagnostics to smooth edition transition 
This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes rust-lang#5778
fa7a387

@alexcrichton alexcrichton self-assigned this Jul 31, 2018

bors added a commit that referenced this issue Jul 31, 2018

@bors
Auto merge of #5824 - alexcrichton:careful-transition, r=alexcrichton 
Add more diagnostics to smooth edition transition

This commit adds two diagnostics in particular to ease the transition into the
2018 edition. The current transition process is pretty particular and must be
done carefully, so let's try to automate things to make it as painless as
possible! Notably the new diagnostics are:

* If you `cargo fix --prepare-for 2018` a crate which already has the 2018
  edition enabled, then an error is generated. This is because the compiler
  can't prepare for the 2018 edition if you're already in the 2018 edition, the
  lints won't have a chance to fire. You can only execute `--prepare-for 2018`
  over crates in the 2015 edition.

* If you `cargo fix --prepare-for 2018` and have forgotten the
  `rust_2018_preview` feature, a warning is issued. The lints don't fire unless
  the feature is enabled, so this is intended to warn in this situation to
  ensure that lints fire as much as they can.

After this commit if `cargo fix --prepare-for` exits successfully with zero
warnings then crates should be guaranteed to be compatible!

Closes #5778
Loading status checks…
9bbab73

@bors bors closed this in #5824 Jul 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.