Tidy check for test revisions that are mentioned but not declared

If a `[revision]` name appears in a test header directive or error annotation,
but isn't declared in the `//@ revisions:` header, that is almost always a
mistake.

In cases where a revision needs to be temporarily disabled, adding it to an
`//@ unused-revision-names:` header will suppress these checks for that name.

Adding the wildcard name `*` to the unused list will suppress these checks for
the entire file.
This commit is contained in:
Zalathar 2024-05-04 18:06:16 +10:00
parent 1fbabc622f
commit be590d3887
4 changed files with 118 additions and 0 deletions

View File

@ -932,6 +932,8 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"test-mir-pass",
"unset-exec-env",
"unset-rustc-env",
// Used by the tidy check `unknown_revision`.
"unused-revision-names",
// tidy-alphabetical-end
];

View File

@ -87,6 +87,7 @@ pub mod tests_placement;
pub mod tests_revision_unpaired_stdout_stderr;
pub mod ui_tests;
pub mod unit_tests;
pub mod unknown_revision;
pub mod unstable_book;
pub mod walk;
pub mod x_version;

View File

@ -110,6 +110,7 @@ fn main() {
check!(rustdoc_gui_tests, &tests_path);
check!(rustdoc_css_themes, &librustdoc_path);
check!(known_bug, &crashes_path);
check!(unknown_revision, &tests_path);
// Checks that only make sense for the compiler.
check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose);

View File

@ -0,0 +1,114 @@
//! Checks that test revision names appearing in header directives and error
//! annotations have actually been declared in `revisions`.
// FIXME(jieyouxu) Ideally these checks would be integrated into compiletest's
// own directive and revision handling, but for now they've been split out as a
// separate `tidy` check to avoid making compiletest even messier.
use std::collections::{BTreeSet, HashMap, HashSet};
use std::path::Path;
use std::sync::OnceLock;
use ignore::DirEntry;
use regex::Regex;
use crate::iter_header::{iter_header, HeaderLine};
use crate::walk::{filter_dirs, filter_not_rust, walk};
pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
walk(
tests_path.as_ref(),
|path, is_dir| {
filter_dirs(path) || filter_not_rust(path) || {
// Auxiliary source files for incremental tests can refer to revisions
// declared by the main file, which this check doesn't handle.
is_dir && path.file_name().is_some_and(|name| name == "auxiliary")
}
},
&mut |entry, contents| visit_test_file(entry, contents, bad),
);
}
fn visit_test_file(entry: &DirEntry, contents: &str, bad: &mut bool) {
let mut revisions = HashSet::new();
let mut unused_revision_names = HashSet::new();
// Maps each mentioned revision to the first line it was mentioned on.
let mut mentioned_revisions = HashMap::<&str, usize>::new();
let mut add_mentioned_revision = |line_number: usize, revision| {
let first_line = mentioned_revisions.entry(revision).or_insert(line_number);
*first_line = (*first_line).min(line_number);
};
// Scan all `//@` headers to find declared revisions and mentioned revisions.
iter_header(contents, &mut |HeaderLine { line_number, revision, directive }| {
if let Some(revs) = directive.strip_prefix("revisions:") {
revisions.extend(revs.split_whitespace());
} else if let Some(revs) = directive.strip_prefix("unused-revision-names:") {
unused_revision_names.extend(revs.split_whitespace());
}
if let Some(revision) = revision {
add_mentioned_revision(line_number, revision);
}
});
// If a wildcard appears in `unused-revision-names`, skip all revision name
// checking for this file.
if unused_revision_names.contains(&"*") {
return;
}
// Scan all `//[rev]~` error annotations to find mentioned revisions.
for_each_error_annotation_revision(contents, &mut |ErrorAnnRev { line_number, revision }| {
add_mentioned_revision(line_number, revision);
});
let path = entry.path().display();
// Fail if any revision names appear in both places, since that's probably a mistake.
for rev in revisions.intersection(&unused_revision_names).copied().collect::<BTreeSet<_>>() {
tidy_error!(
bad,
"revision name [{rev}] appears in both `revisions` and `unused-revision-names` in {path}"
);
}
// Compute the set of revisions that were mentioned but not declared,
// sorted by the first line number they appear on.
let mut bad_revisions = mentioned_revisions
.into_iter()
.filter(|(rev, _)| !revisions.contains(rev) && !unused_revision_names.contains(rev))
.map(|(rev, line_number)| (line_number, rev))
.collect::<Vec<_>>();
bad_revisions.sort();
for (line_number, rev) in bad_revisions {
tidy_error!(bad, "unknown revision [{rev}] at {path}:{line_number}");
}
}
struct ErrorAnnRev<'a> {
line_number: usize,
revision: &'a str,
}
fn for_each_error_annotation_revision<'a>(
contents: &'a str,
callback: &mut dyn FnMut(ErrorAnnRev<'a>),
) {
let error_regex = {
// Simplified from the regex used by `parse_expected` in `src/tools/compiletest/src/errors.rs`,
// because we only care about extracting revision names.
static RE: OnceLock<Regex> = OnceLock::new();
RE.get_or_init(|| Regex::new(r"//\[(?<revs>[^]]*)\]~").unwrap())
};
for (line_number, line) in (1..).zip(contents.lines()) {
let Some(captures) = error_regex.captures(line) else { continue };
for revision in captures.name("revs").unwrap().as_str().split(',') {
callback(ErrorAnnRev { line_number, revision });
}
}
}