Import cargo fix directly in to Cargo

This commit imports the `cargo fix` subcommand in rust-lang-nursery/rustfix
directly into Cargo as a subcommand. This should allow us to ease our
distribution story of `cargo fix` as we prepare for the upcoming 2018 edition
release.

It's been attempted here to make the code as idiomatic as possible for Cargo's
own codebase. Additionally all tests from cargo-fix were imported into Cargo's
test suite as well. After this lands and is published in nightly the `cargo-fix`
command in rust-lang-nursery/rustfix will likely be removed.

cc rust-lang/rust#52272

Cargo.toml

@@ -40,6 +40,7 @@ libc = "0.2"
 libgit2-sys = "0.7.1"
 log = "0.4"
 num_cpus = "1.0"
+rustfix = "0.4"
 same-file = "1"
 semver = { version = "0.9.0", features = ["serde"] }
 serde = "1.0"

src/bin/cargo/commands/fix.rs

@@ -0,0 +1,117 @@
+use command_prelude::*;
+
+use cargo::ops;
+
+pub fn cli() -> App {
+    subcommand("fix")
+        .about("Automatically fix lint warnings reported by rustc")
+        .arg_package_spec(
+            "Package(s) to fix",
+            "Fix all packages in the workspace",
+            "Exclude packages from the fixes",
+        )
+        .arg_jobs()
+        .arg_targets_all(
+            "Fix only this package's library",
+            "Fix only the specified binary",
+            "Fix all binaries",
+            "Fix only the specified example",
+            "Fix all examples",
+            "Fix only the specified test target",
+            "Fix all tests",
+            "Fix only the specified bench target",
+            "Fix all benches",
+            "Fix all targets (lib and bin targets by default)",
+        )
+        .arg_release("Fix artifacts in release mode, with optimizations")
+        .arg(opt("profile", "Profile to build the selected target for").value_name("PROFILE"))
+        .arg_features()
+        .arg_target_triple("Fix for the target triple")
+        .arg_target_dir()
+        .arg_manifest_path()
+        .arg_message_format()
+        .arg(
+            Arg::with_name("broken-code")
+                .long("broken-code")
+                .help("Fix code even if it already has compiler errors"),
+        )
+        .arg(
+            Arg::with_name("edition")
+                .long("prepare-for")
+                .help("Fix warnings in preparation of an edition upgrade")
+                .takes_value(true)
+                .possible_values(&["2018"]),
+        )
+        .arg(
+            Arg::with_name("allow-no-vcs")
+                .long("allow-no-vcs")
+                .help("Fix code even if a VCS was not detected"),
+        )
+        .arg(
+            Arg::with_name("allow-dirty")
+                .long("allow-dirty")
+                .help("Fix code even if the working directory is dirty"),
+        )
+        .after_help(
+            "\
+This Cargo subcommmand will automatically take rustc's suggestions from
+diagnostics like warnings and apply them to your source code. This is intended
+to help automate tasks that rustc itself already knows how to tell you to fix!
+The `cargo fix` subcommand is also being developed for the Rust 2018 edition
+to provide code the ability to easily opt-in to the new edition without having
+to worry about any breakage.
+
+Executing `cargo fix` will under the hood execute `cargo check`. Any warnings
+applicable to your crate will be automatically fixed (if possible) and all
+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
+
+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
+
+or if your crate has optional features:
+
+    cargo fix --prepare-for 2018 --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
+https://github.com/rust-lang/cargo
+",
+        )
+}
+
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
+    let ws = args.workspace(config)?;
+    let test = match args.value_of("profile") {
+        Some("test") => true,
+        None => false,
+        Some(profile) => {
+            let err = format_err!(
+                "unknown profile: `{}`, only `test` is \
+                 currently supported",
+                profile
+            );
+            return Err(CliError::new(err, 101));
+        }
+    };
+    let mode = CompileMode::Check { test };
+    ops::fix(&ws, &mut ops::FixOptions {
+        edition: args.value_of("edition"),
+        compile_opts: args.compile_options(config, mode)?,
+        allow_dirty: args.is_present("allow-dirty"),
+        allow_no_vcs: args.is_present("allow-no-vcs"),
+        broken_code: args.is_present("broken-code"),
+    })?;
+    Ok(())
+}

src/bin/cargo/commands/mod.rs

@@ -8,6 +8,7 @@ pub fn builtin() -> Vec<App> {
         clean::cli(),
         doc::cli(),
         fetch::cli(),
+        fix::cli(),
         generate_lockfile::cli(),
         git_checkout::cli(),
         init::cli(),
@@ -42,6 +43,7 @@ pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches) -> CliResu
         "clean" => clean::exec,
         "doc" => doc::exec,
         "fetch" => fetch::exec,
+        "fix" => fix::exec,
         "generate-lockfile" => generate_lockfile::exec,
         "git-checkout" => git_checkout::exec,
         "init" => init::exec,
@@ -76,6 +78,7 @@ pub mod check;
 pub mod clean;
 pub mod doc;
 pub mod fetch;
+pub mod fix;
 pub mod generate_lockfile;
 pub mod git_checkout;
 pub mod init;

src/bin/cargo/main.rs

@@ -35,10 +35,14 @@ fn main() {
         }
     };
 
