Auto merge of #139720 - petrochenkov:errkind2, r=jieyouxu

compiletest: Make diagnostic kind mandatory on line annotations (take 2)

Compiletest currently accepts line annotations without kind in UI tests.
```
    let a = b + c; //~ my message
```

Such annotations have two effects.
- First, they match any compiler-produced diagnostic kind. This functionality is never used in practice, there are no target-dependent diagnostic kinds of something like that.
- Second, they are not "viral". For example, any explicit `//~ NOTE my msg` in a test requires all other `NOTE` diagnostics in the same test to be annotated. Implicit `//~ my msg` will just match the note and won't require other annotations.

The second functionality has a replacement since recently - directive `//@ dont-require-annotations: NOTE`.

This PR removes support for `//~ my message` and makes the explicit diagnostic kind mandatory.
Unwanted additional annotations are suppressed using the `dont-require-annotations` directive.

Closes rust-lang/compiler-team#862.
Previous attempt - #139427.
r? `@jieyouxu`

Author: bors

src/doc/rustc-dev-guide/src/tests/ui.md

@@ -372,9 +372,9 @@ E.g. use `//@ dont-require-annotations: NOTE` to annotate notes selectively.
 Avoid using this directive for `ERROR`s and `WARN`ings, unless there's a serious reason, like
 target-dependent compiler output.
 
-Missing diagnostic kinds (`//~ message`) are currently accepted, but are being phased away.
-They will match any compiler output kind, but will not force exhaustive annotations for that kind.
-Prefer explicit kind and `//@ dont-require-annotations` to achieve the same effect.
+Some diagnostics are never required to be line-annotated, regardless of their kind or directives,
+for example secondary lines of multiline diagnostics,
+or ubiquitous diagnostics like `aborting due to N previous errors`.
 
 UI tests use the `-A unused` flag by default to ignore all unused warnings, as
 unused warnings are usually not the focus of a test. However, simple code

src/tools/compiletest/src/errors.rs

@@ -30,24 +30,20 @@ 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,
+            // `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,
             "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 +63,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 +74,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 {
@@ -173,9 +163,10 @@ fn parse_expected(
     // Get the part of the comment after the sigil (e.g. `~^^` or ~|).
     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_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 = 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

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

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

@@ -675,9 +675,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);
         }
 
@@ -709,7 +707,7 @@ impl<'test> TestCx<'test> {
         // if one of them actually occurs in the test.
         let expected_kinds: HashSet<_> = [ErrorKind::Error, ErrorKind::Warning]
             .into_iter()
-            .chain(expected_errors.iter().filter_map(|e| e.kind))
+            .chain(expected_errors.iter().map(|e| e.kind))
             .collect();
 
         // Parse the JSON output from the compiler and extract out the messages.
@@ -723,8 +721,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)
                 });
 
@@ -737,19 +734,14 @@ impl<'test> TestCx<'test> {
 
                 None => {
                     if actual_error.require_annotation
-                        && actual_error.kind.map_or(false, |kind| {
-                            expected_kinds.contains(&kind)
-                                && !self.props.dont_require_annotations.contains(&kind)
-                        })
+                        && 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
-                                .as_ref()
-                                .map_or(String::from("message"), |k| k.to_string()),
+                            actual_error.kind,
                             actual_error.msg
                         ));
                         unexpected.push(actual_error);
@@ -766,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);

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