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.
This commit is contained in:
Eric Huss 2018-12-11 16:24:20 -08:00
parent 79f962f8c9
commit fffb05d8fc
4 changed files with 63 additions and 13 deletions

View file

@ -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"}

View file

@ -496,7 +496,9 @@ fn log_failed_fix(stderr: &[u8]) -> Result<(), Error> {
.filter(|x| !x.is_empty())
.filter_map(|line| serde_json::from_str::<Diagnostic>(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(())
}

View file

@ -36,6 +36,7 @@ pub enum Message {
FixFailed {
files: Vec<String>,
krate: Option<String>,
errors: Vec<String>,
},
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 } => {

View file

@ -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]