Skip to content

Commit

Permalink
Error on invalid compiletest directives in Rust test files
Browse files Browse the repository at this point in the history
  • Loading branch information
jieyouxu committed Mar 9, 2024
1 parent b054da8 commit 2dd05e8
Show file tree
Hide file tree
Showing 7 changed files with 171 additions and 31 deletions.
127 changes: 96 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,6 +694,7 @@ 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",
Expand All @@ -716,6 +718,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 +734,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 +769,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 +832,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[
"only-aarch64",
"only-arm",
"only-avr",
"only-beta",
"only-bpf",
"only-cdb",
"only-gnu",
Expand All @@ -818,13 +850,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 +880,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 +911,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 +966,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 +985,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 +1030,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);
}

0 comments on commit 2dd05e8

Please sign in to comment.