Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify cargo and rustc's error reporting #9655

Merged
merged 2 commits into from
Jul 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/cargo/core/compiler/future_incompat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub struct FutureBreakageItem {
#[derive(Serialize, Deserialize)]
pub struct Diagnostic {
pub rendered: String,
pub level: String,
}

/// The filename in the top-level `target` directory where we store
Expand Down
47 changes: 46 additions & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,20 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
},
)
.map_err(verbose_if_simple_exit_code)
.with_context(|| format!("could not compile `{}`", name))?;
.with_context(|| {
// adapted from rustc_errors/src/lib.rs
let warnings = match output_options.warnings_seen {
0 => String::new(),
1 => "; 1 warning emitted".to_string(),
count => format!("; {} warnings emitted", count),
};
let errors = match output_options.errors_seen {
0 => String::new(),
1 => " due to previous error".to_string(),
count => format!(" due to {} previous errors", count),
};
format!("could not compile `{}`{}{}", name, errors, warnings)
})?;
}

if rustc_dep_info_loc.exists() {
Expand Down Expand Up @@ -1161,6 +1174,8 @@ struct OutputOptions {
/// Other types of messages are processed regardless
/// of the value of this flag
show_warnings: bool,
warnings_seen: usize,
errors_seen: usize,
}

impl OutputOptions {
Expand All @@ -1177,6 +1192,8 @@ impl OutputOptions {
color,
cache_cell,
show_warnings: true,
warnings_seen: 0,
errors_seen: 0,
}
}
}
Expand Down Expand Up @@ -1244,7 +1261,18 @@ fn on_stderr_line_inner(
}
};

let count_diagnostic = |level, options: &mut OutputOptions| {
if level == "warning" {
options.warnings_seen += 1;
} else if level == "error" {
options.errors_seen += 1;
}
};

if let Ok(report) = serde_json::from_str::<FutureIncompatReport>(compiler_message.get()) {
for item in &report.future_incompat_report {
count_diagnostic(&*item.diagnostic.level, options);
}
state.future_incompat_report(report.future_incompat_report);
return Ok(true);
}
Expand All @@ -1265,8 +1293,14 @@ fn on_stderr_line_inner(
#[derive(serde::Deserialize)]
struct CompilerMessage {
rendered: String,
message: String,
level: String,
}
if let Ok(mut error) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
if error.level == "error" && error.message.starts_with("aborting due to") {
// Skip this line; we'll print our own summary at the end.
return Ok(true);
}
// state.stderr will add a newline
if error.rendered.ends_with('\n') {
error.rendered.pop();
Expand All @@ -1281,6 +1315,7 @@ fn on_stderr_line_inner(
.expect("strip should never fail")
};
if options.show_warnings {
count_diagnostic(&error.level, options);
state.stderr(rendered)?;
}
return Ok(true);
Expand Down Expand Up @@ -1368,6 +1403,14 @@ fn on_stderr_line_inner(
return Ok(true);
}

#[derive(serde::Deserialize)]
struct CompilerMessage {
level: String,
}
if let Ok(message) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
count_diagnostic(&message.level, options);
}

let msg = machine_message::FromCompiler {
package_id,
manifest_path,
Expand Down Expand Up @@ -1399,6 +1442,8 @@ fn replay_output_cache(
color,
cache_cell: None,
show_warnings,
warnings_seen: 0,
errors_seen: 0,
};
Work::new(move |state| {
if !path.exists() {
Expand Down
8 changes: 1 addition & 7 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,7 @@ pub fn exit_with_error(err: CliError, shell: &mut Shell) -> ! {
/// Displays an error, and all its causes, to stderr.
pub fn display_error(err: &Error, shell: &mut Shell) {
debug!("display_error; err={:?}", err);
let has_verbose = _display_error(err, shell, true);
if has_verbose {
drop(writeln!(
shell.err(),
"\nTo learn more, run the command again with --verbose."
));
}
_display_error(err, shell, true);
if err
.chain()
.any(|e| e.downcast_ref::<InternalError>().is_some())
Expand Down
7 changes: 1 addition & 6 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,12 +588,7 @@ fn cargo_compile_with_invalid_code() {

p.cargo("build")
.with_status(101)
.with_stderr_contains(
"\
[ERROR] could not compile `foo`

To learn more, run the command again with --verbose.\n",
)
.with_stderr_contains("[ERROR] could not compile `foo` due to previous error\n")
.run();
assert!(p.root().join("Cargo.lock").is_file());
}
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1458,7 +1458,7 @@ fn build_deps_not_for_normal() {
.with_stderr_contains("[..]can't find crate for `aaaaa`[..]")
.with_stderr_contains(
"\
[ERROR] could not compile `foo`
[ERROR] could not compile `foo` due to previous error

Caused by:
process didn't exit successfully: [..]
Expand Down
3 changes: 1 addition & 2 deletions tests/testsuite/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,8 +803,7 @@ fn short_message_format() {
.with_stderr_contains(
"\
src/lib.rs:1:27: error[E0308]: mismatched types
error: aborting due to previous error
error: could not compile `foo`
error: could not compile `foo` due to previous error
",
)
.run();
Expand Down
4 changes: 1 addition & 3 deletions tests/testsuite/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn do_not_fix_broken_builds() {
p.cargo("fix --allow-no-vcs")
.env("__CARGO_FIX_YOLO", "1")
.with_status(101)
.with_stderr_contains("[ERROR] could not compile `foo`")
.with_stderr_contains("[ERROR] could not compile `foo` due to previous error")
.run();
assert!(p.read_file("src/lib.rs").contains("let mut x = 3;"));
}
Expand Down Expand Up @@ -833,8 +833,6 @@ fn prepare_for_unstable() {
[ERROR] cannot migrate src/lib.rs to edition {next}
Edition {next} is unstable and not allowed in this release, consider trying the nightly release channel.
error: could not compile `foo`

To learn more, run the command again with --verbose.
", next=next))
.run();

Expand Down
4 changes: 1 addition & 3 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,9 +742,7 @@ fn compile_failure() {
found at `[..]target`

Caused by:
could not compile `foo`

To learn more, run the command again with --verbose.
could not compile `foo` due to previous error
",
)
.run();
Expand Down