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

Inconsistent summary on error count with --message-format=json #14472

Closed
zjp-CN opened this issue Aug 31, 2024 · 2 comments · Fixed by #14598
Closed

Inconsistent summary on error count with --message-format=json #14472

zjp-CN opened this issue Aug 31, 2024 · 2 comments · Fixed by #14598
Assignees
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. A-json-output Area: JSON message output C-bug Category: bug E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@zjp-CN
Copy link

zjp-CN commented Aug 31, 2024

Problem

cargo check emits a good summary on error count, but with --message-format json, the count number is increased by 1 because it seems to include the error summary itself as a new error. This also influences summary count of clippy in the same way.

As for warning count, the former reports the count, but with json format, no count is reported.

Steps

Repoducible repo: https://github.com/os-checker/MRE-cargo-error-count

// src/main.rs
#![deny(warnings)]
fn main() {
    let a = 1;
}
$ cargo check
error: could not compile `cargo-error-count` (bin "cargo-error-count") due to 1 previous error

$ cargo check --message-format json
error: could not compile `cargo-error-count` (bin "cargo-error-count") due to 2 previous errors

Note: this inconsistency might be a bit different on warning report

# with no #![deny(warnings)]
$ cargo check
warning: `cargo-error-count` (bin "cargo-error-count") generated 1 warning

# with no #![deny(warnings)]
$ cargo check --message-format json
# no count reported

Possible Solution(s)

No response

Notes

No response

Version

cargo 1.82.0-nightly (0d8d22f83 2024-08-08)
release: 1.82.0-nightly
commit-hash: 0d8d22f83b066503f6b2b755925197e959e58b4f
commit-date: 2024-08-08
host: x86_64-unknown-linux-gnu
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.9.0-DEV (sys:0.4.74+curl-8.9.0 vendored ssl:OpenSSL/3.3.1)
ssl: OpenSSL 3.3.1 4 Jun 2024
os: Ubuntu 22.4.0 (jammy) [64-bit]
@zjp-CN zjp-CN added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Aug 31, 2024
@weihanglo
Copy link
Member

The root cause is that Cargo skips counting when seeing these messages:

if msg.message.starts_with("aborting due to")
|| msg.message.ends_with("warning emitted")
|| msg.message.ends_with("warnings emitted")
{

However, it doesn't do that here:

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

@Muscraft had made a workable fix during RustConf last week. Basically duplicating the skipping logic. That seems to be an acceptable hack.

@weihanglo weihanglo added A-diagnostics Area: Error and warning messages generated by Cargo itself. S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review A-json-output Area: JSON message output E-easy Experience: Easy and removed S-triage Status: This issue is waiting on initial triage. labels Sep 17, 2024
@yichi170
Copy link
Contributor

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. A-json-output Area: JSON message output C-bug Category: bug E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants