From fffb05d8fc1e214d13b0f6c7e310c3ad4ab6154b Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 11 Dec 2018 16:24:20 -0800 Subject: [PATCH 1/2] Display errors when `cargo fix` fails. It can be difficult to figure out what's wrong when a user reports that `cargo fix` fails. There's often a large list of warnings, and it can be hard to figure out which one caused a compile error. --- Cargo.toml | 5 +++- src/cargo/ops/fix.rs | 9 ++++++- src/cargo/util/diagnostic_server.rs | 23 ++++++++++++++++- tests/testsuite/fix.rs | 39 +++++++++++++++++++++-------- 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d55278c2089..478f51f54fe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ log = "0.4.6" libgit2-sys = "0.7.9" num_cpus = "1.0" opener = "0.3.0" -rustfix = "0.4.2" +rustfix = "0.4.3" same-file = "1" semver = { version = "0.9.0", features = ["serde"] } serde = { version = "1.0.82", features = ['derive'] } @@ -108,3 +108,6 @@ doc = false deny-warnings = [] vendored-openssl = ['openssl/vendored'] pretty-env-logger = ['pretty_env_logger'] + +[patch.crates-io] +rustfix = {git="https://github.com/ehuss/rustfix", branch="pub-rendered"} diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index f9e5ce1e88f..431cac156b5 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -496,7 +496,9 @@ fn log_failed_fix(stderr: &[u8]) -> Result<(), Error> { .filter(|x| !x.is_empty()) .filter_map(|line| serde_json::from_str::(line).ok()); let mut files = BTreeSet::new(); + let mut errors = Vec::new(); for diagnostic in diagnostics { + errors.push(diagnostic.rendered.unwrap_or(diagnostic.message)); for span in diagnostic.spans.into_iter() { files.insert(span.file_name); } @@ -516,7 +518,12 @@ fn log_failed_fix(stderr: &[u8]) -> Result<(), Error> { } let files = files.into_iter().collect(); - Message::FixFailed { files, krate }.post()?; + Message::FixFailed { + files, + krate, + errors, + } + .post()?; Ok(()) } diff --git a/src/cargo/util/diagnostic_server.rs b/src/cargo/util/diagnostic_server.rs index 854795256fe..48da0a3d2bd 100644 --- a/src/cargo/util/diagnostic_server.rs +++ b/src/cargo/util/diagnostic_server.rs @@ -36,6 +36,7 @@ pub enum Message { FixFailed { files: Vec, krate: Option, + errors: Vec, }, ReplaceFailed { file: String, @@ -109,7 +110,11 @@ impl<'a> DiagnosticPrinter<'a> { write!(self.config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?; Ok(()) } - Message::FixFailed { files, krate } => { + Message::FixFailed { + files, + krate, + errors, + } => { if let Some(ref krate) = *krate { self.config.shell().warn(&format!( "failed to automatically apply fixes suggested by rustc \ @@ -133,6 +138,22 @@ impl<'a> DiagnosticPrinter<'a> { writeln!(self.config.shell().err())?; } write!(self.config.shell().err(), "{}", PLEASE_REPORT_THIS_BUG)?; + if !errors.is_empty() { + writeln!( + self.config.shell().err(), + "The following errors were reported:" + )?; + for error in errors { + write!(self.config.shell().err(), "{}", error)?; + if !error.ends_with('\n') { + writeln!(self.config.shell().err())?; + } + } + } + writeln!( + self.config.shell().err(), + "Original diagnostics will follow.\n" + )?; Ok(()) } Message::EditionAlreadyEnabled { file, edition } => { diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index d31693a6c6f..24a68e26efc 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -54,6 +54,19 @@ fn fix_broken_if_requested() { #[test] fn broken_fixes_backed_out() { + // This works as follows: + // - Create a `rustc` shim (the "foo" project) which will pretend that the + // verification step fails. + // - There is an empty build script so `foo` has `OUT_DIR` to track the steps. + // - The first "check", `foo` creates a file in OUT_DIR, and it completes + // successfully with a warning diagnostic to remove unused `mut`. + // - rustfix removes the `mut`. + // - The second "check" to verify the changes, `foo` swaps out the content + // with something that fails to compile. It creates a second file so it + // won't do anything in the third check. + // - cargo fix discovers that the fix failed, and it backs out the changes. + // - The third "check" is done to display the original diagnostics of the + // original code. let p = project() .file( "foo/Cargo.toml", @@ -74,19 +87,19 @@ fn broken_fixes_backed_out() { use std::process::{self, Command}; fn main() { + // Ignore calls to things like --print=file-names and compiling build.rs. let is_lib_rs = env::args_os() .map(PathBuf::from) .any(|l| l == Path::new("src/lib.rs")); if is_lib_rs { let path = PathBuf::from(env::var_os("OUT_DIR").unwrap()); - let path = path.join("foo"); - if path.exists() { - fs::File::create("src/lib.rs") - .unwrap() - .write_all(b"not rust code") - .unwrap(); + let first = path.join("first"); + let second = path.join("second"); + if first.exists() && !second.exists() { + fs::write("src/lib.rs", b"not rust code").unwrap(); + fs::File::create(&second).unwrap(); } else { - fs::File::create(&path).unwrap(); + fs::File::create(&first).unwrap(); } } @@ -127,8 +140,6 @@ fn broken_fixes_backed_out() { .cwd(p.root().join("bar")) .env("__CARGO_FIX_YOLO", "1") .env("RUSTC", p.root().join("foo/target/debug/foo")) - .with_status(101) - .with_stderr_contains("[..]not rust code[..]") .with_stderr_contains( "\ warning: failed to automatically apply fixes suggested by rustc \ @@ -144,11 +155,19 @@ fn broken_fixes_backed_out() { 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!\ + quoting the full output of this command we'd be very appreciative!\n\ + \n\ + The following errors were reported:\n\ + error: expected one of `!` or `::`, found `rust`\n\ ", ) + .with_stderr_contains("Original diagnostics will follow.") + .with_stderr_contains("[WARNING] variable does not need to be mutable") .with_stderr_does_not_contain("[..][FIXING][..]") .run(); + + // Make sure the fix which should have been applied was backed out + assert!(p.read_file("bar/src/lib.rs").contains("let mut x = 3;")); } #[test] From 502ab6505cd4c18c8fd97011949c87b8b6a31bd3 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 13 Dec 2018 14:01:44 -0800 Subject: [PATCH 2/2] Update rustfix to 0.4.4. --- Cargo.toml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 478f51f54fe..354c5be89ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ log = "0.4.6" libgit2-sys = "0.7.9" num_cpus = "1.0" opener = "0.3.0" -rustfix = "0.4.3" +rustfix = "0.4.4" same-file = "1" semver = { version = "0.9.0", features = ["serde"] } serde = { version = "1.0.82", features = ['derive'] } @@ -108,6 +108,3 @@ doc = false deny-warnings = [] vendored-openssl = ['openssl/vendored'] pretty-env-logger = ['pretty_env_logger'] - -[patch.crates-io] -rustfix = {git="https://github.com/ehuss/rustfix", branch="pub-rendered"}