Rollup merge of #121475 - jieyouxu:tidy-stderr-check, r=the8472,compiler-errors

Add tidy check for .stderr/.stdout files for non-existent test revisions

Closes #77498.

Author: matthiaskrgr

Diff for: src/tools/tidy/src/iter_header.rs

@@ -0,0 +1,32 @@
+const COMMENT: &str = "//@";
+
+/// A header line, like `//@name: value` consists of the prefix `//@` and the directive
+/// `name: value`. It is also possibly revisioned, e.g. `//@[revision] name: value`.
+pub(crate) struct HeaderLine<'ln> {
+    pub(crate) revision: Option<&'ln str>,
+    pub(crate) directive: &'ln str,
+}
+
+/// Iterate through compiletest headers in a test contents.
+///
+/// Adjusted from compiletest/src/header.rs.
+pub(crate) fn iter_header<'ln>(contents: &'ln str, it: &mut dyn FnMut(HeaderLine<'ln>)) {
+    for ln in contents.lines() {
+        let ln = ln.trim();
+
+        // We're left with potentially `[rev]name: value`.
+        let Some(remainder) = ln.strip_prefix(COMMENT) else {
+            continue;
+        };
+
+        if let Some(remainder) = remainder.trim_start().strip_prefix('[') {
+            let Some((revision, remainder)) = remainder.split_once(']') else {
+                panic!("malformed revision directive: expected `//@[rev]`, found `{ln}`");
+            };
+            // We trimmed off the `[rev]` portion, left with `name: value`.
+            it(HeaderLine { revision: Some(revision), directive: remainder.trim() });
+        } else {
+            it(HeaderLine { revision: None, directive: remainder.trim() });
+        }
+    }
+}

Diff for: src/tools/tidy/src/lib.rs

@@ -65,6 +65,7 @@ pub mod ext_tool_checks;
 pub mod extdeps;
 pub mod features;
 pub mod fluent_alphabetical;
+pub(crate) mod iter_header;
 pub mod mir_opt_tests;
 pub mod pal;
 pub mod rustdoc_css_themes;
@@ -73,6 +74,7 @@ pub mod style;
 pub mod target_policy;
 pub mod target_specific_tests;
 pub mod tests_placement;
+pub mod tests_revision_unpaired_stdout_stderr;
 pub mod ui_tests;
 pub mod unit_tests;
 pub mod unstable_book;

Diff for: src/tools/tidy/src/main.rs

@@ -100,6 +100,7 @@ fn main() {
 
         // Checks over tests.
         check!(tests_placement, &root_path);
+        check!(tests_revision_unpaired_stdout_stderr, &tests_path);
         check!(debug_artifacts, &tests_path);
         check!(ui_tests, &tests_path, bless);
         check!(mir_opt_tests, &tests_path, bless);

Diff for: src/tools/tidy/src/target_specific_tests.rs

@@ -4,32 +4,12 @@
 use std::collections::BTreeMap;
 use std::path::Path;
 
+use crate::iter_header::{iter_header, HeaderLine};
 use crate::walk::filter_not_rust;
 
-const COMMENT: &str = "//@";
 const LLVM_COMPONENTS_HEADER: &str = "needs-llvm-components:";
 const COMPILE_FLAGS_HEADER: &str = "compile-flags:";
 
-/// Iterate through compiletest headers in a test contents.
-///
-/// Adjusted from compiletest/src/header.rs.
-fn iter_header<'a>(contents: &'a str, it: &mut dyn FnMut(Option<&'a str>, &'a str)) {
-    for ln in contents.lines() {
-        let ln = ln.trim();
-        if ln.starts_with(COMMENT) && ln[COMMENT.len()..].trim_start().starts_with('[') {
-            if let Some(close_brace) = ln.find(']') {
-                let open_brace = ln.find('[').unwrap();
-                let lncfg = &ln[open_brace + 1..close_brace];
-                it(Some(lncfg), ln[(close_brace + 1)..].trim_start());
-            } else {
-                panic!("malformed condition directive: expected `//[foo]`, found `{ln}`")
-            }
-        } else if ln.starts_with(COMMENT) {
-            it(None, ln[COMMENT.len()..].trim_start());
-        }
-    }
-}
-
 #[derive(Default, Debug)]
 struct RevisionInfo<'a> {
     target_arch: Option<&'a str>,
