Auto merge of rust-lang#138865 - petrochenkov:errwhere, r=jieyouxu

compiletest: Support matching on diagnostics without a span

Using `//~? ERROR my message` on any line of the test.

The new checks are exhaustive, like all other `//~` checks, and unlike the `error-pattern` directive that is sometimes used now to check for span-less diagnostics.

This will allow to eliminate most on `error-pattern` directives in compile-fail tests (except those that are intentionally imprecise due to platform-specific diagnostics).
I didn't migrate any of `error-pattern`s in this PR though, except those where the migration was necessary for the tests to pass.

Author: bors

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

@@ -202,6 +202,9 @@ several ways to match the message with the line (see the examples below):
 * `~|`: Associates the error level and message with the *same* line as the
   *previous comment*. This is more convenient than using multiple carets when
   there are multiple messages associated with the same line.
+* `~?`: Used to match error levels and messages with errors not having line
+  information. These can be placed on any line in the test file, but are
+  conventionally placed at the end.
 
 Example:
 
@@ -270,10 +273,23 @@ fn main() {
 //~| ERROR this pattern has 1 field, but the corresponding tuple struct has 3 fields [E0023]
 ```
 
+#### Error without line information
+
+Use `//~?` to match an error without line information.
+`//~?` is precise and will not match errors if their line information is available.
+It should be preferred to using `error-pattern`, which is imprecise and non-exhaustive.
+
+```rust,ignore
+//@ compile-flags: --print yyyy
+
+//~? ERROR unknown print request: `yyyy`
+```
+
 ### `error-pattern`
 
-The `error-pattern` [directive](directives.md) can be used for messages that don't
-have a specific span.
+The `error-pattern` [directive](directives.md) can be used for runtime messages, which don't
+have a specific span, or for compile time messages if imprecise matching is required due to
+multi-line platform specific diagnostics.
 
 Let's think about this test:
 
@@ -300,7 +316,9 @@ fn main() {
 }
 ```
 
-But for strict testing, try to use the `ERROR` annotation as much as possible.
+But for strict testing, try to use the `ERROR` annotation as much as possible,
+including `//~?` annotations for diagnostics without span.
+For compile time diagnostics `error-pattern` should very rarely be necessary.
 
 ### Error levels
 
@@ -353,7 +371,7 @@ would be a `.mir.stderr` and `.thir.stderr` file with the different outputs of
 the different revisions.
 
 > Note: cfg revisions also work inside the source code with `#[cfg]` attributes.
-> 
+>
 > By convention, the `FALSE` cfg is used to have an always-false config.
 
 ## Controlling pass/fail expectations

src/tools/compiletest/src/errors.rs

@@ -9,8 +9,6 @@ use std::sync::OnceLock;
 use regex::Regex;
 use tracing::*;
 
-use self::WhichLine::*;
-
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub enum ErrorKind {
     Help,
@@ -50,7 +48,7 @@ impl fmt::Display for ErrorKind {
 
 #[derive(Debug)]
 pub struct Error {
-    pub line_num: usize,
+    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>,
@@ -63,17 +61,14 @@ impl Error {
         format!(
             "{: <10}line {: >3}: {}",
             self.kind.map(|kind| kind.to_string()).unwrap_or_default().to_uppercase(),
-            self.line_num,
+            self.line_num_str(),
             self.msg.cyan(),
         )
     }
-}
 
-#[derive(PartialEq, Debug)]
-enum WhichLine {
-    ThisLine,
-    FollowPrevious(usize),
-    AdjustBackward(usize),
+    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"
@@ -105,12 +100,10 @@ pub fn load_errors(testfile: &Path, revision: Option<&str>) -> Vec<Error> {
         .filter(|(_, line)| line.is_ok())
         .filter_map(|(line_num, line)| {
             parse_expected(last_nonfollow_error, line_num + 1, &line.unwrap(), revision).map(
-                |(which, error)| {
-                    match which {
-                        FollowPrevious(_) => {}
-                        _ => last_nonfollow_error = Some(error.line_num),
+                |(follow_prev, error)| {
+                    if !follow_prev {
+                        last_nonfollow_error = error.line_num;
                     }
-
                     error
                 },
             )
@@ -123,18 +116,19 @@ fn parse_expected(
     line_num: usize,
     line: &str,
     test_revision: Option<&str>,
-) -> Option<(WhichLine, Error)> {
+) -> Option<(bool, Error)> {
     // Matches comments like:
     //     //~
     //     //~|
     //     //~^
     //     //~^^^^^
+    //     //~?
     //     //[rev1]~
     //     //[rev1,rev2]~^^
     static RE: OnceLock<Regex> = OnceLock::new();
 
     let captures = RE
-        .get_or_init(|| Regex::new(r"//(?:\[(?P<revs>[\w\-,]+)])?~(?P<adjust>\||\^*)").unwrap())
+        .get_or_init(|| Regex::new(r"//(?:\[(?P<revs>[\w\-,]+)])?~(?P<adjust>\?|\||\^*)").unwrap())
         .captures(line)?;
 
     match (test_revision, captures.name("revs")) {
@@ -151,11 +145,6 @@ fn parse_expected(
         (Some(_), None) | (None, None) => {}
     }
 
-    let (follow, adjusts) = match &captures["adjust"] {
-        "|" => (true, 0),
-        circumflexes => (false, circumflexes.len()),
-    };
-
     // 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());
@@ -170,28 +159,24 @@ fn parse_expected(
 
     let msg = msg.trim().to_owned();
 
-    let (which, line_num) = if follow {
-        assert_eq!(adjusts, 0, "use either //~| or //~^, not both.");
-        let line_num = last_nonfollow_error.expect(
-            "encountered //~| without \
-             preceding //~^ line.",
-        );
-        (FollowPrevious(line_num), line_num)
+    let line_num_adjust = &captures["adjust"];
+    let (follow_prev, line_num) = if line_num_adjust == "|" {
+        (true, Some(last_nonfollow_error.expect("encountered //~| without preceding //~^ line")))
+    } else if line_num_adjust == "?" {
+        (false, None)
     } else {
-        let which = if adjusts > 0 { AdjustBackward(adjusts) } else { ThisLine };
-        let line_num = line_num - adjusts;
-        (which, line_num)
+        (false, Some(line_num - line_num_adjust.len()))
     };
 
     debug!(
-        "line={} tag={:?} which={:?} kind={:?} msg={:?}",
+        "line={:?} tag={:?} follow_prev={:?} kind={:?} msg={:?}",
         line_num,
         whole_match.as_str(),
-        which,
+        follow_prev,
         kind,
         msg
     );
-    Some((which, Error { line_num, kind, msg }))
+    Some((follow_prev, Error { line_num, kind, msg }))
 }
 
 #[cfg(test)]

src/tools/compiletest/src/json.rs

@@ -2,7 +2,9 @@
 
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
+use std::sync::OnceLock;
 
+use regex::Regex;
 use serde::Deserialize;
 
 use crate::errors::{Error, ErrorKind};
@@ -213,36 +215,24 @@ fn push_expected_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: &DiagnosticSpan, text: &str| {
-        match diagnostic.code {
-            Some(ref code) =>
-            // 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.
-            {
-                format!(
-                    "{}:{}: {}:{}: {} [{}]",
-                    span.line_start,
-                    span.column_start,
-                    span.line_end,
-                    span.column_end,
-                    text,
-                    code.code.clone()
-                )
-            }
-            None =>
-            // FIXME(#33000) -- it'd be better to use a dedicated UI harness
-            {
-                format!(
-                    "{}:{}: {}:{}: {}",
-                    span.line_start, span.column_start, span.line_end, span.column_end, text
-                )
+    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}"),
         }
     };
 
@@ -251,19 +241,41 @@ fn push_expected_errors(
     // more structured shortly anyhow.
     let mut message_lines = diagnostic.message.lines();
     if let Some(first_line) = message_lines.next() {
-        for span in primary_spans {
-            let msg = with_code(span, first_line);
+        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: span.line_start, kind, msg });
+            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 });
+            }
         }
     }
     for next_line in message_lines {
-        for span in primary_spans {
+        if primary_spans.is_empty() {
             expected_errors.push(Error {
-                line_num: span.line_start,
+                line_num: None,
                 kind: None,
-                msg: with_code(span, next_line),
+                msg: with_code(None, next_line),
             });
+        } else {
+            for span in primary_spans {
+                expected_errors.push(Error {
+                    line_num: Some(span.line_start),
+                    kind: None,
+                    msg: with_code(Some(span), next_line),
+                });
+            }
         }
     }
 
@@ -272,7 +284,7 @@ fn push_expected_errors(
         if let Some(ref suggested_replacement) = span.suggested_replacement {
             for (index, line) in suggested_replacement.lines().enumerate() {
                 expected_errors.push(Error {
-                    line_num: span.line_start + index,
+                    line_num: Some(span.line_start + index),
                     kind: Some(ErrorKind::Suggestion),
                     msg: line.to_string(),
                 });
@@ -290,7 +302,7 @@ fn push_expected_errors(
     // 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 {
-            line_num: span.line_start,
+            line_num: Some(span.line_start),
             kind: Some(ErrorKind::Note),
             msg: span.label.clone().unwrap(),
         });
@@ -309,7 +321,7 @@ fn push_backtrace(
 ) {
     if Path::new(&expansion.span.file_name) == Path::new(&file_name) {
         expected_errors.push(Error {
-            line_num: expansion.span.line_start,
+            line_num: Some(expansion.span.line_start),
             kind: Some(ErrorKind::Note),
             msg: format!("in this expansion of {}", expansion.macro_decl_name),
         });

src/tools/compiletest/src/runtest.rs

@@ -747,7 +747,7 @@ impl<'test> TestCx<'test> {
                         self.error(&format!(
                             "{}:{}: unexpected {}: '{}'",
                             file_name,
-                            actual_error.line_num,
+                            actual_error.line_num_str(),
                             actual_error
                                 .kind
                                 .as_ref()
@@ -767,7 +767,7 @@ impl<'test> TestCx<'test> {
                 self.error(&format!(
                     "{}:{}: expected {} not found: {}",
                     file_name,
-                    expected_error.line_num,
+                    expected_error.line_num_str(),
                     expected_error.kind.as_ref().map_or("message".into(), |k| k.to_string()),
                     expected_error.msg
                 ));

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

@@ -182,7 +182,7 @@ impl TestCx<'_> {
         } else if explicit && !expected_errors.is_empty() {
             let msg = format!(
                 "line {}: cannot combine `--error-format` with {} annotations; use `error-pattern` instead",
-                expected_errors[0].line_num,
+                expected_errors[0].line_num_str(),
                 expected_errors[0].kind.unwrap_or(ErrorKind::Error),
             );
             self.fatal(&msg);

tests/rustdoc-ui/coverage/html.rs

@@ -2,3 +2,5 @@
 
 /// Foo
 pub struct Xo;
+
+//~? ERROR `--output-format=html` is not supported for the `--show-coverage` option

tests/rustdoc-ui/deprecated-attrs.rs

@@ -19,3 +19,7 @@
 //~| NOTE see issue #44136
 //~| NOTE no longer functions
 //~| NOTE `doc(plugins)` is now a no-op
+
+//~? WARN the `passes` flag no longer functions
+//~? NOTE see issue #44136
+//~? HELP you may want to use --document-private-items

tests/rustdoc-ui/doctest-output.rs

@@ -1 +1,3 @@
 //@ compile-flags:-Z unstable-options --show-coverage --output-format=doctest
+
+//~? ERROR `--output-format=doctest` is not supported for the `--show-coverage` option

tests/rustdoc-ui/generate-link-to-definition/generate-link-to-definition-opt.rs

@@ -5,3 +5,5 @@
 //@ check-pass
 
 pub fn f() {}
+
+//~? WARN `--generate-link-to-definition` option can only be used with HTML output format

tests/rustdoc-ui/include-str-bare-urls.rs

@@ -13,3 +13,5 @@
 
 #![deny(rustdoc::bare_urls)]
 #![doc=include_str!("auxiliary/include-str-bare-urls.md")]
+
+//~? ERROR this URL is not a hyperlink