Rollup merge of rust-lang#123753 - Urgau:compiletest-trailing-directive, r=jieyouxu

compiletest: error when finding a trailing directive

This PR introduce a supplementary check that when checking for a compiletest directive, will also check that the next "word" after that directive is not also by itself a directive.

This is done to avoid situations like this `//@ only-linux only-x86_64` where one might think that both directives are being taken into account while in fact the second in a comment, and so was ignored, until now.

Related to rust-lang#123730
cc `@scottmcm`
r? `@jieyouxu`

Author: matthiaskrgr

src/tools/compiletest/src/header.rs

@@ -935,16 +935,25 @@ struct HeaderLine<'ln> {
 pub(crate) struct CheckDirectiveResult<'ln> {
     is_known_directive: bool,
     directive_name: &'ln str,
+    trailing_directive: Option<&'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);
+    let (directive_name, post) = directive_ln.split_once([':', ' ']).unwrap_or((directive_ln, ""));
+
+    let trailing = post.trim().split_once(' ').map(|(pre, _)| pre).unwrap_or(post);
+    let trailing_directive = {
+        // 1. is the directive name followed by a space? (to exclude `:`)
+        matches!(directive_ln.get(directive_name.len()..), Some(s) if s.starts_with(" "))
+            // 2. is what is after that directive also a directive (ex: "only-x86 only-arm")
+            && KNOWN_DIRECTIVE_NAMES.contains(&trailing)
+    }
+    .then_some(trailing);
 
     CheckDirectiveResult {
         is_known_directive: KNOWN_DIRECTIVE_NAMES.contains(&directive_name),
         directive_name: directive_ln,
+        trailing_directive,
     }
 }
 
@@ -1014,7 +1023,8 @@ fn iter_header(
             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);
+                let CheckDirectiveResult { is_known_directive, trailing_directive, .. } =
+                    check_directive(directive_ln);
 
                 if !is_known_directive {
                     *poisoned = true;
@@ -1028,6 +1038,21 @@ fn iter_header(
 
                     return;
                 }
+
+                if let Some(trailing_directive) = &trailing_directive {
+                    *poisoned = true;
+
+                    eprintln!(
+                        "error: detected trailing compiletest test directive `{}` in {}:{}\n \
+                          help: put the trailing directive in it's own line: `//@ {}`",
+                        trailing_directive,
+                        testfile.display(),
+                        line_number,
+                        trailing_directive,
+                    );
+
+                    return;
+                }
             }
 
             it(HeaderLine {
@@ -1051,7 +1076,8 @@ fn iter_header(
 
             let rest = rest.trim_start();
 
-            let CheckDirectiveResult { is_known_directive, directive_name } = check_directive(rest);
+            let CheckDirectiveResult { is_known_directive, directive_name, .. } =
+                check_directive(rest);
 
             if is_known_directive {
                 *poisoned = true;

src/tools/compiletest/src/header/tests.rs

@@ -667,3 +667,24 @@ fn test_non_rs_unknown_directive_not_checked() {
     );
     assert!(!poisoned);
 }
+
+#[test]
+fn test_trailing_directive() {
+    let mut poisoned = false;
+    run_path(&mut poisoned, Path::new("a.rs"), b"//@ only-x86 only-arm");
+    assert!(poisoned);
+}
+
+#[test]
+fn test_trailing_directive_with_comment() {
+    let mut poisoned = false;
+    run_path(&mut poisoned, Path::new("a.rs"), b"//@ only-x86   only-arm with comment");
+    assert!(poisoned);
+}
+
+#[test]
+fn test_not_trailing_directive() {
+    let mut poisoned = false;
+    run_path(&mut poisoned, Path::new("a.rs"), b"//@ revisions: incremental");
+    assert!(!poisoned);
+}