Skip to content

compiletest: Improve diagnostics for line annotation mismatches #140622

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
34 changes: 19 additions & 15 deletions src/tools/compiletest/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ pub enum ErrorKind {
Note,
Suggestion,
Warning,
/// Used for better recovery and diagnostics in compiletest.
Unknown,
}

impl ErrorKind {
Expand All @@ -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<ErrorKind> {
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`"
),
}
)
})
}
}

Expand All @@ -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"),
}
}
}
Expand All @@ -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())
}
Expand Down Expand Up @@ -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"];
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
},
);
Expand Down
79 changes: 56 additions & 23 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, &[]);
Expand All @@ -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);
}
}
}
Expand All @@ -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 &not_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 &not_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
);
}
}

Expand Down Expand Up @@ -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`"
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/compiletest-self-test/line-annotation-mismatches.rs
Original file line number Diff line number Diff line change
@@ -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
}
61 changes: 61 additions & 0 deletions tests/ui/compiletest-self-test/line-annotation-mismatches.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading