Start giving cargo fix a dedicated CLI

This commit pulls in `clap` to have a location to actually insert a CLI for
`cargo fix` itself. This'll hopefully allow us to continue to configure
cargo-fix's behavior while also allowing passing arguments down to Cargo.

This starts out by adding a `--broken-code` option which can be used to specify
that previously broken code should be fixed, despite it being originally
broken.

Closes rust-lang#70
Closes rust-lang#71
Closes rust-lang#84

cargo-fix/Cargo.toml

@@ -18,6 +18,7 @@ rustfix = { path = "..", version = "0.2" }
 serde_json = "1"
 log = "0.4"
 env_logger = { version = "0.5", default-features = false }
+clap = "2.31"
 
 [dev-dependencies]
 difference = "2"

cargo-fix/src/cli.rs

@@ -0,0 +1,71 @@
+use std::env;
+use std::process::Command;
+
+use failure::{Error, ResultExt};
+use clap::{Arg, App, SubCommand, AppSettings};
+
+use lock::Server;
+use super::exit_with;
+
+pub fn run() -> Result<(), Error> {
+    let matches = App::new("Cargo Fix")
+        .bin_name("cargo")
+        .subcommand(
+            SubCommand::with_name("fix")
+                .version(env!("CARGO_PKG_VERSION"))
+                .author("The Rust Project Developers")
+                .about("Automatically apply rustc's suggestions about fixing code")
+                .setting(AppSettings::TrailingVarArg)
+                .arg(Arg::with_name("args").multiple(true))
+                .arg(
+                    Arg::with_name("broken-code")
+                        .long("broken-code")
+                        .help("Fix code even if it already has compiler errors")
+                )
+        )
+        .get_matches();
+    let matches = match matches.subcommand() {
+        ("fix", Some(matches)) => matches,
+        _ => bail!("unknown cli arguments passed"),
+    };
+
+    if matches.is_present("broken-code") {
+        env::set_var("__CARGO_FIX_BROKEN_CODE", "1");
+    }
+
+    // Spin up our lock server which our subprocesses will use to synchronize
+    // fixes.
+    let _lockserver = Server::new()?.start()?;
+
+    let cargo = env::var_os("CARGO").unwrap_or("cargo".into());
+    let mut cmd = Command::new(&cargo);
+    // TODO: shouldn't hardcode `check` here, we want to allow things like
+    // `cargo fix bench` or something like that
+    //
+    // TODO: somehow we need to force `check` to actually do something here, if
+    // `cargo check` was previously run it won't actually do anything again.
+    cmd.arg("check");
+    if let Some(args) = matches.values_of("args") {
+        cmd.args(args);
+    }
+
+    // Override the rustc compiler as ourselves. That way whenever rustc would
+    // run we run instead and have an opportunity to inject fixes.
+    let me = env::current_exe()
+        .context("failed to learn about path to current exe")?;
+    cmd.env("RUSTC", &me)
+        .env("__CARGO_FIX_NOW_RUSTC", "1");
+    if let Some(rustc) = env::var_os("RUSTC") {
+        cmd.env("RUSTC_ORIGINAL", rustc);
+    }
+
+    // An now execute all of Cargo! This'll fix everything along the way.
+    //
+    // TODO: we probably want to do something fancy here like collect results
+    // from the client processes and print out a summary of what happened.
+    let status = cmd.status()
+        .with_context(|e| {
+            format!("failed to execute `{}`: {}", cargo.to_string_lossy(), e)
+        })?;
+    exit_with(status);
+}

cargo-fix/src/main.rs

@@ -1,3 +1,4 @@
+extern crate clap;
 #[macro_use]
 extern crate failure;
 extern crate rustfix;
@@ -17,14 +18,15 @@ use std::path::Path;
 use rustfix::diagnostics::Diagnostic;
 use failure::{Error, ResultExt};
 
