Rollup merge of #139618 - petrochenkov:virsugg, r=jieyouxu

compiletest: Make `SUGGESTION` annotations viral

If one of them is expected in a test file, then others should be annotated as well, in the same way as with `HELP`s and `NOTE`s.
This doesn't require much of an additional annotation burden, but simplifies the rules.

r? ```@jieyouxu```

Author: ChrisDenton

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

@@ -101,6 +101,7 @@ for more details.
 | `normalize-stdout`                | Normalize actual stdout with a rule `"<raw>" -> "<normalized>"` before comparing against snapshot                        | `ui`, `incremental`                          | `"<RAW>" -> "<NORMALIZED>"`, `<RAW>`/`<NORMALIZED>` is regex capture and replace syntax |
 | `dont-check-compiler-stderr`      | Don't check actual compiler stderr vs stderr snapshot                                                                    | `ui`                                         | N/A                                                                                     |
 | `dont-check-compiler-stdout`      | Don't check actual compiler stdout vs stdout snapshot                                                                    | `ui`                                         | N/A                                                                                     |
+| `dont-require-annotations`        | Don't require line annotations for the given diagnostic kind (`//~ KIND`) to be exhaustive                               | `ui`, `incremental`                          | `ERROR`, `WARN`, `NOTE`, `HELP`, `SUGGESTION`                                           |
 | `run-rustfix`                     | Apply all suggestions via `rustfix`, snapshot fixed output, and check fixed output builds                                | `ui`                                         | N/A                                                                                     |
 | `rustfix-only-machine-applicable` | `run-rustfix` but only machine-applicable suggestions                                                                    | `ui`                                         | N/A                                                                                     |
 | `exec-env`                        | Env var to set when executing a test                                                                                     | `ui`, `crashes`                              | `<KEY>=<VALUE>`                                                                         |

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

@@ -303,8 +303,7 @@ It should be preferred to using `error-pattern`, which is imprecise and non-exha
 ### `error-pattern`
 
 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.
+have a specific span, or in exceptional cases for compile time messages.
 
 Let's think about this test:
 
@@ -318,7 +317,7 @@ fn main() {
 ```
 
 We want to ensure this shows "index out of bounds" but we cannot use the `ERROR`
-annotation since the error doesn't have any span. Then it's time to use the
+annotation since the runtime error doesn't have any span. Then it's time to use the
 `error-pattern` directive:
 
 ```rust,ignore
@@ -331,29 +330,51 @@ fn main() {
 }
 ```
 
-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.
+Use of `error-pattern` is not recommended in general.
 
-Per-line annotations (`//~`) are still checked in tests using `error-pattern`.
-To opt out of these checks, use `//@ compile-flags: --error-format=human`.
-Do that only in exceptional cases.
+For strict testing of compile time output, try to use the line annotations `//~` as much as
+possible, including `//~?` annotations for diagnostics without span.
 
-### Error levels
+If the compile time output is target dependent or too verbose, use directive
+`//@ dont-require-annotations: <diagnostic-kind>` to make the line annotation checking
+non-exhaustive, some of the compiler messages can stay uncovered by annotations in this mode.
 
-The error levels that you can have are:
+For checking runtime output `//@ check-run-results` may be preferable.
+
+Only use `error-pattern` if none of the above works.
+
+Line annotations `//~` are still checked in tests using `error-pattern`.
+In exceptional cases use `//@ compile-flags: --error-format=human` to opt out of these checks.
+
+### Diagnostic kinds (error levels)
+
+The diagnostic kinds that you can have are:
 
 - `ERROR`
-- `WARN` or `WARNING`
+- `WARN` (or `WARNING`)
 - `NOTE`
-- `HELP` and `SUGGESTION`
-
-You are allowed to not include a level, but you should include it at least for
-the primary message.
+- `HELP`
+- `SUGGESTION`
 
-The `SUGGESTION` level is used for specifying what the expected replacement text
+The `SUGGESTION` kind is used for specifying what the expected replacement text
 should be for a diagnostic suggestion.
 
+`ERROR` and `WARN` kinds are required to be exhaustively covered by line annotations
+`//~` by default.
+
+Other kinds only need to be line-annotated if at least one annotation of that kind appears
+in the test file. For example, one `//~ NOTE` will also require all other `//~ NOTE`s in the file
+to be written out explicitly.
+
+Use directive `//@ dont-require-annotations` to opt out of exhaustive annotations.
+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.
+
 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
 samples often have unused warnings. If the test is specifically testing an

src/tools/compiletest/src/header.rs

@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::HashSet;
 use std::env;
 use std::fs::File;
 use std::io::BufReader;
@@ -198,7 +198,7 @@ pub struct TestProps {
     /// that don't otherwise want/need `-Z build-std`.
     pub add_core_stubs: bool,
     /// Whether line annotatins are required for the given error kind.
-    pub require_annotations: HashMap<ErrorKind, bool>,
+    pub dont_require_annotations: HashSet<ErrorKind>,
 }
 
 mod directives {
@@ -301,13 +301,7 @@ impl TestProps {
             no_auto_check_cfg: false,
             has_enzyme: false,
             add_core_stubs: false,
-            require_annotations: HashMap::from([
-                (ErrorKind::Help, true),
-                (ErrorKind::Note, true),
-                (ErrorKind::Error, true),
-                (ErrorKind::Warning, true),
-                (ErrorKind::Suggestion, false),
-            ]),
+            dont_require_annotations: Default::default(),
         }
     }
 
@@ -593,8 +587,8 @@ impl TestProps {
                     if let Some(err_kind) =
                         config.parse_name_value_directive(ln, DONT_REQUIRE_ANNOTATIONS)
                     {
-                        self.require_annotations
-                            .insert(ErrorKind::expect_from_user_str(&err_kind), false);
+                        self.dont_require_annotations
+                            .insert(ErrorKind::expect_from_user_str(err_kind.trim()));
                     }
                 },
             );

