Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect typos for compiletest test directives #121561

Merged
merged 2 commits into from
Mar 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 97 additions & 31 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,8 @@ pub fn line_directive<'line>(
/// This is generated by collecting directives from ui tests and then extracting their directive
/// names. This is **not** an exhaustive list of all possible directives. Instead, this is a
/// best-effort approximation for diagnostics.
const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
// tidy-alphabetical-start
"assembly-output",
"aux-build",
"aux-crate",
Expand All @@ -693,13 +694,15 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"check-stdout",
"check-test-line-numbers-match",
"compile-flags",
"count",
"dont-check-compiler-stderr",
"dont-check-compiler-stdout",
"dont-check-failure-status",
"edition",
"error-pattern",
"exec-env",
"failure-status",
"filecheck-flags",
"forbid-output",
"force-host",
"ignore-16bit",
Expand All @@ -716,6 +719,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"ignore-compare-mode-polonius",
"ignore-cross-compile",
"ignore-debug",
"ignore-eabi",
"ignore-emscripten",
"ignore-endian-big",
"ignore-freebsd",
Expand All @@ -731,14 +735,30 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"ignore-lldb",
"ignore-llvm-version",
"ignore-loongarch64",
"ignore-macabi",
"ignore-macos",
"ignore-mode-assembly",
"ignore-mode-codegen",
"ignore-mode-codegen-units",
"ignore-mode-coverage-map",
"ignore-mode-coverage-run",
"ignore-mode-debuginfo",
"ignore-mode-incremental",
"ignore-mode-js-doc-test",
"ignore-mode-mir-opt",
"ignore-mode-pretty",
"ignore-mode-run-make",
"ignore-mode-run-pass-valgrind",
"ignore-mode-rustdoc",
"ignore-mode-rustdoc-json",
"ignore-mode-ui",
"ignore-mode-ui-fulldeps",
"ignore-msp430",
"ignore-msvc",
"ignore-musl",
"ignore-netbsd",
"ignore-nightly",
"ignore-none",
"ignore-nto",
"ignore-nvptx64",
"ignore-openbsd",
Expand All @@ -750,35 +770,47 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"ignore-spirv",
"ignore-stable",
"ignore-stage1",
"ignore-stage2",
"ignore-test",
"ignore-thumb",
"ignore-thumbv8m.base-none-eabi",
"ignore-thumbv8m.main-none-eabi",
"ignore-unix",
"ignore-unknown",
"ignore-uwp",
"ignore-vxworks",
"ignore-wasi",
"ignore-wasm",
"ignore-wasm32",
"ignore-wasm32-bare",
"ignore-wasm64",
"ignore-windows",
"ignore-windows-gnu",
"ignore-x32",
"ignore-x86",
"ignore-x86_64",
"ignore-x86_64-apple-darwin",
"ignore-x86_64-unknown-linux-gnu",
"incremental",
"known-bug",
"llvm-cov-flags",
"min-cdb-version",
"min-gdb-version",
"min-lldb-version",
"min-llvm-version",
"min-system-llvm-version",
"needs-asm-support",
"needs-dlltool",
"needs-dynamic-linking",
"needs-git-hash",
"needs-llvm-components",
"needs-profiler-support",
"needs-relocation-model-pic",
"needs-run-enabled",
"needs-rust-lldb",
"needs-sanitizer-address",
"needs-sanitizer-cfi",
"needs-sanitizer-dataflow",
"needs-sanitizer-hwaddress",
"needs-sanitizer-leak",
"needs-sanitizer-memory",
Expand All @@ -801,6 +833,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"only-aarch64",
"only-arm",
"only-avr",
"only-beta",
"only-bpf",
"only-cdb",
"only-gnu",
Expand All @@ -818,13 +851,15 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"only-riscv64",
"only-sparc",
"only-sparc64",
"only-stable",
"only-thumb",
"only-wasm32",
"only-wasm32-bare",
"only-windows",
"only-x86",
"only-x86_64",
"only-x86_64-fortanix-unknown-sgx",
"only-x86_64-pc-windows-gnu",
"only-x86_64-pc-windows-msvc",
"only-x86_64-unknown-linux-gnu",
"pp-exact",
Expand All @@ -846,6 +881,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"unit-test",
"unset-exec-env",
"unset-rustc-env",
// tidy-alphabetical-end
];

/// The broken-down contents of a line containing a test header directive,
Expand Down Expand Up @@ -876,6 +912,22 @@ struct HeaderLine<'ln> {
directive: &'ln str,
}

pub(crate) struct CheckDirectiveResult<'ln> {
is_known_directive: bool,
directive_name: &'ln str,
}

// Returns `(is_known_directive, directive_name)`.
pub(crate) fn check_directive(directive_ln: &str) -> CheckDirectiveResult<'_> {
let directive_name =
directive_ln.split_once([':', ' ']).map(|(pre, _)| pre).unwrap_or(directive_ln);

CheckDirectiveResult {
is_known_directive: KNOWN_DIRECTIVE_NAMES.contains(&directive_name),
directive_name: directive_ln,
}
}

