diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index ff38c4a8c..3687ee60f 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -96,6 +96,7 @@ pub struct Context<'a, 'cfg: 'a> { pub links: Links<'a>, pub used_in_plugin: HashSet>, pub jobserver: Client, + primary_packages: HashSet<&'a PackageId>, unit_dependencies: HashMap, Vec>>, files: Option>, } @@ -129,6 +130,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { jobserver, build_script_overridden: HashSet::new(), + primary_packages: HashSet::new(), unit_dependencies: HashMap::new(), files: None, }) @@ -321,6 +323,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { Some(target) => Some(Layout::new(self.bcx.ws, Some(target), dest)?), None => None, }; + self.primary_packages.extend(units.iter().map(|u| u.pkg.package_id())); build_unit_dependencies(units, self.bcx, &mut self.unit_dependencies)?; self.build_used_in_plugin_map(units)?; @@ -487,6 +490,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> { let dir = self.files().layout(unit.kind).incremental().display(); Ok(vec!["-C".to_string(), format!("incremental={}", dir)]) } + + pub fn is_primary_package(&self, unit: &Unit<'a>) -> bool { + self.primary_packages.contains(unit.pkg.package_id()) + } } #[derive(Default)] diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index a50228a8e..5155a6e63 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/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 45b472345..13decdf0a 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -161,11 +161,14 @@ fn compile<'a, 'cfg: 'a>( } fn rustc<'a, 'cfg>( - mut cx: &mut Context<'a, 'cfg>, + cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>, exec: &Arc, ) -> CargoResult { let mut rustc = prepare_rustc(cx, &unit.target.rustc_crate_types(), unit)?; + if cx.is_primary_package(unit) { + rustc.env("CARGO_PRIMARY_PACKAGE", "1"); + } let build_plan = cx.bcx.build_config.build_plan; let name = unit.pkg.name().to_string(); @@ -195,7 +198,7 @@ fn rustc<'a, 'cfg>( } else { root.join(&cx.files().file_stem(unit)) }.with_extension("d"); - let dep_info_loc = fingerprint::dep_info_loc(&mut cx, unit); + let dep_info_loc = fingerprint::dep_info_loc(cx, unit); rustc.args(&cx.bcx.rustflags_args(unit)?); let json_messages = cx.bcx.build_config.json_messages(); diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index f44e467a4..5d7ddb803 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 !Path::new(&path).is_absolute() { + 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 f948da406..624762b1c 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!(config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?; + 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(()) + } + 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 7e0176472..0e5f52483 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 bf0351efc..2dcad7934 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -196,7 +196,7 @@ fn fix_path_deps() { .build(); assert_that( - p.cargo("fix --allow-no-vcs") + p.cargo("fix --allow-no-vcs -p foo -p bar") .env("__CARGO_FIX_YOLO", "1"), execs() .with_status(0) @@ -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), + ); +}