diff --git a/Cargo.toml b/Cargo.toml index d55278c2089..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.2" +rustfix = "0.4.4" same-file = "1" semver = { version = "0.9.0", features = ["serde"] } serde = { version = "1.0.82", features = ['derive'] } diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index b020d6bc27a..8d17abe30cc 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 679d4786155..51a698024a0 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]