-    let result = {
+    let result = match cargo::ops::fix_maybe_exec_rustc() {
-        init_git_transports(&mut config);
+        Ok(true) => Ok(()),
-        let _token = cargo::util::job::setup();
+        Ok(false) => {
-        cli::main(&mut config)
+            init_git_transports(&mut config);
+            let _token = cargo::util::job::setup();
+            cli::main(&mut config)
+        }
+        Err(e) => Err(CliError::from(e)),
     };
 
     match result {

src/cargo/core/compiler/build_config.rs

@@ -1,5 +1,7 @@
 use std::path::Path;
-use util::{CargoResult, CargoResultExt, Config};
+use std::cell::RefCell;
+
+use util::{CargoResult, CargoResultExt, Config, RustfixDiagnosticServer};
 
 /// Configuration information for a rustc build.
 #[derive(Debug)]
@@ -16,6 +18,13 @@ pub struct BuildConfig {
     pub message_format: MessageFormat,
     /// Output a build plan to stdout instead of actually compiling.
     pub build_plan: bool,
+    /// Use Cargo itself as the wrapper around rustc, only used for `cargo fix`
+    pub cargo_as_rustc_wrapper: bool,
+    /// Extra env vars to inject into rustc commands
+    pub extra_rustc_env: Vec<(String, String)>,
+    /// Extra args to inject into rustc commands
+    pub extra_rustc_args: Vec<String>,
+    pub rustfix_diagnostic_server: RefCell<Option<RustfixDiagnosticServer>>,
 }
 
 impl BuildConfig {
@@ -71,6 +80,10 @@ impl BuildConfig {
             mode,
             message_format: MessageFormat::Human,
             build_plan: false,
+            cargo_as_rustc_wrapper: false,
+            extra_rustc_env: Vec::new(),
+            extra_rustc_args: Vec::new(),
+            rustfix_diagnostic_server: RefCell::new(None),
         })
     }
 

src/cargo/core/compiler/compilation.rs

@@ -1,4 +1,5 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
+use std::env;
 use std::ffi::OsStr;
 use std::path::PathBuf;
 
@@ -80,8 +81,24 @@ pub struct Compilation<'cfg> {
 }
 
 impl<'cfg> Compilation<'cfg> {
-    pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> Compilation<'cfg> {
+    pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> CargoResult<Compilation<'cfg>> {
-        Compilation {
+        let mut rustc = bcx.rustc.process();
+        for (k, v) in bcx.build_config.extra_rustc_env.iter() {
+            rustc.env(k, v);
+        }
+        for arg in bcx.build_config.extra_rustc_args.iter() {
+            rustc.arg(arg);
+        }
+        if bcx.build_config.cargo_as_rustc_wrapper {
+            let prog = rustc.get_program().to_owned();
+            rustc.env("RUSTC", prog);
+            rustc.program(env::current_exe()?);
+        }
+        let srv = bcx.build_config.rustfix_diagnostic_server.borrow();
+        if let Some(server) = &*srv {
+            server.configure(&mut rustc);
+        }
+        Ok(Compilation {
             libraries: HashMap::new(),
             native_dirs: BTreeSet::new(), // TODO: deprecated, remove
             root_output: PathBuf::from("/"),
@@ -96,11 +113,11 @@ impl<'cfg> Compilation<'cfg> {
             cfgs: HashMap::new(),
             rustdocflags: HashMap::new(),
             config: bcx.config,
-            rustc_process: bcx.rustc.process(),
+            rustc_process: rustc,
             host: bcx.host_triple().to_string(),
             target: bcx.target_triple().to_string(),
             target_runner: LazyCell::new(),
-        }
+        })
     }
 
     /// See `process`.

src/cargo/core/compiler/context/mod.rs

@@ -118,7 +118,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
         Ok(Self {
             bcx,
-            compilation: Compilation::new(bcx),
+            compilation: Compilation::new(bcx)?,
             build_state: Arc::new(BuildState::new(&bcx.host_config, &bcx.target_config)),
             fingerprints: HashMap::new(),
             compiled: HashSet::new(),

src/cargo/core/compiler/job_queue.rs

@@ -15,6 +15,7 @@ use handle_error;
 use util::{internal, profile, CargoResult, CargoResultExt, ProcessBuilder};
 use util::{Config, DependencyQueue, Dirty, Fresh, Freshness};
 use util::{Progress, ProgressStyle};
+use util::diagnostic_server;
 
 use super::job::Job;
 use super::{BuildContext, BuildPlan, CompileMode, Context, Kind, Unit};
@@ -64,6 +65,7 @@ enum Message<'a> {
     BuildPlanMsg(String, ProcessBuilder, Arc<Vec<OutputFile>>),
     Stdout(String),
     Stderr(String),
+    FixDiagnostic(diagnostic_server::Message),
     Token(io::Result<Acquired>),
     Finish(Key<'a>, CargoResult<()>),
 }
@@ -134,9 +136,9 @@ impl<'a> JobQueue<'a> {
         self.queue.queue_finished();
 
         // We need to give a handle to the send half of our message queue to the
-        // jobserver helper thread. Unfortunately though we need the handle to be
+        // jobserver and (optionally) diagnostic helper thread. Unfortunately
-        // `'static` as that's typically what's required when spawning a
+        // though we need the handle to be `'static` as that's typically what's
-        // thread!
+        // required when spawning a thread!
         //
         // To work around this we transmute the `Sender` to a static lifetime.
         // we're only sending "longer living" messages and we should also
@@ -148,12 +150,20 @@ impl<'a> JobQueue<'a> {
         // practice.
         let tx = self.tx.clone();
         let tx = unsafe { mem::transmute::<Sender<Message<'a>>, Sender<Message<'static>>>(tx) };
+        let tx2 = tx.clone();
         let helper = cx.jobserver
             .clone()
             .into_helper_thread(move |token| {
                 drop(tx.send(Message::Token(token)));
             })
             .chain_err(|| "failed to create helper thread for jobserver management")?;
+        let _diagnostic_server = cx.bcx.build_config
+            .rustfix_diagnostic_server
+            .borrow_mut()
+            .take()
+            .map(move |srv| {
+                srv.start(move |msg| drop(tx2.send(Message::FixDiagnostic(msg))))
+            });
 
         crossbeam::scope(|scope| self.drain_the_queue(cx, plan, scope, &helper))
     }
@@ -279,6 +289,9 @@ impl<'a> JobQueue<'a> {
                         writeln!(cx.bcx.config.shell().err(), "{}", err)?;
                     }
                 }
+                Message::FixDiagnostic(msg) => {
+                    msg.print_to(cx.bcx.config)?;
+                }
                 Message::Finish(key, result) => {
                     info!("end: {:?}", key);
 

src/cargo/lib.rs

@@ -30,6 +30,7 @@ extern crate libgit2_sys;
 #[macro_use]
 extern crate log;
 extern crate num_cpus;
+extern crate rustfix;
 extern crate same_file;
 extern crate semver;
 #[macro_use]

src/cargo/ops/cargo_new.rs

@@ -409,7 +409,7 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
     Ok(())
 }
 
-fn existing_vcs_repo(path: &Path, cwd: &Path) -> bool {
+pub fn existing_vcs_repo(path: &Path, cwd: &Path) -> bool {
     GitRepo::discover(path, cwd).is_ok() || HgRepo::discover(path, cwd).is_ok()
 }
 

src/cargo/ops/fix.rs

@@ -0,0 +1,371 @@
+use std::collections::{HashMap, HashSet, BTreeSet};
+use std::env;
+use std::fs::File;
+use std::io::Write;
+use std::path::Path;
+use std::process::{self, Command, ExitStatus};
+use std::str;
+
+use failure::{Error, ResultExt};
+use git2;
+use rustfix::diagnostics::Diagnostic;
+use rustfix;
+use serde_json;
+
+use core::Workspace;
+use ops::{self, CompileOptions};
+use ops::cargo_new::existing_vcs_repo;
+use util::errors::CargoResult;
+use util::{LockServer, LockServerClient};
+use util::diagnostic_server::{Message, RustfixDiagnosticServer};
+use util::paths;
+
+const FIX_ENV: &str = "__CARGO_FIX_PLZ";
+const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE";
+
+pub struct FixOptions<'a> {
+    pub edition: Option<&'a str>,
+    pub compile_opts: CompileOptions<'a>,
+    pub allow_dirty: bool,
+    pub allow_no_vcs: bool,
+    pub broken_code: bool,
+}
+
+pub fn fix(ws: &Workspace, opts: &mut FixOptions) -> CargoResult<()> {
+    check_version_control(opts)?;
+
+    // Spin up our lock server which our subprocesses will use to synchronize
+    // fixes.
+    let lock_server = LockServer::new()?;
+    opts.compile_opts.build_config.extra_rustc_env.push((
+        FIX_ENV.to_string(),
+        lock_server.addr().to_string(),
+    ));
+    let _started = lock_server.start()?;
+
+    if opts.broken_code {
+        let key = BROKEN_CODE_ENV.to_string();
+        opts.compile_opts.build_config.extra_rustc_env.push((key, "1".to_string()));
+    }
+
+    if let Some(edition) = opts.edition {
+        opts.compile_opts.build_config.extra_rustc_args.push("-W".to_string());
+        let lint_name = format!("rust-{}-compatibility", edition);
+        opts.compile_opts.build_config.extra_rustc_args.push(lint_name);
+    }
+    opts.compile_opts.build_config.cargo_as_rustc_wrapper = true;
+    *opts.compile_opts.build_config.rustfix_diagnostic_server.borrow_mut() =
+        Some(RustfixDiagnosticServer::new()?);
+
+    ops::compile(ws, &opts.compile_opts)?;
+    Ok(())
+}
+
+fn check_version_control(opts: &FixOptions) -> CargoResult<()> {
+    if opts.allow_no_vcs {
+        return Ok(())
+    }
+    let config = opts.compile_opts.config;
+    if !existing_vcs_repo(config.cwd(), config.cwd()) {
+        bail!("no VCS found for this project and `cargo fix` can potentially \
+               perform destructive changes; if you'd like to suppress this \
+               error pass `--allow-no-vcs`")
+    }
+
+    if opts.allow_dirty {
+        return Ok(())
+    }
+
+    let mut dirty_files = Vec::new();
+    if let Ok(repo) = git2::Repository::discover(config.cwd()) {
+        for status in repo.statuses(None)?.iter() {
+            if status.status() != git2::Status::CURRENT {
+                if let Some(path) = status.path() {
+                    dirty_files.push(path.to_string());
+                }
+            }
+
+        }
+    }
+
+    if dirty_files.len() == 0 {
+        return Ok(())
+    }
+
+    let mut files_list = String::new();
+    for file in dirty_files {
+        files_list.push_str("  * ");
+        files_list.push_str(&file);
+        files_list.push_str("\n");
+    }
+
+    bail!("the working directory of this project is detected as dirty, and \
+           `cargo fix` can potentially perform destructive changes; if you'd \
+           like to suppress this error pass `--allow-dirty`, or commit the \
+           changes to these files:\n\
+           \n\
+           {}\n\
+          ", files_list);
+}
+
+pub fn fix_maybe_exec_rustc() -> CargoResult<bool> {
+    let lock_addr = match env::var(FIX_ENV) {
+        Ok(s) => s,
+        Err(_) => return Ok(false),
+    };
+
+    // Try to figure out what we're compiling by looking for a rust-like file
+    // that exists.
+    let filename = env::args()
+        .skip(1)
+        .filter(|s| s.ends_with(".rs"))
+        .filter(|s| Path::new(s).exists())
+        .next();
+
+    trace!("cargo-fix as rustc got file {:?}", filename);
+    let rustc = env::var_os("RUSTC").expect("failed to find RUSTC env var");
+
+    // Our goal is to fix only the crates that the end user is interested in.
+    // That's very likely to only mean the crates in the workspace the user is
+    // working on, not random crates.io crates.
+    //
+    // To that end we only actually try to fix things if it looks like we're
+    // compiling a Rust file and it *doesn't* have an absolute filename. That's
+    // not the best heuristic but matches what Cargo does today at least.
+    let mut fixes = FixedCrate::default();
+    if let Some(path) = filename {
+        if !Path::new(&path).is_absolute() {
+            trace!("start rustfixing {:?}", path);
+            fixes = rustfix_crate(&lock_addr, rustc.as_ref(), &path)?;
+        }
+    }
+
+    // Ok now we have our final goal of testing out the changes that we applied.
+    // If these changes went awry and actually started to cause the crate to
+    // *stop* compiling then we want to back them out and continue to print
+    // warnings to the user.
+    //
+    // If we didn't actually make any changes then we can immediately exec the
+    // new rustc, and otherwise we capture the output to hide it in the scenario
+    // that we have to back it all out.
+    let mut cmd = Command::new(&rustc);
+    cmd.args(env::args().skip(1));
+    cmd.arg("--cap-lints=warn");
+    cmd.arg("--error-format=json");
+    if fixes.original_files.len() > 0 {
+        let output = cmd.output().context("failed to spawn rustc")?;
+
+        if output.status.success() {
+            for message in fixes.messages.drain(..) {
+                message.post()?;
+            }
+        }
+
+        // If we succeeded then we'll want to commit to the changes we made, if
+        // any. If stderr is empty then there's no need for the final exec at
+        // the end, we just bail out here.
+        if output.status.success() && output.stderr.len() == 0 {
+            return Ok(true);
+        }
+
+        // Otherwise if our rustc just failed then that means that we broke the
+        // user's code with our changes. Back out everything and fall through
+        // below to recompile again.
+        if !output.status.success() {
+            for (k, v) in fixes.original_files {
+                File::create(&k)
+                    .and_then(|mut f| f.write_all(v.as_bytes()))
+                    .with_context(|_| format!("failed to write file `{}`", k))?;
+            }
+            log_failed_fix(&output.stderr)?;
+        }
+    }
+
+    let mut cmd = Command::new(&rustc);
+    cmd.args(env::args().skip(1));
+    cmd.arg("--cap-lints=warn");
+    exit_with(cmd.status().context("failed to spawn rustc")?);
+}
+
+#[derive(Default)]
+struct FixedCrate {
+    messages: Vec<Message>,
+    original_files: HashMap<String, String>,
+}
+
+fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str)
+    -> Result<FixedCrate, Error>
+{
+    // If not empty, filter by these lints
+    //
+    // TODO: Implement a way to specify this
+    let only = HashSet::new();
+
+    // First up we want to make sure that each crate is only checked by one
+    // process at a time. If two invocations concurrently check a crate then
+    // it's likely to corrupt it.
+    //
+    // Currently we do this by assigning the name on our lock to the first
+    // argument that looks like a Rust file.
+    let _lock = LockServerClient::lock(&lock_addr.parse()?, filename)?;
+
+    let mut cmd = Command::new(&rustc);
+    cmd.args(env::args().skip(1));
+    cmd.arg("--error-format=json").arg("--cap-lints=warn");
+    let output = cmd.output()
+        .with_context(|_| format!("failed to execute `{}`", rustc.display()))?;
+
+    // If rustc didn't succeed for whatever reasons then we're very likely to be
+    // looking at otherwise broken code. Let's not make things accidentally
+    // worse by applying fixes where a bug could cause *more* broken code.
+    // Instead, punt upwards which will reexec rustc over the original code,
+    // displaying pretty versions of the diagnostics we just read out.
+    if !output.status.success() && env::var_os(BROKEN_CODE_ENV).is_none() {
+        debug!(
+            "rustfixing `{:?}` failed, rustc exited with {:?}",
+            filename,
+            output.status.code()
+        );
+        return Ok(Default::default());
+    }
+
+    let fix_mode = env::var_os("__CARGO_FIX_YOLO")
+        .map(|_| rustfix::Filter::Everything)
+        .unwrap_or(rustfix::Filter::MachineApplicableOnly);
+
+    // Sift through the output of the compiler to look for JSON messages
+    // indicating fixes that we can apply.
+    let stderr = str::from_utf8(&output.stderr).context("failed to parse rustc stderr as utf-8")?;
+
+    let suggestions = stderr.lines()
+        .filter(|x| !x.is_empty())
+        .inspect(|y| trace!("line: {}", y))
+
+        // Parse each line of stderr ignoring errors as they may not all be json
+        .filter_map(|line| serde_json::from_str::<Diagnostic>(line).ok())
+
+        // From each diagnostic try to extract suggestions from rustc
+        .filter_map(|diag| rustfix::collect_suggestions(&diag, &only, fix_mode));
+
+    // Collect suggestions by file so we can apply them one at a time later.
+    let mut file_map = HashMap::new();
+    let mut num_suggestion = 0;
+    for suggestion in suggestions {
+        trace!("suggestion");
+        // Make sure we've got a file associated with this suggestion and all
+        // snippets point to the same location. Right now it's not clear what
+        // we would do with multiple locations.
+        let (file_name, range) = match suggestion.snippets.get(0) {
+            Some(s) => (s.file_name.clone(), s.line_range),
+            None => {
+                trace!("rejecting as it has no snippets {:?}", suggestion);
+                continue;
+            }
+        };
+        if !suggestion
+            .snippets
+            .iter()
+            .all(|s| s.file_name == file_name && s.line_range == range)
+        {
+            trace!("rejecting as it spans multiple files {:?}", suggestion);
+            continue;
+        }
+
+        file_map
+            .entry(file_name)
+            .or_insert_with(Vec::new)
+            .push(suggestion);
+        num_suggestion += 1;
+    }
+
+    debug!(
+        "collected {} suggestions for `{}`",
+        num_suggestion, filename
+    );
+
+    let mut original_files = HashMap::with_capacity(file_map.len());
+    let mut messages = Vec::new();
+    for (file, suggestions) in file_map {
+        // Attempt to read the source code for this file. If this fails then
+        // that'd be pretty surprising, so log a message and otherwise keep
+        // going.
+        let code = match paths::read(file.as_ref()) {
+            Ok(s) => s,
+            Err(e) => {
+                warn!("failed to read `{}`: {}", file, e);
+                continue;
+            }
+        };
+        let num_suggestions = suggestions.len();
+        debug!("applying {} fixes to {}", num_suggestions, file);
+
+        messages.push(Message::fixing(&file, num_suggestions));
+
+        match rustfix::apply_suggestions(&code, &suggestions) {
+            Err(e) => {
+                Message::ReplaceFailed {
+                    file: file,
+                    message: e.to_string(),
+                }.post()?;
+                // TODO: Add flag to decide if we want to continue or bail out
+                continue;
+            }
+            Ok(new_code) => {
+                File::create(&file)
+                    .and_then(|mut f| f.write_all(new_code.as_bytes()))
+                    .with_context(|_| format!("failed to write file `{}`", file))?;
+                original_files.insert(file, code);
+            }
+        }
+    }
+
+    Ok(FixedCrate {
+        messages,
+        original_files,
+    })
+}
+
+fn exit_with(status: ExitStatus) -> ! {
+    #[cfg(unix)]
+    {
+        use std::os::unix::prelude::*;
+        if let Some(signal) = status.signal() {
+            eprintln!("child failed with signal `{}`", signal);
+            process::exit(2);
+        }
+    }
+    process::exit(status.code().unwrap_or(3));
+}
+
+fn log_failed_fix(stderr: &[u8]) -> Result<(), Error> {
+    let stderr = str::from_utf8(stderr).context("failed to parse rustc stderr as utf-8")?;
+
+    let diagnostics = stderr
+        .lines()
+        .filter(|x| !x.is_empty())
+        .filter_map(|line| serde_json::from_str::<Diagnostic>(line).ok());
+    let mut files = BTreeSet::new();
+    for diagnostic in diagnostics {
+        for span in diagnostic.spans.into_iter() {
+            files.insert(span.file_name);
+        }
+    }
+    let mut krate = None;
+    let mut prev_dash_dash_krate_name = false;
+    for arg in env::args() {
+        if prev_dash_dash_krate_name {
+            krate = Some(arg.clone());
+        }
+
+        if arg == "--crate-name" {
+            prev_dash_dash_krate_name = true;
+        } else {
+            prev_dash_dash_krate_name = false;
+        }
+    }
+
+    let files = files.into_iter().collect();
+    Message::FixFailed { files, krate }.post()?;
+
+    Ok(())
+}

src/cargo/ops/mod.rs

@@ -21,6 +21,7 @@ pub use self::cargo_pkgid::pkgid;
 pub use self::resolve::{add_overrides, get_resolved_packages, resolve_with_previous, resolve_ws,
                         resolve_ws_precisely, resolve_ws_with_method};
 pub use self::cargo_output_metadata::{output_metadata, ExportInfo, OutputMetadataOptions};
+pub use self::fix::{fix, FixOptions, fix_maybe_exec_rustc};
 
 mod cargo_clean;
 mod cargo_compile;
@@ -38,3 +39,4 @@ mod cargo_test;
 mod lockfile;
 mod registry;
 mod resolve;
+mod fix;

src/cargo/util/diagnostic_server.rs

@@ -0,0 +1,189 @@
+//! A small TCP server to handle collection of diagnostics information in a
+//! cross-platform way for the `cargo fix` command.
+
+use std::env;
+use std::io::{BufReader, Read, Write};
+use std::net::{Shutdown, SocketAddr, TcpListener, TcpStream};
+use std::sync::Arc;
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::thread::{self, JoinHandle};
+
+use failure::{Error, ResultExt};
+use serde_json;
+
+use util::{Config, ProcessBuilder};
+use util::errors::CargoResult;
+
+const DIAGNOSICS_SERVER_VAR: &str = "__CARGO_FIX_DIAGNOSTICS_SERVER";
+const PLEASE_REPORT_THIS_BUG: &str =
+    "\
+     This likely indicates a bug in either rustc or cargo itself,\n\
+     and we would appreciate a bug report! You're likely to see \n\
+     a number of compiler warnings after this message which cargo\n\
+     attempted to fix but failed. If you could open an issue at\n\
+     https://github.com/rust-lang/cargo/issues\n\
+     quoting the full output of this command we'd be very appreciative!\n\n\
+     ";
+
+#[derive(Deserialize, Serialize)]
+pub enum Message {
+    Fixing {
+        file: String,
+        fixes: usize,
+    },
+    FixFailed {
+        files: Vec<String>,
+        krate: Option<String>,
+    },
+    ReplaceFailed {
+        file: String,
+        message: String,
+    },
+}
+
+impl Message {
+    pub fn fixing(file: &str, num: usize) -> Message {
+        Message::Fixing {
+            file: file.into(),
+            fixes: num,
+        }
+    }
+
+    pub fn post(&self) -> Result<(), Error> {
+        let addr = env::var(DIAGNOSICS_SERVER_VAR)
+            .context("diagnostics collector misconfigured")?;
+        let mut client =
+            TcpStream::connect(&addr).context("failed to connect to parent diagnostics target")?;
+
+        let s = serde_json::to_string(self).context("failed to serialize message")?;
+        client
+            .write_all(s.as_bytes())
+            .context("failed to write message to diagnostics target")?;
+        client
+            .shutdown(Shutdown::Write)
+            .context("failed to shutdown")?;
+
+        let mut tmp = Vec::new();
+        client
+            .read_to_end(&mut tmp)
+            .context("failed to receive a disconnect")?;
+
+        Ok(())
+    }
+
+    pub fn print_to(&self, config: &Config) -> CargoResult<()> {
+        match self {
+            Message::Fixing { file, fixes } => {
+                let msg = if *fixes == 1 { "fix" } else { "fixes" };
+                let msg = format!("{} ({} {})", file, fixes, msg);
+                config.shell().status("Fixing", msg)
+            }
+            Message::ReplaceFailed { file, message } => {
+                let msg = format!("error applying suggestions to `{}`\n", file);
+                config.shell().warn(&msg)?;
+                write!(
+                    config.shell().err(),
+                    "The full error message was:\n\n> {}",
+                    message,
+                )?;
+                write!(config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?;
+                Ok(())
+            }
+            Message::FixFailed { files, krate } => {
+                if let Some(ref krate) = *krate {
+                    config.shell().warn(&format!(
+                        "failed to automatically apply fixes suggested by rustc \
+                         to crate `{}`",
+                        krate,
+                    ))?;
+                } else {
+                    config.shell().warn(
+                        "failed to automatically apply fixes suggested by rustc"
+                    )?;
+                }
+                if files.len() > 0 {
+                    write!(
+                        config.shell().err(),
+                        "\nafter fixes were automatically applied the compiler \
+                         reported errors within these files:\n\n"
+                    )?;
+                    for file in files {
+                        write!(config.shell().err(), "  * {}\n", file)?;
+                    }
+                    write!(config.shell().err(), "\n")?;
+                }
+                write!(config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?;
+                Ok(())
+            }
+        }
+
+    }
+}
+
+#[derive(Debug)]
+pub struct RustfixDiagnosticServer {
+    listener: TcpListener,
+    addr: SocketAddr,
+}
+
+pub struct StartedServer {
+    addr: SocketAddr,
+    done: Arc<AtomicBool>,
+    thread: Option<JoinHandle<()>>,
+}
+
+impl RustfixDiagnosticServer {
+    pub fn new() -> Result<Self, Error> {
+        let listener = TcpListener::bind("127.0.0.1:0")
+            .with_context(|_| "failed to bind TCP listener to manage locking")?;
+        let addr = listener.local_addr()?;
+
+        Ok(RustfixDiagnosticServer { listener, addr })
+    }
+
+    pub fn configure(&self, process: &mut ProcessBuilder) {
+        process.env(DIAGNOSICS_SERVER_VAR, self.addr.to_string());
+    }
+
+    pub fn start<F>(self, on_message: F) -> Result<StartedServer, Error>
+    where
+        F: Fn(Message) + Send + 'static,
+    {
+        let addr = self.addr;
+        let done = Arc::new(AtomicBool::new(false));
+        let done2 = done.clone();
+        let thread = thread::spawn(move || {
+            self.run(&on_message, &done2);
+        });
+
+        Ok(StartedServer {
+            addr,
+            thread: Some(thread),
+            done,
+        })
+    }
+
+    fn run(self, on_message: &Fn(Message), done: &AtomicBool) {
+        while let Ok((client, _)) = self.listener.accept() {
+            let mut client = BufReader::new(client);
+            match serde_json::from_reader(client) {
+                Ok(message) => on_message(message),
+                Err(e) => warn!("invalid diagnostics message: {}", e),
+            }
+            if done.load(Ordering::SeqCst) {
+                break
+            }
+        }
+    }
+}
+
+impl Drop for StartedServer {
+    fn drop(&mut self) {
+        self.done.store(true, Ordering::SeqCst);
+        // Ignore errors here as this is largely best-effort
+        if TcpStream::connect(&self.addr).is_err() {
+            return;
+        }
+        drop(self.thread.take().unwrap().join());
+    }
+}

src/cargo/util/lockserver.rs

@@ -0,0 +1,172 @@
+//! An implementation of IPC locks, guaranteed to be released if a process dies
+//!
+//! This module implements a locking server/client where the main `cargo fix`
+//! process will start up a server and then all the client processes will
+//! connect to it. The main purpose of this file is to enusre that each crate
+//! (aka file entry point) is only fixed by one process at a time, currently
+//! concurrent fixes can't happen.
+//!
+//! The basic design here is to use a TCP server which is pretty portable across
+//! platforms. For simplicity it just uses threads as well. Clients connect to
+//! the main server, inform the server what its name is, and then wait for the
+//! server to give it the lock (aka write a byte).
+
+use std::collections::HashMap;
+use std::io::{BufRead, BufReader, Read, Write};
+use std::net::{SocketAddr, TcpListener, TcpStream};
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::{Arc, Mutex};
+use std::thread::{self, JoinHandle};
+
+use failure::{Error, ResultExt};
+
+pub struct LockServer {
+    listener: TcpListener,
+    addr: SocketAddr,
+    threads: HashMap<String, ServerClient>,
+    done: Arc<AtomicBool>,
+}
+
+pub struct LockServerStarted {
+    done: Arc<AtomicBool>,
+    addr: SocketAddr,
+    thread: Option<JoinHandle<()>>,
+}
+
+pub struct LockServerClient {
+    _socket: TcpStream,
+}
+
+struct ServerClient {
+    thread: Option<JoinHandle<()>>,
+    lock: Arc<Mutex<(bool, Vec<TcpStream>)>>,
+}
+
+impl LockServer {
+    pub fn new() -> Result<LockServer, Error> {
+        let listener = TcpListener::bind("127.0.0.1:0")
+            .with_context(|_| "failed to bind TCP listener to manage locking")?;
+        let addr = listener.local_addr()?;
+        Ok(LockServer {
+            listener,
+            addr,
+            threads: HashMap::new(),
+            done: Arc::new(AtomicBool::new(false)),
+        })
+    }
+
+    pub fn addr(&self) -> &SocketAddr {
+        &self.addr
+    }
+
+    pub fn start(self) -> Result<LockServerStarted, Error> {
+        let addr = self.addr;
+        let done = self.done.clone();
+        let thread = thread::spawn(|| {
+            self.run();
+        });
+        Ok(LockServerStarted {
+            addr,
+            thread: Some(thread),
+            done,
+        })
+    }
+
+    fn run(mut self) {
+        while let Ok((client, _)) = self.listener.accept() {
+            if self.done.load(Ordering::SeqCst) {
+                break;
+            }
+
+            // Learn the name of our connected client to figure out if it needs
+            // to wait for another process to release the lock.
+            let mut client = BufReader::new(client);
+            let mut name = String::new();
+            if client.read_line(&mut name).is_err() {
+                continue;
+            }
+            let client = client.into_inner();
+
+            // If this "named mutex" is already registered and the thread is
+            // still going, put it on the queue. Otherwise wait on the previous
+            // thread and we'll replace it just below.
+            if let Some(t) = self.threads.get_mut(&name) {
+                let mut state = t.lock.lock().unwrap();
+                if state.0 {
+                    state.1.push(client);
+                    continue;
+                }
+                drop(t.thread.take().unwrap().join());
+            }
+
+            let lock = Arc::new(Mutex::new((true, vec![client])));
+            let lock2 = lock.clone();
+            let thread = thread::spawn(move || {
+                loop {
+                    let mut client = {
+                        let mut state = lock2.lock().unwrap();
+                        if state.1.len() == 0 {
+                            state.0 = false;
+                            break;
+                        } else {
+                            state.1.remove(0)
+                        }
+                    };
+                    // Inform this client that it now has the lock and wait for
+                    // it to disconnect by waiting for EOF.
+                    if client.write_all(&[1]).is_err() {
+                        continue;
+                    }
+                    let mut dst = Vec::new();
+                    drop(client.read_to_end(&mut dst));
+                }
+            });
+
+            self.threads.insert(
+                name,
+                ServerClient {
+                    thread: Some(thread),
+                    lock,
+                },
+            );
+        }
+    }
+}
+
+impl Drop for LockServer {
+    fn drop(&mut self) {
+        for (_, mut client) in self.threads.drain() {
+            if let Some(thread) = client.thread.take() {
+                drop(thread.join());
+            }
+        }
+    }
+}
+
+impl Drop for LockServerStarted {
+    fn drop(&mut self) {
+        self.done.store(true, Ordering::SeqCst);
+        // Ignore errors here as this is largely best-effort
+        if TcpStream::connect(&self.addr).is_err() {
+            return;
+        }
+        drop(self.thread.take().unwrap().join());
+    }
+}
+
+impl LockServerClient {
+    pub fn lock(addr: &SocketAddr, name: &str) -> Result<LockServerClient, Error> {
+        let mut client =
+            TcpStream::connect(&addr).with_context(|_| "failed to connect to parent lock server")?;
+        client
+            .write_all(name.as_bytes())
+            .and_then(|_| client.write_all(b"\n"))
+            .with_context(|_| "failed to write to lock server")?;
+        let mut buf = [0];
+        client
+            .read_exact(&mut buf)
+            .with_context(|_| "failed to acquire lock")?;
+        Ok(LockServerClient { _socket: client })
+    }
+}
+

src/cargo/util/mod.rs

@@ -18,6 +18,8 @@ pub use self::to_url::ToUrl;
 pub use self::vcs::{FossilRepo, GitRepo, HgRepo, PijulRepo};
 pub use self::read2::read2;
 pub use self::progress::{Progress, ProgressStyle};
+pub use self::lockserver::{LockServer, LockServerStarted, LockServerClient};
+pub use self::diagnostic_server::RustfixDiagnosticServer;
 
 pub mod config;
 pub mod errors;
@@ -42,3 +44,5 @@ mod vcs;
 mod flock;
 mod read2;
 mod progress;
+mod lockserver;
+pub mod diagnostic_server;

tests/testsuite/build.rs

@@ -535,7 +535,7 @@ Caused by:
 #[test]
 fn cargo_compile_without_manifest() {
     let tmpdir = tempfile::Builder::new().prefix("cargo").tempdir().unwrap();
-    let p = ProjectBuilder::new("foo", tmpdir.path().to_path_buf()).build();
+    let p = ProjectBuilder::new(tmpdir.path().to_path_buf()).build();
 
     assert_that(
         p.cargo("build"),
@@ -3587,7 +3587,7 @@ fn ignore_dotdirs() {
 
 #[test]
 fn dotdir_root() {
-    let p = ProjectBuilder::new("foo", root().join(".foo"))
+    let p = ProjectBuilder::new(root().join(".foo"))
         .file(
             "Cargo.toml",
             r#"

tests/testsuite/cargotest/support/mod.rs

@@ -14,6 +14,7 @@ use cargo::util::ProcessError;
 use hamcrest as ham;
 use serde_json::{self, Value};
 use url::Url;
+use tempfile::TempDir;
 
 use cargotest::support::paths::CargoPathExt;
 
@@ -94,15 +95,13 @@ impl SymlinkBuilder {
     }
 }
 
-#[derive(PartialEq, Clone)]
+pub enum Project {
-pub struct Project {
+    Rooted(PathBuf),
-    root: PathBuf,
+    Temp(TempDir),
 }
 
 #[must_use]
-#[derive(PartialEq, Clone)]
 pub struct ProjectBuilder {
-    name: String,
     root: Project,
     files: Vec<FileBuilder>,
     symlinks: Vec<SymlinkBuilder>,
@@ -119,10 +118,9 @@ impl ProjectBuilder {
         self.root.target_debug_dir()
     }
 
-    pub fn new(name: &str, root: PathBuf) -> ProjectBuilder {
+    pub fn new(root: PathBuf) -> ProjectBuilder {
         ProjectBuilder {
-            name: name.to_string(),
+            root: Project::Rooted(root),
-            root: Project { root },
             files: vec![],
             symlinks: vec![],
         }
@@ -136,25 +134,30 @@ impl ProjectBuilder {
 
     fn _file(&mut self, path: &Path, body: &str) {
         self.files
-            .push(FileBuilder::new(self.root.root.join(path), body));
+            .push(FileBuilder::new(self.root.root().join(path), body));
     }
 
     /// Add a symlink to the project.
     pub fn symlink<T: AsRef<Path>>(mut self, dst: T, src: T) -> Self {
         self.symlinks.push(SymlinkBuilder::new(
-            self.root.root.join(dst),
+            self.root.root().join(dst),
-            self.root.root.join(src),
+            self.root.root().join(src),
         ));
         self
     }
 
+    pub fn use_temp_dir(mut self) -> Self {
+        self.root = Project::Temp(TempDir::new().unwrap());
+        self
+    }
+
     /// Create the project.
     pub fn build(self) -> Project {
         // First, clean the directory if it already exists
         self.rm_root();
 
         // Create the empty directory
-        self.root.root.mkdir_p();
+        self.root.root().mkdir_p();
 
         for file in self.files.iter() {
             file.mk();
@@ -165,7 +168,6 @@ impl ProjectBuilder {
         }
 
         let ProjectBuilder {
-            name: _,
             root,
             files: _,
             symlinks: _,
@@ -175,19 +177,22 @@ impl ProjectBuilder {
     }
 
     fn rm_root(&self) {
-        self.root.root.rm_rf()
+        self.root.root().rm_rf()
     }
 }
 
 impl Project {
     /// Root of the project, ex: `/path/to/cargo/target/cit/t0/foo`
     pub fn root(&self) -> PathBuf {
-        self.root.clone()
+        match self {
+            Project::Rooted(p) => p.clone(),
+            Project::Temp(t) => t.path().to_path_buf(),
+        }
     }
 
     /// Project's target dir, ex: `/path/to/cargo/target/cit/t0/foo/target`
     pub fn build_dir(&self) -> PathBuf {
-        self.root.join("target")
+        self.root().join("target")
     }
 
     /// Project's debug dir, ex: `/path/to/cargo/target/cit/t0/foo/target/debug`
@@ -243,7 +248,7 @@ impl Project {
 
     /// Change the contents of an existing file.
     pub fn change_file(&self, path: &str, body: &str) {
-        FileBuilder::new(self.root.join(path), body).mk()
+        FileBuilder::new(self.root().join(path), body).mk()
     }
 
     /// Create a `ProcessBuilder` to run a program in the project.
@@ -275,8 +280,13 @@ impl Project {
 
     /// Returns the contents of `Cargo.lock`.
     pub fn read_lockfile(&self) -> String {
+        self.read_file("Cargo.lock")
+    }
+
+    /// Returns the contents of a path in the project root
+    pub fn read_file(&self, path: &str) -> String {
         let mut buffer = String::new();
-        fs::File::open(self.root().join("Cargo.lock"))
+        fs::File::open(self.root().join(path))
             .unwrap()
             .read_to_string(&mut buffer)
             .unwrap();
@@ -336,12 +346,12 @@ impl Project {
 
 // Generates a project layout
 pub fn project(name: &str) -> ProjectBuilder {
-    ProjectBuilder::new(name, paths::root().join(name))
+    ProjectBuilder::new(paths::root().join(name))
 }
 
 // Generates a project layout inside our fake home dir
 pub fn project_in_home(name: &str) -> ProjectBuilder {
-    ProjectBuilder::new(name, paths::home().join(name))
+    ProjectBuilder::new(paths::home().join(name))
 }
 
 // === Helpers ===
@@ -1174,6 +1184,7 @@ fn substitute_macros(input: &str) -> String {
         ("[REPLACING]", "   Replacing"),
         ("[UNPACKING]", "   Unpacking"),
         ("[SUMMARY]", "     Summary"),
+        ("[FIXING]", "      Fixing"),
         ("[EXE]", if cfg!(windows) { ".exe" } else { "" }),
         ("[/]", if cfg!(windows) { "\\" } else { "/" }),
     ];

tests/testsuite/generate_lockfile.rs

@@ -253,7 +253,7 @@ fn cargo_update_generate_lockfile() {
 
 #[test]
 fn duplicate_entries_in_lockfile() {
-    let _a = ProjectBuilder::new("a", paths::root().join("a"))
+    let _a = ProjectBuilder::new(paths::root().join("a"))
         .file(
             "Cargo.toml",
             r#"
@@ -276,12 +276,12 @@ fn duplicate_entries_in_lockfile() {
         version = "0.0.1"
         "#;
 
-    let _common_in_a = ProjectBuilder::new("common", paths::root().join("a/common"))
+    let _common_in_a = ProjectBuilder::new(paths::root().join("a/common"))
         .file("Cargo.toml", common_toml)
         .file("src/lib.rs", "")
         .build();
 
-    let b = ProjectBuilder::new("common", paths::root().join("b"))
+    let b = ProjectBuilder::new(paths::root().join("b"))
         .file(
             "Cargo.toml",
             r#"
@@ -298,7 +298,7 @@ fn duplicate_entries_in_lockfile() {
         .file("src/lib.rs", "")
         .build();
 
-    let _common_in_b = ProjectBuilder::new("common", paths::root().join("b/common"))
+    let _common_in_b = ProjectBuilder::new(paths::root().join("b/common"))
         .file("Cargo.toml", common_toml)
         .file("src/lib.rs", "")
         .build();

tests/testsuite/main.rs

@@ -51,6 +51,7 @@ mod directory;
 mod doc;
 mod features;
 mod fetch;
+mod fix;
 mod freshness;
 mod generate_lockfile;
 mod git;