+mod cli;
 mod lock;
 
 fn main() {
     env_logger::init();
     let result = if env::var("__CARGO_FIX_NOW_RUSTC").is_ok() {
         cargo_fix_rustc()
     } else {
-        cargo_fix()
+        cli::run()
     };
     let err = match result {
         Ok(()) => return,
@@ -37,42 +39,6 @@ fn main() {
     process::exit(102);
 }
 
-fn cargo_fix() -> Result<(), Error> {
-    // Spin up our lock server which our subprocesses will use to synchronize
-    // fixes.
-    let _lockserver = lock::Server::new()?.start()?;
-
-    let cargo = env::var_os("CARGO").unwrap_or("cargo".into());
-    let mut cmd = Command::new(&cargo);
-    // TODO: shouldn't hardcode `check` here, we want to allow things like
-    // `cargo fix bench` or something like that
-    //
-    // TODO: somehow we need to force `check` to actually do something here, if
-    // `cargo check` was previously run it won't actually do anything again.
-    cmd.arg("check");
-    cmd.args(env::args().skip(2)); // skip `cmd-fix fix`
-
-    // Override the rustc compiler as ourselves. That way whenever rustc would
-    // run we run instead and have an opportunity to inject fixes.
-    let me = env::current_exe()
-        .context("failed to learn about path to current exe")?;
-    cmd.env("RUSTC", &me)
-        .env("__CARGO_FIX_NOW_RUSTC", "1");
-    if let Some(rustc) = env::var_os("RUSTC") {
-        cmd.env("RUSTC_ORIGINAL", rustc);
-    }
-
-    // An now execute all of Cargo! This'll fix everything along the way.
-    //
-    // TODO: we probably want to do something fancy here like collect results
-    // from the client processes and print out a summary of what happened.
-    let status = cmd.status()
-        .with_context(|e| {
-            format!("failed to execute `{}`: {}", cargo.to_string_lossy(), e)
-        })?;
-    exit_with(status);
-}
-
 fn cargo_fix_rustc() -> Result<(), Error> {
     // Try to figure out what we're compiling by looking for a rust-like file
     // that exists.
@@ -163,7 +129,7 @@ fn rustfix_crate(rustc: &Path, filename: &str) -> Result<HashMap<String, String>
     //
     // TODO: this should be configurable by the CLI to sometimes proceed to
     // attempt to fix broken code.
-    if !output.status.success() {
+    if !output.status.success() && env::var_os("__CARGO_FIX_BROKEN_CODE").is_none() {
         return Ok(HashMap::new())
     }
 

cargo-fix/tests/all/broken_build.rs

@@ -24,6 +24,25 @@ fn do_not_fix_broken_builds() {
     assert!(p.read("src/lib.rs").contains("let mut x = 3;"));
 }
 
+#[test]
+fn fix_broken_if_requested() {
+    let p = project()
+        .file(
+            "src/lib.rs",
+            r#"
+                fn foo(a: &u32) -> u32 { a + 1 }
+                pub fn bar() {
+                    foo(1);
+                }
+            "#
+        )
+        .build();
+
+    p.expect_cmd("cargo-fix fix --broken-code")
+        .status(0)
+        .run();
+}
+
 #[test]
 fn broken_fixes_backed_out() {
     let p = project()

cargo-fix/tests/all/subtargets.rs

@@ -28,7 +28,7 @@ fn fixes_missing_ampersand() {
 [COMPILING] foo v0.1.0 (CWD)
 [FINISHED] dev [unoptimized + debuginfo]
 ";
-    p.expect_cmd("cargo fix --all-targets").stdout("").stderr(stderr).run();
+    p.expect_cmd("cargo fix -- --all-targets").stdout("").stderr(stderr).run();
     p.expect_cmd("cargo build").run();
     p.expect_cmd("cargo test").run();
 }
@@ -57,6 +57,6 @@ fn fix_features() {
 
     p.expect_cmd("cargo fix").run();
     p.expect_cmd("cargo build").run();
-    p.expect_cmd("cargo fix --features bar").run();
+    p.expect_cmd("cargo fix -- --features bar").run();
     p.expect_cmd("cargo build --features bar").run();
 }