Rollup merge of #139485 - petrochenkov:errkind-light, r=oli-obk,jieyouxu

compiletest: Stricter parsing for diagnostic kinds

Non-controversial parts of https://github.com/rust-lang/rust/pull/139427 not requiring many changes in the test suite.

r? ``@jieyouxu``

src/tools/compiletest/src/errors.rs

@@ -3,7 +3,6 @@ use std::fs::File;
 use std::io::BufReader;
 use std::io::prelude::*;
 use std::path::Path;
-use std::str::FromStr;
 use std::sync::OnceLock;
 
 use regex::Regex;
@@ -18,30 +17,39 @@ pub enum ErrorKind {
     Warning,
 }
 
-impl FromStr for ErrorKind {
-    type Err = ();
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let s = s.to_uppercase();
-        let part0: &str = s.split(':').next().unwrap();
-        match part0 {
-            "HELP" => Ok(ErrorKind::Help),
-            "ERROR" => Ok(ErrorKind::Error),
-            "NOTE" => Ok(ErrorKind::Note),
-            "SUGGESTION" => Ok(ErrorKind::Suggestion),
-            "WARN" | "WARNING" => Ok(ErrorKind::Warning),
-            _ => Err(()),
+impl ErrorKind {
+    pub fn from_compiler_str(s: &str) -> ErrorKind {
+        match s {
+            "help" => ErrorKind::Help,
+            "error" | "error: internal compiler error" => ErrorKind::Error,
+            "note" | "failure-note" => ErrorKind::Note,
+            "warning" => ErrorKind::Warning,
+            _ => panic!("unexpected compiler diagnostic kind `{s}`"),
         }
     }
+
+    /// Either the canonical uppercase string, or some additional versions for compatibility.
+    /// FIXME: consider keeping only the canonical versions here.
+    fn from_user_str(s: &str) -> Option<ErrorKind> {
+        Some(match s {
+            "HELP" | "help" => ErrorKind::Help,
+            "ERROR" | "error" => ErrorKind::Error,
+            "NOTE" | "note" => ErrorKind::Note,
+            "SUGGESTION" => ErrorKind::Suggestion,
+            "WARN" | "WARNING" | "warn" | "warning" => ErrorKind::Warning,
+            _ => return None,
+        })
+    }
 }
 
 impl fmt::Display for ErrorKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
-            ErrorKind::Help => write!(f, "help message"),
-            ErrorKind::Error => write!(f, "error"),
-            ErrorKind::Note => write!(f, "note"),
-            ErrorKind::Suggestion => write!(f, "suggestion"),
-            ErrorKind::Warning => write!(f, "warning"),
+            ErrorKind::Help => write!(f, "HELP"),
+            ErrorKind::Error => write!(f, "ERROR"),
+            ErrorKind::Note => write!(f, "NOTE"),
+            ErrorKind::Suggestion => write!(f, "SUGGESTION"),
+            ErrorKind::Warning => write!(f, "WARN"),
         }
     }
 }