@@ -40,9 +20,9 @@ pub fn check(path: &Path, bad: &mut bool) {
     crate::walk::walk(path, |path, _is_dir| filter_not_rust(path), &mut |entry, content| {
         let file = entry.path().display();
         let mut header_map = BTreeMap::new();
-        iter_header(content, &mut |cfg, directive| {
+        iter_header(content, &mut |HeaderLine { revision, directive }| {
             if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) {
-                let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
+                let info = header_map.entry(revision).or_insert(RevisionInfo::default());
                 let comp_vec = info.llvm_components.get_or_insert(Vec::new());
                 for component in value.split(' ') {
                     let component = component.trim();
@@ -56,7 +36,7 @@ pub fn check(path: &Path, bad: &mut bool) {
                     if let Some((arch, _)) =
                         v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-")
                     {
-                        let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
+                        let info = header_map.entry(revision).or_insert(RevisionInfo::default());
                         info.target_arch.replace(arch);
                     } else {
                         eprintln!("{file}: seems to have a malformed --target value");

Diff for: src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs

@@ -0,0 +1,146 @@
+//! Checks that there are no unpaired `.stderr` or `.stdout` for a test with and without revisions.
+
+use std::collections::{BTreeMap, BTreeSet};
+use std::ffi::OsStr;
+use std::path::Path;
+
+use crate::iter_header::*;
+use crate::walk::*;
+
+// Should be kept in sync with `CompareMode` in `src/tools/compiletest/src/common.rs`,
+// as well as `run`.
+const IGNORES: &[&str] = &[
+    "polonius",
+    "chalk",
+    "split-dwarf",
+    "split-dwarf-single",
+    "next-solver-coherence",
+    "next-solver",
+    "run",
+];
+const EXTENSIONS: &[&str] = &["stdout", "stderr"];
+const SPECIAL_TEST: &str = "tests/ui/command/need-crate-arg-ignore-tidy.x.rs";
+
+pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
+    // Recurse over subdirectories under `tests/`
+    walk_dir(tests_path.as_ref(), filter, &mut |entry| {
+        // We are inspecting a folder. Collect the paths to interesting files `.rs`, `.stderr`,
+        // `.stdout` under the current folder (shallow).
+        let mut files_under_inspection = BTreeSet::new();
+        for sibling in std::fs::read_dir(entry.path()).unwrap() {
+            let Ok(sibling) = sibling else {
+                continue;
+            };
+
+            if sibling.path().is_dir() {
+                continue;
+            }
+
+            let sibling_path = sibling.path();
+
+            let Some(ext) = sibling_path.extension().map(OsStr::to_str).flatten() else {
+                continue;
+            };
+
+            if ext == "rs" || EXTENSIONS.contains(&ext) {
+                files_under_inspection.insert(sibling_path);
+            }
+        }
+
+        let mut test_info = BTreeMap::new();
+
+        for test in
+            files_under_inspection.iter().filter(|f| f.extension().is_some_and(|ext| ext == "rs"))
+        {
+            if test.ends_with(SPECIAL_TEST) {
+                continue;
+            }
+
+            let mut expected_revisions = BTreeSet::new();
+
+            let contents = std::fs::read_to_string(test).unwrap();
+
+            // Collect directives.
+            iter_header(&contents, &mut |HeaderLine { revision, directive }| {
+                // We're trying to *find* `//@ revision: xxx` directives themselves, not revisioned
+                // directives.
+                if revision.is_some() {
+                    return;
+                }
+
+                let directive = directive.trim();
+
+                if directive.starts_with("revisions") {
+                    let Some((name, value)) = directive.split_once([':', ' ']) else {
+                        return;
+                    };
+
+                    if name == "revisions" {
+                        let revs = value.split(' ');
+                        for rev in revs {
+                            expected_revisions.insert(rev.to_owned());
+                        }
+                    }
+                }
+            });
+
+            let Some((test_name, _)) = test.to_str().map(|s| s.split_once('.')).flatten() else {
+                continue;
+            };
+
+            test_info.insert(test_name.to_string(), (test, expected_revisions));
+        }
+
+        // Our test file `foo.rs` has specified no revisions. There should not be any
+        // `foo.rev{.stderr,.stdout}` files. rustc-dev-guide says test output files can have names
+        // of the form: `test-name.revision.compare_mode.extension`, but our only concern is
+        // `test-name.revision` and `extension`.
+        for sibling in files_under_inspection.iter().filter(|f| {
+            f.extension().map(OsStr::to_str).flatten().is_some_and(|ext| EXTENSIONS.contains(&ext))
+        }) {
+            let filename_components = sibling.to_str().unwrap().split('.').collect::<Vec<_>>();
+            let file_prefix = filename_components[0];
+
+            let Some((test_path, expected_revisions)) = test_info.get(file_prefix) else {
+                continue;
+            };
+
+            match filename_components[..] {
+                // Cannot have a revision component, skip.
+                [] | [_] => return,
+                [_, _] if !expected_revisions.is_empty() => {
+                    // Found unrevisioned output files for a revisioned test.
+                    tidy_error!(
+                        bad,
+                        "found unrevisioned output file `{}` for a revisioned test `{}`",
+                        sibling.display(),
+                        test_path.display(),
+                    );
+                }
+                [_, _] => return,
+                [_, found_revision, .., extension] => {
+                    if !IGNORES.contains(&found_revision)
+                        && !expected_revisions.contains(found_revision)
+                        // This is from `//@ stderr-per-bitwidth`
+                        && !(extension == "stderr" && ["32bit", "64bit"].contains(&found_revision))
+                    {
+                        // Found some unexpected revision-esque component that is not a known
+                        // compare-mode or expected revision.
+                        tidy_error!(
+                            bad,
+                            "found output file `{}` for unexpected revision `{}` of test `{}`",
+                            sibling.display(),
+                            found_revision,
+                            test_path.display()
+                        );
+                    }
+                }
+            }
+        }
+    });
+}
+
+fn filter(path: &Path) -> bool {
+    filter_dirs(path) // ignore certain dirs
+        || (path.file_name().is_some_and(|name| name == "auxiliary")) // ignore auxiliary folder
+}

Diff for: src/tools/tidy/src/walk.rs

@@ -86,3 +86,20 @@ pub(crate) fn walk_no_read(
         }
     }
 }
+
+// Walk through directories and skip symlinks.
+pub(crate) fn walk_dir(
+    path: &Path,
+    skip: impl Send + Sync + 'static + Fn(&Path) -> bool,
+    f: &mut dyn FnMut(&DirEntry),
+) {
+    let mut walker = ignore::WalkBuilder::new(path);
+    let walker = walker.filter_entry(move |e| !skip(e.path()));
+    for entry in walker.build() {
+        if let Ok(entry) = entry {
+            if entry.path().is_dir() {
+                f(&entry);
+            }
+        }
+    }
+}