Auto merge of #5845 - alexcrichton:fix-edition, r=alexcrichton

Rename `--prepare-for` to `--edition`, drop arg

This commit tweaks the UI of `cargo fix` for the edition. Previously you'd
execute `cargo fix --prepare-for 2018`, but that's a lot of typing! Plus it's
some manual data that Cargo can already infer.

Instead, after this commit, you now type `cargo fix --edition`, and that's it!
The idea is that this'll tell Cargo to fix code for the *next* edition,
inferring whatever edition is in use and figuring out what to pass to rustc.

Functionality-wise this should be the exact same as `--prepare-for 2018` though

If others agree w/ this change I'll send a PR to the edition guide after this
merges!

Author: bors

src/bin/cargo/commands/fix.rs

@@ -37,10 +37,19 @@ pub fn cli() -> App {
         )
         .arg(
             Arg::with_name("edition")
+                .long("edition")
+                .help("Fix in preparation for the next edition"),
+        )
+        .arg(
+            // This is a deprecated argument, we'll want to phase it out
+            // eventually.
+            Arg::with_name("prepare-for")
                 .long("prepare-for")
                 .help("Fix warnings in preparation of an edition upgrade")
                 .takes_value(true)
-                .possible_values(&["2018"]),
+                .possible_values(&["2018"])
+                .conflicts_with("edition")
+                .hidden(true),
         )
         .arg(
             Arg::with_name("allow-no-vcs")
@@ -67,22 +76,16 @@ remaining warnings will be displayed when the check process is finished. For
 example if you'd like to prepare for the 2018 edition, you can do so by
 executing:
 
-    cargo fix --prepare-for 2018
-
-Note that this is not guaranteed to fix all your code as it only fixes code that
-`cargo check` would otherwise compile. For example unit tests are left out
-from this command, but they can be checked with:
-
-    cargo fix --prepare-for 2018 --all-targets
+    cargo fix --edition
 
 which behaves the same as `cargo check --all-targets`. Similarly if you'd like
 to fix code for different platforms you can do:
 
-    cargo fix --prepare-for 2018 --target x86_64-pc-windows-gnu
+    cargo fix --edition --target x86_64-pc-windows-gnu
 
 or if your crate has optional features:
 
-    cargo fix --prepare-for 2018 --no-default-features --features foo
+    cargo fix --edition --no-default-features --features foo
 
 If you encounter any problems with `cargo fix` or otherwise have any questions
 or feature requests please don't hesitate to file an issue at
@@ -121,7 +124,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
         }
     }
     ops::fix(&ws, &mut ops::FixOptions {
-        edition: args.value_of("edition"),
+        edition: args.is_present("edition"),
+        prepare_for: args.value_of("prepare-for"),
         compile_opts: opts,
         allow_dirty: args.is_present("allow-dirty"),
         allow_no_vcs: args.is_present("allow-no-vcs"),

src/cargo/ops/fix.rs

@@ -21,10 +21,12 @@ use util::paths;
 
 const FIX_ENV: &str = "__CARGO_FIX_PLZ";
 const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE";
+const PREPARE_FOR_ENV: &str = "__CARGO_FIX_PREPARE_FOR";
 const EDITION_ENV: &str = "__CARGO_FIX_EDITION";
 
 pub struct FixOptions<'a> {
-    pub edition: Option<&'a str>,
+    pub edition: bool,
+    pub prepare_for: Option<&'a str>,
     pub compile_opts: CompileOptions<'a>,
     pub allow_dirty: bool,
     pub allow_no_vcs: bool,
@@ -48,9 +50,12 @@ pub fn fix(ws: &Workspace, opts: &mut FixOptions) -> CargoResult<()> {
         opts.compile_opts.build_config.extra_rustc_env.push((key, "1".to_string()));
     }
 
-    if let Some(edition) = opts.edition {
+    if opts.edition {
+        let key = EDITION_ENV.to_string();
+        opts.compile_opts.build_config.extra_rustc_env.push((key, "1".to_string()));
+    } else if let Some(edition) = opts.prepare_for {
         opts.compile_opts.build_config.extra_rustc_env.push((
-            EDITION_ENV.to_string(),
+            PREPARE_FOR_ENV.to_string(),
             edition.to_string(),
         ));
     }
@@ -465,11 +470,23 @@ fn log_failed_fix(stderr: &[u8]) -> Result<(), Error> {
 #[derive(Default)]
 struct FixArgs {
     file: Option<PathBuf>,
-    prepare_for_edition: Option<String>,
+    prepare_for_edition: PrepareFor,
     enabled_edition: Option<String>,
     other: Vec<OsString>,
 }
 
+enum PrepareFor {
+    Next,
+    Edition(String),
+    None,
+}
+
+impl Default for PrepareFor {
+    fn default() -> PrepareFor {
+        PrepareFor::None
+    }
+}
+
 impl FixArgs {
     fn get() -> FixArgs {
         let mut ret = FixArgs::default();
@@ -490,8 +507,10 @@ impl FixArgs {
             }
             ret.other.push(path.into());
         }
-        if let Ok(s) = env::var(EDITION_ENV) {
-            ret.prepare_for_edition = Some(s);
+        if let Ok(s) = env::var(PREPARE_FOR_ENV) {
+            ret.prepare_for_edition = PrepareFor::Edition(s);
+        } else if env::var(EDITION_ENV).is_ok() {
+            ret.prepare_for_edition = PrepareFor::Next;
         }
         return ret
     }
@@ -505,8 +524,15 @@ impl FixArgs {
         if let Some(edition) = &self.enabled_edition {
             cmd.arg("--edition").arg(edition);
         }
-        if let Some(prepare_for) = &self.prepare_for_edition {
-            cmd.arg("-W").arg(format!("rust-{}-compatibility", prepare_for));
+        match &self.prepare_for_edition {
+            PrepareFor::Edition(edition) => {
+                cmd.arg("-W").arg(format!("rust-{}-compatibility", edition));
+            }
+            PrepareFor::Next => {
+                let edition = self.next_edition();
+                cmd.arg("-W").arg(format!("rust-{}-compatibility", edition));
+            }
+            PrepareFor::None => {}
         }
     }
 
@@ -519,8 +545,9 @@ impl FixArgs {
     /// then yield an error to the user, indicating that this is happening.
     fn verify_not_preparing_for_enabled_edition(&self) -> CargoResult<()> {
         let edition = match &self.prepare_for_edition {
-            Some(s) => s,
-            None => return Ok(()),
+            PrepareFor::Edition(s) => s,
+            PrepareFor::Next => self.next_edition(),
+            PrepareFor::None => return Ok(()),
         };
         let enabled = match &self.enabled_edition {
             Some(s) => s,
@@ -544,8 +571,9 @@ impl FixArgs {
 
     fn warn_if_preparing_probably_inert(&self) -> CargoResult<()> {
         let edition = match &self.prepare_for_edition {
-            Some(s) => s,
-            None => return Ok(()),
+            PrepareFor::Edition(s) => s,
+            PrepareFor::Next => self.next_edition(),
+            PrepareFor::None => return Ok(()),
         };
         let path = match &self.file {
             Some(s) => s,
@@ -567,4 +595,16 @@ impl FixArgs {
 
         Ok(())
     }
+
+    fn next_edition(&self) -> &str {
+        match self.enabled_edition.as_ref().map(|s| &**s) {
+            // 2015 -> 2018,
+            None | Some("2015") => "2018",
+
+            // This'll probably be wrong in 2020, but that's future Cargo's
+            // problem. Eventually though we'll just add more editions here as
+            // necessary.
+            _ => "2018",
+        }
+    }
 }

tests/testsuite/fix.rs

@@ -284,7 +284,7 @@ fn prepare_for_2018() {
 [FINISHED] [..]
 ";
     assert_that(
-        p.cargo("fix --prepare-for 2018 --allow-no-vcs"),
+        p.cargo("fix --edition --allow-no-vcs"),
         execs().with_status(0).with_stderr(stderr).with_stdout(""),
     );
 
@@ -324,7 +324,7 @@ fn local_paths() {
 ";
 
     assert_that(
-        p.cargo("fix --prepare-for 2018 --allow-no-vcs"),
+        p.cargo("fix --edition --allow-no-vcs"),
         execs().with_status(0).with_stderr(stderr).with_stdout(""),
     );
 
@@ -362,7 +362,7 @@ issues in preparation for the 2018 edition
 [FINISHED] [..]
 ";
     assert_that(
-        p.cargo("fix --prepare-for 2018 --allow-no-vcs"),
+        p.cargo("fix --edition --allow-no-vcs"),
         execs().with_status(0).with_stderr(stderr).with_stdout(""),
     );
 }
@@ -452,7 +452,7 @@ fn specify_rustflags() {
 [FINISHED] [..]
 ";
     assert_that(
-        p.cargo("fix --prepare-for 2018 --allow-no-vcs")
+        p.cargo("fix --edition --allow-no-vcs")
             .env("RUSTFLAGS", "-C target-cpu=native"),
         execs().with_status(0).with_stderr(stderr).with_stdout(""),
     );
@@ -882,7 +882,6 @@ fn prepare_for_and_enable() {
         .build();
 
     let stderr = "\
-[CHECKING] foo v0.1.0 ([..])
 error: cannot prepare for the 2018 edition when it is enabled, so cargo cannot
 automatically fix errors in `src/lib.rs`
 
@@ -895,7 +894,7 @@ information about transitioning to the 2018 edition see:
 
 ";
     assert_that(
-        p.cargo("fix --prepare-for 2018 --allow-no-vcs")
+        p.cargo("fix --edition --allow-no-vcs")
             .masquerade_as_nightly_cargo(),
         execs()
             .with_stderr_contains(stderr)
@@ -920,7 +919,7 @@ issues in preparation for the 2018 edition
 [FINISHED] [..]
 ";
     assert_that(
-        p.cargo("fix --prepare-for 2018 --allow-no-vcs")
+        p.cargo("fix --edition --allow-no-vcs")
             .masquerade_as_nightly_cargo(),
         execs()
             .with_stderr(stderr)
@@ -966,3 +965,27 @@ fn fix_overlapping() {
     println!("{}", contents);
     assert!(contents.contains("crate::foo::<crate::A>()"));
 }
+
+#[test]
+fn both_edition_migrate_flags() {
+    if !is_nightly() {
+        return
+    }
+    let p = project()
+        .file("src/lib.rs", "")
+        .build();
+
+    let stderr = "\
+error: The argument '--edition' cannot be used with '--prepare-for <prepare-for>'
+
+USAGE:
+    cargo[..] fix --edition --message-format <FMT>
+
+For more information try --help
+";
+
+    assert_that(
+        p.cargo("fix --prepare-for 2018 --edition"),
+        execs().with_status(1).with_stderr(stderr),
+    );
+}