compiletest: Improve diagnostics for line annotation mismatches

Author: petrochenkov

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<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`"
-            ),
-        }
+            )
+        })
     }
 }
 
@@ -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"];

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()));
                     }
                 },
             );

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 &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
+            );
         }
     }
 
@@ -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`"

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
+}

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`.