Unrolled build for #140622

Rollup merge of #140622 - petrochenkov:annusexp, r=jieyouxu

compiletest: Improve diagnostics for line annotation mismatches

When some line annotations are missing or misplaced, compiletest reports an error, but the error is not very convenient.
This PR attempts to improve the user experience.

- The "expected ... not found" messages are no longer duplicated.
- The `proc_res.status` and `proc_res.cmdline` message is no longer put in the middle of other messages describing the annotation mismatches, it's now put into the end.
- Compiletest now makes suggestions if there are fuzzy matches between expected and actually reported errors (e.g. the annotation is put on a wrong line).
- Missing diagnostic kinds are no longer produce an error eagerly, but instead treated as always mismatching kinds, so they can produce suggestions telling the right kind.

I'll post screenshots in the thread below, but the behavior shown on the screenshots can be reproduced locally using the new test `tests/ui/compiletest-self-test/line-annotation-mismatches.rs`.

This also fixes #140940.

r? ``@jieyouxu``

Author: rust-timer

src/tools/compiletest/src/errors.rs

@@ -16,6 +16,8 @@ pub enum ErrorKind {
     Suggestion,
     Warning,
     Raw,
+    /// Used for better recovery and diagnostics in compiletest.
+    Unknown,
 }
 
 impl ErrorKind {
@@ -31,21 +33,25 @@ 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,
             "RAW" => ErrorKind::Raw,
-            _ => 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`"
-            ),
-        }
+                 `ERROR`, `WARN`, `NOTE`, `HELP`, `SUGGESTION` or `RAW`"
+            )
+        })
     }
 }
 
@@ -58,13 +64,15 @@ impl fmt::Display for ErrorKind {
             ErrorKind::Suggestion => write!(f, "SUGGESTION"),
             ErrorKind::Warning => write!(f, "WARN"),
             ErrorKind::Raw => write!(f, "RAW"),
+            ErrorKind::Unknown => write!(f, "UNKNOWN"),
         }
     }
 }
 
 #[derive(Debug)]
 pub struct Error {
     pub line_num: Option<usize>,
+    pub column_num: Option<usize>,
     /// What kind of message we expect (e.g., warning, error, suggestion).
     pub kind: ErrorKind,
     pub msg: String,
@@ -74,17 +82,6 @@ pub struct Error {
     pub require_annotation: bool,
 }
 
-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())
-    }
-}
-
 /// Looks for either "//~| KIND MESSAGE" or "//~^^... KIND MESSAGE"
 /// The former is a "follow" that inherits its target from the preceding line;
 /// the latter is an "adjusts" that goes that many lines up.
@@ -168,8 +165,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"];
@@ -182,6 +181,7 @@ fn parse_expected(
     } else {
         (false, Some(line_num - line_num_adjust.len()))
     };
+    let column_num = Some(tag.start() + 1);
 
     debug!(
         "line={:?} tag={:?} follow_prev={:?} kind={:?} msg={:?}",
@@ -191,7 +191,7 @@ fn parse_expected(
         kind,
         msg
     );
-    Some((follow_prev, Error { line_num, kind, msg, require_annotation: true }))
+    Some((follow_prev, Error { line_num, column_num, kind, msg, require_annotation: true }))
 }
 
 #[cfg(test)]

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/json.rs

@@ -36,9 +36,7 @@ struct UnusedExternNotification {
 struct DiagnosticSpan {
     file_name: String,
     line_start: usize,
-    line_end: usize,
     column_start: usize,
-    column_end: usize,
     is_primary: bool,
     label: Option<String>,
     suggested_replacement: Option<String>,
@@ -148,6 +146,7 @@ pub fn parse_output(file_name: &str, output: &str) -> Vec<Error> {
             Ok(diagnostic) => push_actual_errors(&mut errors, &diagnostic, &[], file_name),
             Err(_) => errors.push(Error {
                 line_num: None,
+                column_num: None,
                 kind: ErrorKind::Raw,
                 msg: line.to_string(),
                 require_annotation: false,
@@ -193,25 +192,9 @@ fn push_actual_errors(
     // also ensure that `//~ ERROR E123` *always* works. The
     // assumption is that these multi-line error messages are on their
     // way out anyhow.
-    let with_code = |span: Option<&DiagnosticSpan>, text: &str| {
-        // FIXME(#33000) -- it'd be better to use a dedicated
-        // UI harness than to include the line/col number like
-        // this, but some current tests rely on it.
-        //
-        // Note: Do NOT include the filename. These can easily
-        // cause false matches where the expected message
-        // appears in the filename, and hence the message
-        // changes but the test still passes.
-        let span_str = match span {
-            Some(DiagnosticSpan { line_start, column_start, line_end, column_end, .. }) => {
-                format!("{line_start}:{column_start}: {line_end}:{column_end}")
-            }
-            None => format!("?:?: ?:?"),
-        };
-        match &diagnostic.code {
-            Some(code) => format!("{span_str}: {text} [{}]", code.code),
-            None => format!("{span_str}: {text}"),
-        }
+    let with_code = |text| match &diagnostic.code {
+        Some(code) => format!("{text} [{}]", code.code),
+        None => format!("{text}"),
     };
 
     // Convert multi-line messages into multiple errors.
@@ -225,17 +208,19 @@ fn push_actual_errors(
             || Regex::new(r"aborting due to \d+ previous errors?|\d+ warnings? emitted").unwrap();
         errors.push(Error {
             line_num: None,
+            column_num: None,
             kind,
-            msg: with_code(None, first_line),
+            msg: with_code(first_line),
             require_annotation: diagnostic.level != "failure-note"
                 && !RE.get_or_init(re_init).is_match(first_line),
         });
     } else {
         for span in primary_spans {
             errors.push(Error {
                 line_num: Some(span.line_start),
+                column_num: Some(span.column_start),
                 kind,
-                msg: with_code(Some(span), first_line),
+                msg: with_code(first_line),
                 require_annotation: true,
             });
         }
@@ -244,16 +229,18 @@ fn push_actual_errors(
         if primary_spans.is_empty() {
             errors.push(Error {
                 line_num: None,
+                column_num: None,
                 kind,
-                msg: with_code(None, next_line),
+                msg: with_code(next_line),
                 require_annotation: false,
             });
         } else {
             for span in primary_spans {
                 errors.push(Error {
                     line_num: Some(span.line_start),
+                    column_num: Some(span.column_start),
                     kind,
-                    msg: with_code(Some(span), next_line),
+                    msg: with_code(next_line),
                     require_annotation: false,
                 });
             }
@@ -266,6 +253,7 @@ fn push_actual_errors(
             for (index, line) in suggested_replacement.lines().enumerate() {
                 errors.push(Error {
                     line_num: Some(span.line_start + index),
+                    column_num: Some(span.column_start),
                     kind: ErrorKind::Suggestion,
                     msg: line.to_string(),
                     // Empty suggestions (suggestions to remove something) are common
@@ -288,6 +276,7 @@ fn push_actual_errors(
         if let Some(label) = &span.label {
             errors.push(Error {
                 line_num: Some(span.line_start),
+                column_num: Some(span.column_start),
                 kind: ErrorKind::Note,
                 msg: label.clone(),
                 // Empty labels (only underlining spans) are common and do not need annotations.
@@ -310,6 +299,7 @@ fn push_backtrace(
     if Path::new(&expansion.span.file_name) == Path::new(&file_name) {
         errors.push(Error {
             line_num: Some(expansion.span.line_start),
+            column_num: Some(expansion.span.column_start),
             kind: ErrorKind::Note,
             msg: format!("in this expansion of {}", expansion.macro_decl_name),
             require_annotation: true,