compiletest: Make diagnostic kind mandatory on line annotations

Author: petrochenkov

src/tools/compiletest/src/errors.rs

@@ -30,24 +30,18 @@ impl ErrorKind {
 
     /// 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 {
+    pub fn from_user_str(s: &str) -> ErrorKind {
+        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,
-        })
-    }
-
-    pub fn expect_from_user_str(s: &str) -> ErrorKind {
-        ErrorKind::from_user_str(s).unwrap_or_else(|| {
-            panic!(
+            _ => panic!(
                 "unexpected diagnostic kind `{s}`, expected \
                  `ERROR`, `WARN`, `NOTE`, `HELP` or `SUGGESTION`"
-            )
-        })
+            ),
+        }
     }
 }
 
@@ -67,8 +61,7 @@ impl fmt::Display for ErrorKind {
 pub struct Error {
     pub line_num: Option<usize>,
     /// What kind of message we expect (e.g., warning, error, suggestion).
-    /// `None` if not specified or unknown message kind.
-    pub kind: Option<ErrorKind>,
+    pub kind: 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.
@@ -79,12 +72,7 @@ pub struct Error {
 impl Error {
     pub fn render_for_expected(&self) -> String {
         use colored::Colorize;
-        format!(
-            "{: <10}line {: >3}: {}",
-            self.kind.map(|kind| kind.to_string()).unwrap_or_default(),
-            self.line_num_str(),
-            self.msg.cyan(),
-        )
+        format!("{: <10}line {: >3}: {}", self.kind, self.line_num_str(), self.msg.cyan())
     }
 
     pub fn line_num_str(&self) -> String {
@@ -175,7 +163,7 @@ fn parse_expected(
     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 untrimmed_msg = &rest[kind_str.len()..];
     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

@@ -594,7 +594,7 @@ impl TestProps {
                         config.parse_name_value_directive(ln, DONT_REQUIRE_ANNOTATIONS)
                     {
                         self.require_annotations
-                            .insert(ErrorKind::expect_from_user_str(&err_kind), false);
+                            .insert(ErrorKind::from_user_str(err_kind.trim()), false);
                     }
                 },
             );

src/tools/compiletest/src/json.rs

@@ -229,7 +229,7 @@ fn push_actual_errors(
     // Convert multi-line messages into multiple errors.
     // We expect to replace these with something more structured anyhow.
     let mut message_lines = diagnostic.message.lines();
-    let kind = Some(ErrorKind::from_compiler_str(&diagnostic.level));
+    let kind = 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();
@@ -278,7 +278,7 @@ fn push_actual_errors(
             for (index, line) in suggested_replacement.lines().enumerate() {
                 errors.push(Error {
                     line_num: Some(span.line_start + index),
-                    kind: Some(ErrorKind::Suggestion),
+                    kind: ErrorKind::Suggestion,
                     msg: line.to_string(),
                     require_annotation: true,
                 });
@@ -297,7 +297,7 @@ fn push_actual_errors(
     for span in spans_in_this_file.iter().filter(|span| span.label.is_some()) {
         errors.push(Error {
             line_num: Some(span.line_start),
-            kind: Some(ErrorKind::Note),
+            kind: ErrorKind::Note,
             msg: span.label.clone().unwrap(),
             require_annotation: true,
         });
@@ -317,7 +317,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),
-            kind: Some(ErrorKind::Note),
+            kind: ErrorKind::Note,
             msg: format!("in this expansion of {}", expansion.macro_decl_name),
             require_annotation: true,
         });

src/tools/compiletest/src/runtest.rs

@@ -680,9 +680,7 @@ impl<'test> TestCx<'test> {
             "check_expected_errors: expected_errors={:?} proc_res.status={:?}",
             expected_errors, proc_res.status
         );
-        if proc_res.status.success()
-            && expected_errors.iter().any(|x| x.kind == Some(ErrorKind::Error))
-        {
+        if proc_res.status.success() && expected_errors.iter().any(|x| x.kind == ErrorKind::Error) {
             self.fatal_proc_rec("process did not return an error status", proc_res);
         }
 
@@ -710,8 +708,8 @@ impl<'test> TestCx<'test> {
             self.testpaths.file.display().to_string()
         };
 
-        let expect_help = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Help));
-        let expect_note = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Note));
+        let expect_help = expected_errors.iter().any(|ee| ee.kind == ErrorKind::Help);
+        let expect_note = expected_errors.iter().any(|ee| ee.kind == ErrorKind::Note);
 
         // 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);
@@ -724,8 +722,7 @@ impl<'test> TestCx<'test> {
                 expected_errors.iter().enumerate().position(|(index, expected_error)| {
                     !found[index]
                         && actual_error.line_num == expected_error.line_num
-                        && (expected_error.kind.is_none()
-                            || actual_error.kind == expected_error.kind)
+                        && actual_error.kind == expected_error.kind
                         && actual_error.msg.contains(&expected_error.msg)
                 });
 
@@ -744,10 +741,7 @@ impl<'test> TestCx<'test> {
                             "{}:{}: unexpected {}: '{}'",
                             file_name,
                             actual_error.line_num_str(),
-                            actual_error
-                                .kind
-                                .as_ref()
-                                .map_or(String::from("message"), |k| k.to_string()),
+                            actual_error.kind,
                             actual_error.msg
                         ));
                         unexpected.push(actual_error);
@@ -764,7 +758,7 @@ impl<'test> TestCx<'test> {
                     "{}:{}: expected {} not found: {}",
                     file_name,
                     expected_error.line_num_str(),
-                    expected_error.kind.as_ref().map_or("message".into(), |k| k.to_string()),
+                    expected_error.kind,
                     expected_error.msg
                 ));
                 not_found.push(expected_error);
@@ -804,17 +798,15 @@ impl<'test> TestCx<'test> {
         expect_help: bool,
         expect_note: bool,
     ) -> bool {
+        // If the test being checked doesn't contain any "help" or "note" annotations, then
+        // we don't require annotating "help" or "note" (respecively) diagnostics at all.
         actual_error.require_annotation
-            && actual_error.kind.map_or(false, |err_kind| {
-                // If the test being checked doesn't contain any "help" or "note" annotations, then
-                // we don't require annotating "help" or "note" (respecively) diagnostics at all.
-                let default_require_annotations = self.props.require_annotations[&err_kind];
-                match err_kind {
-                    ErrorKind::Help => expect_help && default_require_annotations,
-                    ErrorKind::Note => expect_note && default_require_annotations,
-                    _ => default_require_annotations,
-                }
-            })
+            && self.props.require_annotations[&actual_error.kind]
+            && match actual_error.kind {
+                ErrorKind::Help => expect_help,
+                ErrorKind::Note => expect_note,
+                _ => true,
+            }
     }
 
     fn should_emit_metadata(&self, pm: Option<PassMode>) -> Emit {

src/tools/compiletest/src/runtest/ui.rs

@@ -6,8 +6,8 @@ use rustfix::{Filter, apply_suggestions, get_suggestions_from_json};
 use tracing::debug;
 
 use super::{
-    AllowUnused, Emit, ErrorKind, FailMode, LinkToAux, PassMode, TargetLocation, TestCx,
-    TestOutput, Truncated, UI_FIXED, WillExecute,
+    AllowUnused, Emit, FailMode, LinkToAux, PassMode, TargetLocation, TestCx, TestOutput,
+    Truncated, UI_FIXED, WillExecute,
 };
 use crate::{errors, json};
 
@@ -176,7 +176,7 @@ impl TestCx<'_> {
             let msg = format!(
                 "line {}: cannot combine `--error-format` with {} annotations; use `error-pattern` instead",
                 expected_errors[0].line_num_str(),
-                expected_errors[0].kind.unwrap_or(ErrorKind::Error),
+                expected_errors[0].kind,
             );
             self.fatal(&msg);
         }

tests/rustdoc-ui/issues/ice-generic-type-alias-105742.rs

@@ -1,21 +1,23 @@
 //@ compile-flags: -Znormalize-docs
+//@ dont-require-annotations: NOTE
+
 // https://github.com/rust-lang/rust/issues/105742
 use std::ops::Index;
 
 pub fn next<'a, T>(s: &'a mut dyn SVec<Item = T, Output = T>) {
-    //~^ expected 1 lifetime argument
-    //~| expected 1 generic argument
+    //~^ NOTE expected 1 lifetime argument
+    //~| NOTE expected 1 generic argument
     //~| ERROR the trait `SVec` is not dyn compatible
-    //~| `SVec` is not dyn compatible
+    //~| NOTE `SVec` is not dyn compatible
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     let _ = s;
 }
 
 pub trait SVec: Index<
     <Self as SVec>::Item,
-    //~^ expected 1 lifetime argument
-    //~| expected 1 generic argument
+    //~^ NOTE expected 1 lifetime argument
+    //~| NOTE expected 1 generic argument
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
@@ -25,8 +27,8 @@ pub trait SVec: Index<
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     Output = <Index<<Self as SVec>::Item,
-    //~^ expected 1 lifetime argument
-    //~| expected 1 generic argument
+    //~^ NOTE expected 1 lifetime argument
+    //~| NOTE expected 1 generic argument
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
@@ -36,16 +38,16 @@ pub trait SVec: Index<
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     Output = <Self as SVec>::Item> as SVec>::Item,
-    //~^ expected 1 lifetime argument
-    //~| expected 1 generic argument
-    //~| expected 1 lifetime argument
+    //~^ NOTE expected 1 lifetime argument
+    //~| NOTE expected 1 generic argument
+    //~| NOTE expected 1 lifetime argument
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
-    //~| expected 1 generic argument
+    //~| NOTE expected 1 generic argument
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
     //~| ERROR missing generics for associated type `SVec::Item`
@@ -60,8 +62,8 @@ pub trait SVec: Index<
     type Item<'a, T>;
 
     fn len(&self) -> <Self as SVec>::Item;
-    //~^ expected 1 lifetime argument
+    //~^ NOTE expected 1 lifetime argument
     //~| ERROR missing generics for associated type `SVec::Item`
-    //~| expected 1 generic argument
+    //~| NOTE expected 1 generic argument
     //~| ERROR missing generics for associated type `SVec::Item`
 }