diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index a50228a8e01..5155a6e632b 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -16,7 +16,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 util::diagnostic_server::{self, DiagnosticPrinter}; use super::job::Job; use super::{BuildContext, BuildPlan, CompileMode, Context, Kind, Unit}; @@ -200,6 +200,7 @@ impl<'a> JobQueue<'a> { let mut tokens = Vec::new(); let mut queue = Vec::new(); let build_plan = cx.bcx.build_config.build_plan; + let mut print = DiagnosticPrinter::new(cx.bcx.config); trace!("queue: {:#?}", self.queue); // Iteratively execute the entire dependency graph. Each turn of the @@ -311,7 +312,7 @@ impl<'a> JobQueue<'a> { } } Message::FixDiagnostic(msg) => { - msg.print_to(cx.bcx.config)?; + print.print(&msg)?; } Message::Finish(key, result) => { info!("end: {:?}", key); diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 0fa16c8158d..5d7ddb80345 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -1,8 +1,8 @@ use std::collections::{HashMap, HashSet, BTreeSet}; use std::env; -use std::fs::File; -use std::io::Write; -use std::path::Path; +use std::ffi::OsString; +use std::fs; +use std::path::{Path, PathBuf}; use std::process::{self, Command, ExitStatus}; use std::str; @@ -21,6 +21,7 @@ use util::paths; const FIX_ENV: &str = "__CARGO_FIX_PLZ"; const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE"; +const EDITION_ENV: &str = "__CARGO_FIX_EDITION"; pub struct FixOptions<'a> { pub edition: Option<&'a str>, @@ -48,9 +49,10 @@ pub fn fix(ws: &Workspace, opts: &mut FixOptions) -> CargoResult<()> { } 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.extra_rustc_env.push(( + EDITION_ENV.to_string(), + edition.to_string(), + )); } opts.compile_opts.build_config.cargo_as_rustc_wrapper = true; *opts.compile_opts.build_config.rustfix_diagnostic_server.borrow_mut() = @@ -115,14 +117,8 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { 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")) - .find(|s| Path::new(s).exists()); - - trace!("cargo-fix as rustc got file {:?}", filename); + let args = FixArgs::get(); + trace!("cargo-fix as rustc got file {:?}", args.file); 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. @@ -133,10 +129,10 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { // 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 let Some(path) = &args.file { if env::var("CARGO_PRIMARY_PACKAGE").is_ok() { trace!("start rustfixing {:?}", path); - fixes = rustfix_crate(&lock_addr, rustc.as_ref(), &path)?; + fixes = rustfix_crate(&lock_addr, rustc.as_ref(), path, &args)?; } } @@ -148,11 +144,10 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { // 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.is_empty() { + let mut cmd = Command::new(&rustc); + args.apply(&mut cmd); + cmd.arg("--error-format=json"); let output = cmd.output().context("failed to spawn rustc")?; if output.status.success() { @@ -173,8 +168,7 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { // 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())) + fs::write(&k, v) .with_context(|_| format!("failed to write file `{}`", k))?; } log_failed_fix(&output.stderr)?; @@ -182,8 +176,7 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { } let mut cmd = Command::new(&rustc); - cmd.args(env::args().skip(1)); - cmd.arg("--cap-lints=warn"); + args.apply(&mut cmd); exit_with(cmd.status().context("failed to spawn rustc")?); } @@ -193,9 +186,12 @@ struct FixedCrate { original_files: HashMap, } -fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str) +fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &Path, args: &FixArgs) -> Result { + args.verify_not_preparing_for_enabled_edition()?; + args.warn_if_preparing_probably_inert()?; + // If not empty, filter by these lints // // TODO: Implement a way to specify this @@ -210,8 +206,8 @@ fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str) 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"); + cmd.arg("--error-format=json"); + args.apply(&mut cmd); let output = cmd.output() .with_context(|_| format!("failed to execute `{}`", rustc.display()))?; @@ -280,7 +276,8 @@ fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str) debug!( "collected {} suggestions for `{}`", - num_suggestion, filename + num_suggestion, + filename.display(), ); let mut original_files = HashMap::with_capacity(file_map.len()); @@ -311,8 +308,7 @@ fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str) continue; } Ok(new_code) => { - File::create(&file) - .and_then(|mut f| f.write_all(new_code.as_bytes())) + fs::write(&file, new_code) .with_context(|_| format!("failed to write file `{}`", file))?; original_files.insert(file, code); } @@ -369,3 +365,110 @@ fn log_failed_fix(stderr: &[u8]) -> Result<(), Error> { Ok(()) } + +#[derive(Default)] +struct FixArgs { + file: Option, + prepare_for_edition: Option, + enabled_edition: Option, + other: Vec, +} + +impl FixArgs { + fn get() -> FixArgs { + let mut ret = FixArgs::default(); + for arg in env::args_os().skip(1) { + let path = PathBuf::from(arg); + if path.extension().and_then(|s| s.to_str()) == Some("rs") { + if path.exists() { + ret.file = Some(path); + continue + } + } + if let Some(s) = path.to_str() { + let prefix = "--edition="; + if s.starts_with(prefix) { + ret.enabled_edition = Some(s[prefix.len()..].to_string()); + continue + } + } + ret.other.push(path.into()); + } + if let Ok(s) = env::var(EDITION_ENV) { + ret.prepare_for_edition = Some(s); + } + return ret + } + + fn apply(&self, cmd: &mut Command) { + if let Some(path) = &self.file { + cmd.arg(path); + } + cmd.args(&self.other) + .arg("--cap-lints=warn"); + 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)); + } + } + + /// Verify that we're not both preparing for an enabled edition and enabling + /// the edition. + /// + /// This indicates that `cargo fix --prepare-for` is being executed out of + /// order with enabling the edition itself, meaning that we wouldn't + /// actually be able to fix anything! If it looks like this is happening + /// 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(()), + }; + let enabled = match &self.enabled_edition { + Some(s) => s, + None => return Ok(()), + }; + if edition != enabled { + return Ok(()) + } + let path = match &self.file { + Some(s) => s, + None => return Ok(()), + }; + + Message::EditionAlreadyEnabled { + file: path.display().to_string(), + edition: edition.to_string(), + }.post()?; + + process::exit(1); + } + + fn warn_if_preparing_probably_inert(&self) -> CargoResult<()> { + let edition = match &self.prepare_for_edition { + Some(s) => s, + None => return Ok(()), + }; + let path = match &self.file { + Some(s) => s, + None => return Ok(()), + }; + let contents = match fs::read_to_string(path) { + Ok(s) => s, + Err(_) => return Ok(()) + }; + + let feature_name = format!("rust_{}_preview", edition); + if contents.contains(&feature_name) { + return Ok(()) + } + Message::PreviewNotFound { + file: path.display().to_string(), + edition: edition.to_string(), + }.post()?; + + Ok(()) + } +} diff --git a/src/cargo/util/diagnostic_server.rs b/src/cargo/util/diagnostic_server.rs index f948da4068b..624762b1cc8 100644 --- a/src/cargo/util/diagnostic_server.rs +++ b/src/cargo/util/diagnostic_server.rs @@ -1,6 +1,7 @@ //! A small TCP server to handle collection of diagnostics information in a //! cross-platform way for the `cargo fix` command. +use std::collections::HashSet; use std::env; use std::io::{BufReader, Read, Write}; use std::net::{Shutdown, SocketAddr, TcpListener, TcpStream}; @@ -39,6 +40,14 @@ pub enum Message { file: String, message: String, }, + PreviewNotFound { + file: String, + edition: String, + }, + EditionAlreadyEnabled { + file: String, + edition: String, + }, } impl Message { @@ -70,49 +79,104 @@ impl Message { Ok(()) } +} - pub fn print_to(&self, config: &Config) -> CargoResult<()> { - match self { +pub struct DiagnosticPrinter<'a> { + config: &'a Config, + preview_not_found: HashSet, + edition_already_enabled: HashSet, +} + +impl<'a> DiagnosticPrinter<'a> { + pub fn new(config: &'a Config) -> DiagnosticPrinter<'a> { + DiagnosticPrinter { + config, + preview_not_found: HashSet::new(), + edition_already_enabled: HashSet::new(), + } + } + + pub fn print(&mut self, msg: &Message) -> CargoResult<()> { + match msg { Message::Fixing { file, fixes } => { let msg = if *fixes == 1 { "fix" } else { "fixes" }; let msg = format!("{} ({} {})", file, fixes, msg); - config.shell().status("Fixing", msg) + self.config.shell().status("Fixing", msg) } Message::ReplaceFailed { file, message } => { let msg = format!("error applying suggestions to `{}`\n", file); - config.shell().warn(&msg)?; + self.config.shell().warn(&msg)?; write!( - config.shell().err(), + self.config.shell().err(), "The full error message was:\n\n> {}\n\n", message, )?; - write!(config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?; + write!(self.config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?; Ok(()) } Message::FixFailed { files, krate } => { if let Some(ref krate) = *krate { - config.shell().warn(&format!( + self.config.shell().warn(&format!( "failed to automatically apply fixes suggested by rustc \ to crate `{}`", krate, ))?; } else { - config.shell().warn( + self.config.shell().warn( "failed to automatically apply fixes suggested by rustc" )?; } if !files.is_empty() { writeln!( - config.shell().err(), + self.config.shell().err(), "\nafter fixes were automatically applied the compiler \ reported errors within these files:\n" )?; for file in files { - writeln!(config.shell().err(), " * {}", file)?; + writeln!(self.config.shell().err(), " * {}", file)?; } - writeln!(config.shell().err())?; + writeln!(self.config.shell().err())?; + } + write!(self.config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?; + Ok(()) + } + Message::PreviewNotFound { file, edition } => { + // By default we're fixing a lot of things concurrently, don't + // warn about the same file multiple times. + if !self.preview_not_found.insert(file.clone()) { + return Ok(()) } - write!(config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?; + self.config.shell().warn(&format!( + "failed to find `#![feature(rust_{}_preview)]` in `{}`\n\ + this may cause `cargo fix` to not be able to fix all\n\ + issues in preparation for the {0} edition", + edition, + file, + ))?; + Ok(()) + } + Message::EditionAlreadyEnabled { file, edition } => { + // Like above, only warn once per file + if !self.edition_already_enabled.insert(file.clone()) { + return Ok(()) + } + + let msg = format!( + "\ +cannot prepare for the {} edition when it is enabled, so cargo cannot +automatically fix errors in `{}` + +To prepare for the {0} edition you should first remove `edition = '{0}'` from +your `Cargo.toml` and then rerun this command. Once all warnings have been fixed +then you can re-enable the `edition` key in `Cargo.toml`. For some more +information about transitioning to the {0} edition see: + + https://rust-lang-nursery.github.io/edition-guide/editions/transitioning.html +", + edition, + file, + ); + self.config.shell().error(&msg)?; Ok(()) } } diff --git a/src/cargo/util/lockserver.rs b/src/cargo/util/lockserver.rs index 7e0176472b6..0e5f524835c 100644 --- a/src/cargo/util/lockserver.rs +++ b/src/cargo/util/lockserver.rs @@ -14,6 +14,7 @@ use std::collections::HashMap; use std::io::{BufRead, BufReader, Read, Write}; use std::net::{SocketAddr, TcpListener, TcpStream}; +use std::path::Path; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use std::thread::{self, JoinHandle}; @@ -155,11 +156,11 @@ impl Drop for LockServerStarted { } impl LockServerClient { - pub fn lock(addr: &SocketAddr, name: &str) -> Result { + pub fn lock(addr: &SocketAddr, name: &Path) -> Result { let mut client = TcpStream::connect(&addr).with_context(|_| "failed to connect to parent lock server")?; client - .write_all(name.as_bytes()) + .write_all(name.display().to_string().as_bytes()) .and_then(|_| client.write_all(b"\n")) .with_context(|_| "failed to write to lock server")?; let mut buf = [0]; diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 386d0e33836..2dcad7934b8 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -356,6 +356,9 @@ fn local_paths_no_fix() { let stderr = "\ [CHECKING] foo v0.0.1 ([..]) +warning: failed to find `#![feature(rust_2018_preview)]` in `src[/]lib.rs` +this may cause `cargo fix` to not be able to fix all +issues in preparation for the 2018 edition [FINISHED] [..] "; assert_that( @@ -857,3 +860,70 @@ fn fix_all_targets_by_default() { assert!(!p.read_file("src/lib.rs").contains("let mut x")); assert!(!p.read_file("tests/foo.rs").contains("let mut x")); } + +#[test] +fn prepare_for_and_enable() { + if !is_nightly() { + return + } + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ['edition'] + + [package] + name = 'foo' + version = '0.1.0' + edition = '2018' + "#, + ) + .file("src/lib.rs", "") + .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` + +To prepare for the 2018 edition you should first remove `edition = '2018'` from +your `Cargo.toml` and then rerun this command. Once all warnings have been fixed +then you can re-enable the `edition` key in `Cargo.toml`. For some more +information about transitioning to the 2018 edition see: + + https://[..] + +"; + assert_that( + p.cargo("fix --prepare-for 2018 --allow-no-vcs") + .masquerade_as_nightly_cargo(), + execs() + .with_stderr_contains(stderr) + .with_status(101), + ); +} + +#[test] +fn prepare_for_without_feature_issues_warning() { + if !is_nightly() { + return + } + let p = project() + .file("src/lib.rs", "") + .build(); + + let stderr = "\ +[CHECKING] foo v0.0.1 ([..]) +warning: failed to find `#![feature(rust_2018_preview)]` in `src[/]lib.rs` +this may cause `cargo fix` to not be able to fix all +issues in preparation for the 2018 edition +[FINISHED] [..] +"; + assert_that( + p.cargo("fix --prepare-for 2018 --allow-no-vcs") + .masquerade_as_nightly_cargo(), + execs() + .with_stderr(stderr) + .with_status(0), + ); +}