fn iter_header(
mode: Mode,
_suite: &str,
Expand Down Expand Up @@ -915,6 +967,7 @@ fn iter_header(
let mut ln = String::new();
let mut line_number = 0;

// Match on error annotations like `//~ERROR`.
static REVISION_MAGIC_COMMENT_RE: Lazy<Regex> =
Lazy::new(|| Regex::new("//(\\[.*\\])?~.*").unwrap());

Expand All @@ -933,9 +986,38 @@ fn iter_header(
if ln.starts_with("fn") || ln.starts_with("mod") {
return;

// First try to accept `ui_test` style comments
} else if let Some((header_revision, directive)) = line_directive(comment, ln) {
it(HeaderLine { line_number, original_line, header_revision, directive });
// First try to accept `ui_test` style comments (`//@`)
} else if let Some((header_revision, non_revisioned_directive_line)) =
line_directive(comment, ln)
{
// Perform unknown directive check on Rust files.
if testfile.extension().map(|e| e == "rs").unwrap_or(false) {
let directive_ln = non_revisioned_directive_line.trim();

let CheckDirectiveResult { is_known_directive, .. } = check_directive(directive_ln);

if !is_known_directive {
*poisoned = true;

eprintln!(
"error: detected unknown compiletest test directive `{}` in {}:{}",
directive_ln,
testfile.display(),
line_number,
);

return;
}
}

it(HeaderLine {
line_number,
original_line,
header_revision,
directive: non_revisioned_directive_line,
});
// Then we try to check for legacy-style candidates, which are not the magic ~ERROR family
// error annotations.
} else if !REVISION_MAGIC_COMMENT_RE.is_match(ln) {
let Some((_, rest)) = line_directive("//", ln) else {
continue;
Expand All @@ -949,34 +1031,18 @@ fn iter_header(

let rest = rest.trim_start();

for candidate in DIAGNOSTICS_DIRECTIVE_NAMES.iter() {
if rest.starts_with(candidate) {
let Some(prefix_removed) = rest.strip_prefix(candidate) else {
// We have a comment that's *successfully* parsed as an legacy-style
// directive. We emit an error here to warn the user.
*poisoned = true;
eprintln!(
"error: detected legacy-style directives in compiletest test: {}:{}, please use `ui_test`-style directives `//@` instead:{:#?}",
testfile.display(),
line_number,
line_directive("//", ln),
);
return;
};
let CheckDirectiveResult { is_known_directive, directive_name } = check_directive(rest);

if prefix_removed.starts_with([' ', ':']) {
// We have a comment that's *successfully* parsed as an legacy-style
// directive. We emit an error here to warn the user.
*poisoned = true;
eprintln!(
"error: detected legacy-style directives in compiletest test: {}:{}, please use `ui_test`-style directives `//@` instead:{:#?}",
testfile.display(),
line_number,
line_directive("//", ln),
);
return;
}
}
if is_known_directive {
*poisoned = true;
eprintln!(
"error: detected legacy-style directive {} in compiletest test: {}:{}, please use `ui_test`-style directives `//@` instead: {:#?}",
directive_name,
testfile.display(),
line_number,
line_directive("//", ln),
);
return;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//@ check-pass

//~ HELP
fn main() {} //~ERROR
//~^ ERROR
//~| ERROR
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
//! ignore-wasm
//@ ignore-wasm
//@ check-pass
// regular comment
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// ignore-wasm
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# ignore-owo
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
//@ needs-headpat
62 changes: 62 additions & 0 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use std::str::FromStr;
use crate::common::{Config, Debugger, Mode};
use crate::header::{parse_normalization_string, EarlyProps, HeadersCache};

use super::iter_header;

fn make_test_description<R: Read>(
config: &Config,
name: test::TestName,
Expand Down Expand Up @@ -612,3 +614,63 @@ fn threads_support() {
assert_eq!(check_ignore(&config, "//@ needs-threads"), !has_threads)
}
}

fn run_path(poisoned: &mut bool, path: &Path, buf: &[u8]) {
let rdr = std::io::Cursor::new(&buf);
iter_header(Mode::Ui, "ui", poisoned, path, rdr, &mut |_| {});
}

#[test]
fn test_unknown_directive_check() {
let mut poisoned = false;
run_path(
&mut poisoned,
Path::new("a.rs"),
include_bytes!("./test-auxillary/unknown_directive.rs"),
);
assert!(poisoned);
}

#[test]
fn test_known_legacy_directive_check() {
let mut poisoned = false;
run_path(
&mut poisoned,
Path::new("a.rs"),
include_bytes!("./test-auxillary/known_legacy_directive.rs"),
);
assert!(poisoned);
}

#[test]
fn test_known_directive_check_no_error() {
let mut poisoned = false;
run_path(
&mut poisoned,
Path::new("a.rs"),
include_bytes!("./test-auxillary/known_directive.rs"),
);
assert!(!poisoned);
}

#[test]
fn test_error_annotation_no_error() {
let mut poisoned = false;
run_path(
&mut poisoned,
Path::new("a.rs"),
include_bytes!("./test-auxillary/error_annotation.rs"),
);
assert!(!poisoned);
}

#[test]
fn test_non_rs_unknown_directive_not_checked() {
let mut poisoned = false;
run_path(
&mut poisoned,
Path::new("a.Makefile"),
include_bytes!("./test-auxillary/not_rs.Makefile"),
);
assert!(!poisoned);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

// The nll_beyond revision is disabled due to missing support from two-phase beyond autorefs
//@[nll_beyond]compile-flags: -Z two-phase-beyond-autoref
//[nll_beyond]should-fail
//@[nll_beyond]should-fail

// This is a corner case that the current implementation is (probably)
// treating more conservatively than is necessary. But it also does
Expand Down
Loading
Loading