@@ -53,6 +61,10 @@ pub struct Error {
     /// `None` if not specified or unknown message kind.
     pub kind: Option<ErrorKind>,
     pub msg: String,
+    /// For some `Error`s, like secondary lines of multi-line diagnostics, line annotations
+    /// are not mandatory, even if they would otherwise be mandatory for primary errors.
+    /// Only makes sense for "actual" errors, not for "expected" errors.
+    pub require_annotation: bool,
 }
 
 impl Error {
@@ -60,7 +72,7 @@ impl Error {
         use colored::Colorize;
         format!(
             "{: <10}line {: >3}: {}",
-            self.kind.map(|kind| kind.to_string()).unwrap_or_default().to_uppercase(),
+            self.kind.map(|kind| kind.to_string()).unwrap_or_default(),
             self.line_num_str(),
             self.msg.cyan(),
         )
@@ -150,18 +162,12 @@ fn parse_expected(
     }
 
     // Get the part of the comment after the sigil (e.g. `~^^` or ~|).
-    let whole_match = captures.get(0).unwrap();
-    let (_, mut msg) = line.split_at(whole_match.end());
-
-    let first_word = msg.split_whitespace().next().expect("Encountered unexpected empty comment");
-
-    // If we find `//~ ERROR foo` or something like that, skip the first word.
-    let kind = first_word.parse::<ErrorKind>().ok();
-    if kind.is_some() {
-        msg = &msg.trim_start().split_at(first_word.len()).1;
-    }
-
-    let msg = msg.trim().to_owned();
+    let tag = captures.get(0).unwrap();
+    let rest = line[tag.end()..].trim_start();
+    let (kind_str, _) = rest.split_once(|c: char| !c.is_ascii_alphabetic()).unwrap_or((rest, ""));
+    let kind = ErrorKind::from_user_str(kind_str);
+    let untrimmed_msg = if kind.is_some() { &rest[kind_str.len()..] } else { rest };
+    let msg = untrimmed_msg.strip_prefix(':').unwrap_or(untrimmed_msg).trim().to_owned();
 
     let line_num_adjust = &captures["adjust"];
     let (follow_prev, line_num) = if line_num_adjust == "|" {
@@ -177,12 +183,12 @@ fn parse_expected(
     debug!(
         "line={:?} tag={:?} follow_prev={:?} kind={:?} msg={:?}",
         line_num,
-        whole_match.as_str(),
+        tag.as_str(),
         follow_prev,
         kind,
         msg
     );
-    Some((follow_prev, Error { line_num, kind, msg }))
+    Some((follow_prev, Error { line_num, kind, msg, require_annotation: true }))
 }
 
 #[cfg(test)]

src/tools/compiletest/src/json.rs

@@ -1,7 +1,6 @@
 //! These structs are a subset of the ones found in `rustc_errors::json`.
 
 use std::path::{Path, PathBuf};
-use std::str::FromStr;
 use std::sync::OnceLock;
 
 use regex::Regex;
@@ -142,43 +141,34 @@ pub fn extract_rendered(output: &str) -> String {
 }
 
 pub fn parse_output(file_name: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
-    output.lines().flat_map(|line| parse_line(file_name, line, output, proc_res)).collect()
-}
-
-fn parse_line(file_name: &str, line: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
-    // The compiler sometimes intermingles non-JSON stuff into the
-    // output.  This hack just skips over such lines. Yuck.
-    if line.starts_with('{') {
-        match serde_json::from_str::<Diagnostic>(line) {
-            Ok(diagnostic) => {
-                let mut expected_errors = vec![];
-                push_expected_errors(&mut expected_errors, &diagnostic, &[], file_name);
-                expected_errors
-            }
-            Err(error) => {
-                // Ignore the future compat report message - this is handled
-                // by `extract_rendered`
-                if serde_json::from_str::<FutureIncompatReport>(line).is_ok() {
-                    vec![]
-                } else {
-                    proc_res.fatal(
+    let mut errors = Vec::new();
+    for line in output.lines() {
+        // The compiler sometimes intermingles non-JSON stuff into the
+        // output.  This hack just skips over such lines. Yuck.
+        if line.starts_with('{') {
+            match serde_json::from_str::<Diagnostic>(line) {
+                Ok(diagnostic) => push_actual_errors(&mut errors, &diagnostic, &[], file_name),
+                Err(error) => {
+                    // Ignore the future compat report message - this is handled
+                    // by `extract_rendered`
+                    if serde_json::from_str::<FutureIncompatReport>(line).is_err() {
+                        proc_res.fatal(
                         Some(&format!(
-                            "failed to decode compiler output as json: \
-                         `{}`\nline: {}\noutput: {}",
+                            "failed to decode compiler output as json: `{}`\nline: {}\noutput: {}",
                             error, line, output
                         )),
                         || (),
                     );
+                    }
                 }
             }
         }
-    } else {
-        vec![]
     }
+    errors
 }
 
-fn push_expected_errors(
-    expected_errors: &mut Vec<Error>,
+fn push_actual_errors(
+    errors: &mut Vec<Error>,
     diagnostic: &Diagnostic,
     default_spans: &[&DiagnosticSpan],
     file_name: &str,
@@ -236,44 +226,47 @@ fn push_expected_errors(
         }
     };
 
-    // Convert multi-line messages into multiple expected
-    // errors. We expect to replace these with something
-    // more structured shortly anyhow.
+    // Convert multi-line messages into multiple errors.
+    // We expect to replace these with something more structured anyhow.
     let mut message_lines = diagnostic.message.lines();
-    if let Some(first_line) = message_lines.next() {
-        let ignore = |s| {
-            static RE: OnceLock<Regex> = OnceLock::new();
-            RE.get_or_init(|| {
-                Regex::new(r"aborting due to \d+ previous errors?|\d+ warnings? emitted").unwrap()
-            })
-            .is_match(s)
-        };
-
-        if primary_spans.is_empty() && !ignore(first_line) {
-            let msg = with_code(None, first_line);
-            let kind = ErrorKind::from_str(&diagnostic.level).ok();
-            expected_errors.push(Error { line_num: None, kind, msg });
-        } else {
-            for span in primary_spans {
-                let msg = with_code(Some(span), first_line);
-                let kind = ErrorKind::from_str(&diagnostic.level).ok();
-                expected_errors.push(Error { line_num: Some(span.line_start), kind, msg });
-            }
+    let kind = Some(ErrorKind::from_compiler_str(&diagnostic.level));
+    let first_line = message_lines.next().unwrap_or(&diagnostic.message);
+    if primary_spans.is_empty() {
+        static RE: OnceLock<Regex> = OnceLock::new();
+        let re_init =
+            || Regex::new(r"aborting due to \d+ previous errors?|\d+ warnings? emitted").unwrap();
+        errors.push(Error {
+            line_num: None,
+            kind,
+            msg: with_code(None, 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),
+                kind,
+                msg: with_code(Some(span), first_line),
+                require_annotation: true,
+            });
         }
     }
     for next_line in message_lines {
         if primary_spans.is_empty() {
-            expected_errors.push(Error {
+            errors.push(Error {
                 line_num: None,
-                kind: None,
+                kind,
                 msg: with_code(None, next_line),
+                require_annotation: false,
             });
         } else {
             for span in primary_spans {
-                expected_errors.push(Error {
+                errors.push(Error {
                     line_num: Some(span.line_start),
-                    kind: None,
+                    kind,
                     msg: with_code(Some(span), next_line),
+                    require_annotation: false,
                 });
             }
         }
@@ -283,10 +276,11 @@ fn push_expected_errors(
     for span in primary_spans {
         if let Some(ref suggested_replacement) = span.suggested_replacement {
             for (index, line) in suggested_replacement.lines().enumerate() {
-                expected_errors.push(Error {
+                errors.push(Error {
                     line_num: Some(span.line_start + index),
                     kind: Some(ErrorKind::Suggestion),
                     msg: line.to_string(),
+                    require_annotation: true,
                 });
             }
         }
@@ -295,39 +289,41 @@ fn push_expected_errors(
     // Add notes for the backtrace
     for span in primary_spans {
         if let Some(frame) = &span.expansion {
-            push_backtrace(expected_errors, frame, file_name);
+            push_backtrace(errors, frame, file_name);
         }
     }
 
     // Add notes for any labels that appear in the message.
     for span in spans_in_this_file.iter().filter(|span| span.label.is_some()) {
-        expected_errors.push(Error {
+        errors.push(Error {
             line_num: Some(span.line_start),
             kind: Some(ErrorKind::Note),
             msg: span.label.clone().unwrap(),
+            require_annotation: true,
         });
     }
 
     // Flatten out the children.
     for child in &diagnostic.children {
-        push_expected_errors(expected_errors, child, primary_spans, file_name);
+        push_actual_errors(errors, child, primary_spans, file_name);
     }
 }
 
 fn push_backtrace(
-    expected_errors: &mut Vec<Error>,
+    errors: &mut Vec<Error>,
     expansion: &DiagnosticSpanMacroExpansion,
     file_name: &str,
 ) {
     if Path::new(&expansion.span.file_name) == Path::new(&file_name) {
-        expected_errors.push(Error {
+        errors.push(Error {
             line_num: Some(expansion.span.line_start),
             kind: Some(ErrorKind::Note),
             msg: format!("in this expansion of {}", expansion.macro_decl_name),
+            require_annotation: true,
         });
     }
 
     if let Some(previous_expansion) = &expansion.span.expansion {
-        push_backtrace(expected_errors, previous_expansion, file_name);
+        push_backtrace(errors, previous_expansion, file_name);
     }
 }

src/tools/compiletest/src/runtest.rs

@@ -810,7 +810,7 @@ impl<'test> TestCx<'test> {
         expect_help: bool,
         expect_note: bool,
     ) -> bool {
-        !actual_error.msg.is_empty()
+        actual_error.require_annotation
             && match actual_error.kind {
                 Some(ErrorKind::Help) => expect_help,
                 Some(ErrorKind::Note) => expect_note,