From cfb9778658f74797e72bc2391e8bbae784bbdcd0 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 3 May 2025 15:53:52 +0300 Subject: [PATCH] compiletest: Improve diagnostics for line annotation mismatches --- src/tools/compiletest/src/errors.rs | 34 ++++---- src/tools/compiletest/src/header.rs | 2 +- src/tools/compiletest/src/runtest.rs | 79 +++++++++++++------ .../line-annotation-mismatches.rs | 42 ++++++++++ .../line-annotation-mismatches.stderr | 61 ++++++++++++++ 5 files changed, 179 insertions(+), 39 deletions(-) create mode 100644 tests/ui/compiletest-self-test/line-annotation-mismatches.rs create mode 100644 tests/ui/compiletest-self-test/line-annotation-mismatches.stderr diff --git a/src/tools/compiletest/src/errors.rs b/src/tools/compiletest/src/errors.rs index a45f39b036cc9..b082d4f798c66 100644 --- a/src/tools/compiletest/src/errors.rs +++ b/src/tools/compiletest/src/errors.rs @@ -15,6 +15,8 @@ pub enum ErrorKind { Note, Suggestion, Warning, + /// Used for better recovery and diagnostics in compiletest. + Unknown, } impl ErrorKind { @@ -30,20 +32,24 @@ impl ErrorKind { /// Either the canonical uppercase string, or some additional versions for compatibility. /// FIXME: consider keeping only the canonical versions here. - pub fn from_user_str(s: &str) -> ErrorKind { - match s { + fn from_user_str(s: &str) -> Option { + Some(match s { "HELP" | "help" => ErrorKind::Help, "ERROR" | "error" => ErrorKind::Error, - // `MONO_ITEM` makes annotations in `codegen-units` tests syntactically correct, - // but those tests never use the error kind later on. - "NOTE" | "note" | "MONO_ITEM" => ErrorKind::Note, + "NOTE" | "note" => ErrorKind::Note, "SUGGESTION" => ErrorKind::Suggestion, "WARN" | "WARNING" | "warn" | "warning" => ErrorKind::Warning, - _ => panic!( + _ => return None, + }) + } + + pub fn expect_from_user_str(s: &str) -> ErrorKind { + ErrorKind::from_user_str(s).unwrap_or_else(|| { + panic!( "unexpected diagnostic kind `{s}`, expected \ `ERROR`, `WARN`, `NOTE`, `HELP` or `SUGGESTION`" - ), - } + ) + }) } } @@ -55,6 +61,7 @@ impl fmt::Display for ErrorKind { ErrorKind::Note => write!(f, "NOTE"), ErrorKind::Suggestion => write!(f, "SUGGESTION"), ErrorKind::Warning => write!(f, "WARN"), + ErrorKind::Unknown => write!(f, "UNKNOWN"), } } } @@ -72,11 +79,6 @@ pub struct Error { } impl Error { - pub fn render_for_expected(&self) -> String { - use colored::Colorize; - format!("{: <10}line {: >3}: {}", self.kind, self.line_num_str(), self.msg.cyan()) - } - pub fn line_num_str(&self) -> String { self.line_num.map_or("?".to_string(), |line_num| line_num.to_string()) } @@ -165,8 +167,10 @@ fn parse_expected( let rest = line[tag.end()..].trim_start(); let (kind_str, _) = rest.split_once(|c: char| c != '_' && !c.is_ascii_alphabetic()).unwrap_or((rest, "")); - let kind = ErrorKind::from_user_str(kind_str); - let untrimmed_msg = &rest[kind_str.len()..]; + let (kind, untrimmed_msg) = match ErrorKind::from_user_str(kind_str) { + Some(kind) => (kind, &rest[kind_str.len()..]), + None => (ErrorKind::Unknown, rest), + }; let msg = untrimmed_msg.strip_prefix(':').unwrap_or(untrimmed_msg).trim().to_owned(); let line_num_adjust = &captures["adjust"]; diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 8bee9caacc949..2b203bb309c63 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -593,7 +593,7 @@ impl TestProps { config.parse_name_value_directive(ln, DONT_REQUIRE_ANNOTATIONS) { self.dont_require_annotations - .insert(ErrorKind::from_user_str(err_kind.trim())); + .insert(ErrorKind::expect_from_user_str(err_kind.trim())); } }, ); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 97cb82c9e363a..c950d0a434af6 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -713,6 +713,7 @@ impl<'test> TestCx<'test> { // Parse the JSON output from the compiler and extract out the messages. let actual_errors = json::parse_output(&diagnostic_file_name, &proc_res.stderr, proc_res); let mut unexpected = Vec::new(); + let mut unimportant = Vec::new(); let mut found = vec![false; expected_errors.len()]; for mut actual_error in actual_errors { actual_error.msg = self.normalize_output(&actual_error.msg, &[]); @@ -737,14 +738,9 @@ impl<'test> TestCx<'test> { && expected_kinds.contains(&actual_error.kind) && !self.props.dont_require_annotations.contains(&actual_error.kind) { - self.error(&format!( - "{}:{}: unexpected {}: '{}'", - file_name, - actual_error.line_num_str(), - actual_error.kind, - actual_error.msg - )); unexpected.push(actual_error); + } else { + unimportant.push(actual_error); } } } @@ -754,39 +750,77 @@ impl<'test> TestCx<'test> { // anything not yet found is a problem for (index, expected_error) in expected_errors.iter().enumerate() { if !found[index] { - self.error(&format!( - "{}:{}: expected {} not found: {}", - file_name, - expected_error.line_num_str(), - expected_error.kind, - expected_error.msg - )); not_found.push(expected_error); } } if !unexpected.is_empty() || !not_found.is_empty() { self.error(&format!( - "{} unexpected errors found, {} expected errors not found", + "{} unexpected diagnostics reported, {} expected diagnostics not reported", unexpected.len(), not_found.len() )); - println!("status: {}\ncommand: {}\n", proc_res.status, proc_res.cmdline); + let print = |e: &Error| { + println!("{file_name}:{}: {}: {}", e.line_num_str(), e.kind, e.msg.cyan()) + }; + // Fuzzy matching quality: + // - message and line / message and kind - great, suggested + // - only message - good, suggested + // - known line and kind - ok, suggested + // - only known line - meh, but suggested + // - others are not worth suggesting if !unexpected.is_empty() { - println!("{}", "--- unexpected errors (from JSON output) ---".green()); + println!("{}", "--- reported but not expected (from JSON output) ---".green()); for error in &unexpected { - println!("{}", error.render_for_expected()); + print(error); + for candidate in ¬_found { + if error.msg.contains(&candidate.msg) { + let prefix = if candidate.line_num != error.line_num { + "expected on a different line" + } else { + "expected with a different kind" + } + .red(); + print!(" {prefix}: "); + print(candidate); + } else if candidate.line_num.is_some() + && candidate.line_num == error.line_num + { + print!(" {}: ", "expected with a different message".red()); + print(candidate); + } + } } println!("{}", "---".green()); } if !not_found.is_empty() { - println!("{}", "--- not found errors (from test file) ---".red()); + println!("{}", "--- expected but not reported (from test file) ---".red()); for error in ¬_found { - println!("{}", error.render_for_expected()); + print(error); + for candidate in unexpected.iter().chain(&unimportant) { + if candidate.msg.contains(&error.msg) { + let prefix = if candidate.line_num != error.line_num { + "reported on a different line" + } else { + "reported with a different kind" + } + .green(); + print!(" {prefix}: "); + print(candidate); + } else if candidate.line_num.is_some() + && candidate.line_num == error.line_num + { + print!(" {}: ", "reported with a different message".green()); + print(candidate); + } + } } - println!("{}", "---\n".red()); + println!("{}", "---".red()); } - panic!("errors differ from expected"); + panic!( + "errors differ from expected\nstatus: {}\ncommand: {}\n", + proc_res.status, proc_res.cmdline + ); } } @@ -2069,7 +2103,6 @@ impl<'test> TestCx<'test> { println!("{}", String::from_utf8_lossy(&output.stdout)); eprintln!("{}", String::from_utf8_lossy(&output.stderr)); } else { - use colored::Colorize; eprintln!("warning: no pager configured, falling back to unified diff"); eprintln!( "help: try configuring a git pager (e.g. `delta`) with `git config --global core.pager delta`" diff --git a/tests/ui/compiletest-self-test/line-annotation-mismatches.rs b/tests/ui/compiletest-self-test/line-annotation-mismatches.rs new file mode 100644 index 0000000000000..d2a14374ed4c7 --- /dev/null +++ b/tests/ui/compiletest-self-test/line-annotation-mismatches.rs @@ -0,0 +1,42 @@ +//@ should-fail + +// The warning is reported with unknown line +//@ compile-flags: -D raw_pointer_derive +//~? WARN kind and unknown line match the reported warning, but we do not suggest it + +// The error is expected but not reported at all. +//~ ERROR this error does not exist + +// The error is reported but not expected at all. +// "`main` function not found in crate" (the main function is intentionally not added) + +// An "unimportant" diagnostic is expected on a wrong line. +//~ ERROR aborting due to + +// An "unimportant" diagnostic is expected with a wrong kind. +//~? ERROR For more information about an error + +fn wrong_line_or_kind() { + // A diagnostic expected on a wrong line. + unresolved1; + //~ ERROR cannot find value `unresolved1` in this scope + + // A diagnostic expected with a wrong kind. + unresolved2; //~ WARN cannot find value `unresolved2` in this scope + + // A diagnostic expected with a missing kind (treated as a wrong kind). + unresolved3; //~ cannot find value `unresolved3` in this scope + + // A diagnostic expected with a wrong line and kind. + unresolved4; + //~ WARN cannot find value `unresolved4` in this scope +} + +fn wrong_message() { + // A diagnostic expected with a wrong message, but the line is known and right. + unresolvedA; //~ ERROR stub message 1 + + // A diagnostic expected with a wrong message, but the line is known and right, + // even if the kind doesn't match. + unresolvedB; //~ WARN stub message 2 +} diff --git a/tests/ui/compiletest-self-test/line-annotation-mismatches.stderr b/tests/ui/compiletest-self-test/line-annotation-mismatches.stderr new file mode 100644 index 0000000000000..7ca3bfaf396c9 --- /dev/null +++ b/tests/ui/compiletest-self-test/line-annotation-mismatches.stderr @@ -0,0 +1,61 @@ +warning: lint `raw_pointer_derive` has been removed: using derive with raw pointers is ok + | + = note: requested on the command line with `-D raw_pointer_derive` + = note: `#[warn(renamed_and_removed_lints)]` on by default + +error[E0425]: cannot find value `unresolved1` in this scope + --> $DIR/line-annotation-mismatches.rs:21:5 + | +LL | unresolved1; + | ^^^^^^^^^^^ not found in this scope + +error[E0425]: cannot find value `unresolved2` in this scope + --> $DIR/line-annotation-mismatches.rs:25:5 + | +LL | unresolved2; + | ^^^^^^^^^^^ not found in this scope + +error[E0425]: cannot find value `unresolved3` in this scope + --> $DIR/line-annotation-mismatches.rs:28:5 + | +LL | unresolved3; + | ^^^^^^^^^^^ not found in this scope + +error[E0425]: cannot find value `unresolved4` in this scope + --> $DIR/line-annotation-mismatches.rs:31:5 + | +LL | unresolved4; + | ^^^^^^^^^^^ not found in this scope + +error[E0425]: cannot find value `unresolvedA` in this scope + --> $DIR/line-annotation-mismatches.rs:37:5 + | +LL | unresolvedA; + | ^^^^^^^^^^^ not found in this scope + +error[E0425]: cannot find value `unresolvedB` in this scope + --> $DIR/line-annotation-mismatches.rs:41:5 + | +LL | unresolvedB; + | ^^^^^^^^^^^ not found in this scope + +warning: lint `raw_pointer_derive` has been removed: using derive with raw pointers is ok + | + = note: requested on the command line with `-D raw_pointer_derive` + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0601]: `main` function not found in crate `line_annotation_mismatches` + --> $DIR/line-annotation-mismatches.rs:42:2 + | +LL | } + | ^ consider adding a `main` function to `$DIR/line-annotation-mismatches.rs` + +warning: lint `raw_pointer_derive` has been removed: using derive with raw pointers is ok + | + = note: requested on the command line with `-D raw_pointer_derive` + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 7 previous errors; 3 warnings emitted + +Some errors have detailed explanations: E0425, E0601. +For more information about an error, try `rustc --explain E0425`.