src/tools/compiletest/src/runtest.rs

@@ -22,7 +22,7 @@ use crate::common::{
     output_base_dir, output_base_name, output_testname_unique,
 };
 use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff};
-use crate::errors::{self, Error, ErrorKind};
+use crate::errors::{Error, ErrorKind};
 use crate::header::TestProps;
 use crate::read2::{Truncated, read2_abbreviated};
 use crate::util::{PathBufExt, add_dylib_path, logv, static_regex};
@@ -675,7 +675,7 @@ impl<'test> TestCx<'test> {
         }
     }
 
-    fn check_expected_errors(&self, expected_errors: Vec<errors::Error>, proc_res: &ProcRes) {
+    fn check_expected_errors(&self, expected_errors: Vec<Error>, proc_res: &ProcRes) {
         debug!(
             "check_expected_errors: expected_errors={:?} proc_res.status={:?}",
             expected_errors, proc_res.status
@@ -710,8 +710,12 @@ 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));
+        // Errors and warnings are always expected, other diagnostics are only expected
+        // 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))
+            .collect();
 
         // 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);
@@ -737,8 +741,11 @@ impl<'test> TestCx<'test> {
                 }
 
                 None => {
-                    // If the test is a known bug, don't require that the error is annotated
-                    if self.is_unexpected_compiler_message(&actual_error, expect_help, expect_note)
+                    if actual_error.require_annotation
+                        && actual_error.kind.map_or(false, |kind| {
+                            expected_kinds.contains(&kind)
+                                && !self.props.dont_require_annotations.contains(&kind)
+                        })
                     {
                         self.error(&format!(
                             "{}:{}: unexpected {}: '{}'",
@@ -796,27 +803,6 @@ impl<'test> TestCx<'test> {
         }
     }
 
-    /// Returns `true` if we should report an error about `actual_error`,
-    /// which did not match any of the expected error.
-    fn is_unexpected_compiler_message(
-        &self,
-        actual_error: &Error,
-        expect_help: bool,
-        expect_note: bool,
-    ) -> bool {
-        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,
-                }
-            })
-    }
-
     fn should_emit_metadata(&self, pm: Option<PassMode>) -> Emit {
         match (pm, self.props.fail_mode, self.config.mode) {
             (Some(PassMode::Check), ..) | (_, Some(FailMode::Check), Ui) => Emit::Metadata,

tests/ui/async-await/suggest-missing-await.rs

@@ -1,4 +1,5 @@
 //@ edition:2018
+//@ dont-require-annotations: SUGGESTION
 
 fn take_u32(_x: u32) {}
 

tests/ui/async-await/suggest-missing-await.stderr

@@ -1,18 +1,18 @@
 error[E0308]: mismatched types
-  --> $DIR/suggest-missing-await.rs:12:14
+  --> $DIR/suggest-missing-await.rs:13:14
    |
 LL |     take_u32(x)
    |     -------- ^ expected `u32`, found future
    |     |
    |     arguments to this function are incorrect
    |
 note: calling an async function returns a future
-  --> $DIR/suggest-missing-await.rs:12:14
+  --> $DIR/suggest-missing-await.rs:13:14
    |
 LL |     take_u32(x)
    |              ^
 note: function defined here
-  --> $DIR/suggest-missing-await.rs:3:4
+  --> $DIR/suggest-missing-await.rs:4:4
    |
 LL | fn take_u32(_x: u32) {}
    |    ^^^^^^^^ -------
@@ -22,13 +22,13 @@ LL |     take_u32(x.await)
    |               ++++++
 
 error[E0308]: mismatched types
-  --> $DIR/suggest-missing-await.rs:22:5
+  --> $DIR/suggest-missing-await.rs:23:5
    |
 LL |     dummy()
    |     ^^^^^^^ expected `()`, found future
    |
 note: calling an async function returns a future
-  --> $DIR/suggest-missing-await.rs:22:5
+  --> $DIR/suggest-missing-await.rs:23:5
    |
 LL |     dummy()
    |     ^^^^^^^
@@ -42,7 +42,7 @@ LL |     dummy();
    |            +
 
 error[E0308]: `if` and `else` have incompatible types
-  --> $DIR/suggest-missing-await.rs:35:9
+  --> $DIR/suggest-missing-await.rs:36:9
    |
 LL |       let _x = if true {
    |  ______________-
@@ -64,7 +64,7 @@ LL |         dummy().await
    |                ++++++
 
 error[E0308]: `match` arms have incompatible types
-  --> $DIR/suggest-missing-await.rs:45:14
+  --> $DIR/suggest-missing-await.rs:46:14
    |
 LL |       let _x = match 0usize {
    |  ______________-
@@ -87,7 +87,7 @@ LL ~         1 => dummy().await,
    |
 
 error[E0308]: mismatched types
-  --> $DIR/suggest-missing-await.rs:53:9
+  --> $DIR/suggest-missing-await.rs:54:9
    |
 LL |     let _x = match dummy() {
    |                    ------- this expression has type `impl Future<Output = ()>`
@@ -102,7 +102,7 @@ LL |     let _x = match dummy().await {
    |                           ++++++
 
 error[E0308]: mismatched types
-  --> $DIR/suggest-missing-await.rs:67:9
+  --> $DIR/suggest-missing-await.rs:68:9
    |
 LL |     match dummy_result() {
    |           -------------- this expression has type `impl Future<Output = Result<(), ()>>`
@@ -118,7 +118,7 @@ LL |     match dummy_result().await {
    |                         ++++++
 
 error[E0308]: mismatched types
-  --> $DIR/suggest-missing-await.rs:69:9
+  --> $DIR/suggest-missing-await.rs:70:9
    |
 LL |     match dummy_result() {
    |           -------------- this expression has type `impl Future<Output = Result<(), ()>>`
@@ -134,7 +134,7 @@ LL |     match dummy_result().await {
    |                         ++++++
 
 error[E0308]: mismatched types
-  --> $DIR/suggest-missing-await.rs:77:27
+  --> $DIR/suggest-missing-await.rs:78:27
    |
 LL |     Some(do_async()).map(|()| {});
    |                           ^^

tests/ui/cast/cast-as-bool.rs

@@ -1,3 +1,5 @@
+//@ dont-require-annotations: SUGGESTION
+
 fn main() {
     let u = 5 as bool; //~ ERROR cannot cast `i32` as `bool`
                        //~| HELP compare with zero instead

tests/ui/cast/cast-as-bool.stderr

@@ -1,5 +1,5 @@
 error[E0054]: cannot cast `i32` as `bool`
-  --> $DIR/cast-as-bool.rs:2:13
+  --> $DIR/cast-as-bool.rs:4:13
    |
 LL |     let u = 5 as bool;
    |             ^^^^^^^^^
@@ -11,7 +11,7 @@ LL +     let u = 5 != 0;
    |
 
 error[E0054]: cannot cast `i32` as `bool`
-  --> $DIR/cast-as-bool.rs:6:13
+  --> $DIR/cast-as-bool.rs:8:13
    |
 LL |     let t = (1 + 2) as bool;
    |             ^^^^^^^^^^^^^^^
@@ -23,7 +23,7 @@ LL +     let t = (1 + 2) != 0;
    |
 
 error[E0054]: cannot cast `u32` as `bool`
-  --> $DIR/cast-as-bool.rs:10:13
+  --> $DIR/cast-as-bool.rs:12:13
    |
 LL |     let _ = 5_u32 as bool;
    |             ^^^^^^^^^^^^^
@@ -35,7 +35,7 @@ LL +     let _ = 5_u32 != 0;
    |
 
 error[E0054]: cannot cast `f64` as `bool`
-  --> $DIR/cast-as-bool.rs:13:13
+  --> $DIR/cast-as-bool.rs:15:13
    |
 LL |     let _ = 64.0_f64 as bool;
    |             ^^^^^^^^^^^^^^^^
@@ -47,43 +47,43 @@ LL +     let _ = 64.0_f64 != 0;
    |
 
 error[E0054]: cannot cast `IntEnum` as `bool`
-  --> $DIR/cast-as-bool.rs:24:13
+  --> $DIR/cast-as-bool.rs:26:13
    |
 LL |     let _ = IntEnum::One as bool;
    |             ^^^^^^^^^^^^^^^^^^^^ unsupported cast
 
 error[E0054]: cannot cast `fn(u8) -> String {uwu}` as `bool`
-  --> $DIR/cast-as-bool.rs:33:13
+  --> $DIR/cast-as-bool.rs:35:13
    |
 LL |     let _ = uwu as bool;
    |             ^^^^^^^^^^^ unsupported cast
 
 error[E0054]: cannot cast `unsafe fn() {owo}` as `bool`
-  --> $DIR/cast-as-bool.rs:35:13
+  --> $DIR/cast-as-bool.rs:37:13
    |
 LL |     let _ = owo as bool;
    |             ^^^^^^^^^^^ unsupported cast
 
 error[E0054]: cannot cast `fn(u8) -> String` as `bool`
-  --> $DIR/cast-as-bool.rs:38:13
+  --> $DIR/cast-as-bool.rs:40:13
    |
 LL |     let _ = uwu as fn(u8) -> String as bool;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsupported cast
 
 error[E0054]: cannot cast `char` as `bool`
-  --> $DIR/cast-as-bool.rs:40:13
+  --> $DIR/cast-as-bool.rs:42:13
    |
 LL |     let _ = 'x' as bool;
    |             ^^^^^^^^^^^ unsupported cast
 
 error[E0054]: cannot cast `*const ()` as `bool`
-  --> $DIR/cast-as-bool.rs:44:13
+  --> $DIR/cast-as-bool.rs:46:13
    |
 LL |     let _ = ptr as bool;
    |             ^^^^^^^^^^^ unsupported cast
 
 error[E0606]: casting `&'static str` as `bool` is invalid
-  --> $DIR/cast-as-bool.rs:46:13
+  --> $DIR/cast-as-bool.rs:48:13
    |
 LL |     let v = "hello" as bool;
    |             ^^^^^^^^^^^^^^^