Auto merge of #127968 - fmease:upd-jsondocck-directive-style, r=GuillaumeGomez

Update jsondocck directives to follow ui_test-style

Context: Comment chain in #125813.
Follow-up to #126788.
Use the same temporary approach of "double parsing" until we figure out how we want to support compiletest/ui_test directive "add-ons" for child test runners like HtmlDocCk and JsonDocCk.

I didn't touch much of jsondocck because I want to refactor it some other time (for robustness, maintainability and better diagnostics; basically by following a similar design of my WIP HtmlDocCk-next, cc #125780).

r? `@GuillaumeGomez`

Author: bors

Diff for: src/etc/htmldocck.py

@@ -266,6 +266,7 @@ def filter_line(line):
 # To prevent duplicating the list of commmands between `compiletest` and `htmldocck`, we put
 # it into a common file which is included in rust code and parsed here.
 # FIXME: This setup is temporary until we figure out how to improve this situation.
+#        See <https://github.com/rust-lang/rust/issues/125813#issuecomment-2141953780>.
 KNOWN_DIRECTIVE_NAMES = get_known_directive_names()
 
 LINE_PATTERN = re.compile(r'''

Diff for: src/tools/compiletest/src/header.rs

@@ -723,12 +723,13 @@ pub fn line_directive<'line>(
     }
 }
 
-// To prevent duplicating the list of commmands between `compiletest` and `htmldocck`, we put
-// it into a common file which is included in rust code and parsed here.
+// To prevent duplicating the list of commmands between `compiletest`,`htmldocck` and `jsondocck`,
+// we put it into a common file which is included in rust code and parsed here.
 // FIXME: This setup is temporary until we figure out how to improve this situation.
+//        See <https://github.com/rust-lang/rust/issues/125813#issuecomment-2141953780>.
 include!("command-list.rs");
 
-const KNOWN_RUSTDOC_DIRECTIVE_NAMES: &[&str] = &[
+const KNOWN_HTMLDOCCK_DIRECTIVE_NAMES: &[&str] = &[
     "count",
     "!count",
     "files",
@@ -747,6 +748,9 @@ const KNOWN_RUSTDOC_DIRECTIVE_NAMES: &[&str] = &[
     "!snapshot",
 ];
 
+const KNOWN_JSONDOCCK_DIRECTIVE_NAMES: &[&str] =
+    &["count", "!count", "has", "!has", "is", "!is", "ismany", "!ismany", "set", "!set"];
+
 /// The broken-down contents of a line containing a test header directive,
 /// which [`iter_header`] passes to its callback function.
 ///
@@ -783,17 +787,26 @@ pub(crate) struct CheckDirectiveResult<'ln> {
 
 pub(crate) fn check_directive<'a>(
     directive_ln: &'a str,
-    is_rustdoc: bool,
+    mode: Mode,
     original_line: &str,
 ) -> CheckDirectiveResult<'a> {
     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 is_known = |s: &str| {
         KNOWN_DIRECTIVE_NAMES.contains(&s)
-            || (is_rustdoc
-                && original_line.starts_with("//@")
-                && KNOWN_RUSTDOC_DIRECTIVE_NAMES.contains(&s))
+            || match mode {
+                Mode::Rustdoc | Mode::RustdocJson => {
+                    original_line.starts_with("//@")
+                        && match mode {
+                            Mode::Rustdoc => KNOWN_HTMLDOCCK_DIRECTIVE_NAMES,
+                            Mode::RustdocJson => KNOWN_JSONDOCCK_DIRECTIVE_NAMES,
+                            _ => unreachable!(),
+                        }
+                        .contains(&s)
+                }
+                _ => false,
+            }
     };
     let trailing_directive = {
         // 1. is the directive name followed by a space? (to exclude `:`)
@@ -875,7 +888,7 @@ fn iter_header(
                 let directive_ln = non_revisioned_directive_line.trim();
 
                 let CheckDirectiveResult { is_known_directive, trailing_directive, .. } =
-                    check_directive(directive_ln, mode == Mode::Rustdoc, ln);
+                    check_directive(directive_ln, mode, ln);
 
                 if !is_known_directive {
                     *poisoned = true;
@@ -928,7 +941,7 @@ fn iter_header(
             let rest = rest.trim_start();
 
             let CheckDirectiveResult { is_known_directive, directive_name, .. } =
-                check_directive(rest, mode == Mode::Rustdoc, ln);
+                check_directive(rest, mode, ln);
 
             if is_known_directive {
                 *poisoned = true;

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

@@ -66,14 +66,17 @@ impl CommandKind {
         };
 
         if !count {
-            print_err(&format!("Incorrect number of arguments to `@{}`", self), lineno);
+            print_err(&format!("Incorrect number of arguments to `{}`", self), lineno);
             return false;
         }
 
         if let CommandKind::Count = self {
             if args[1].parse::<usize>().is_err() {
                 print_err(
-                    &format!("Second argument to @count must be a valid usize (got `{}`)", args[1]),
+                    &format!(
+                        "Second argument to `count` must be a valid usize (got `{}`)",
+                        args[1]
+                    ),
                     lineno,
                 );
                 return false;
@@ -101,7 +104,8 @@ static LINE_PATTERN: OnceLock<Regex> = OnceLock::new();
 fn line_pattern() -> Regex {
     RegexBuilder::new(
         r#"
-        \s(?P<invalid>!?)@(?P<negated>!?)
+        //@\s+
+        (?P<negated>!?)
         (?P<cmd>[A-Za-z]+(?:-[A-Za-z]+)*)
         (?P<args>.*)$
     "#,
@@ -116,6 +120,10 @@ fn print_err(msg: &str, lineno: usize) {
     eprintln!("Invalid command: {} on line {}", msg, lineno)
 }
 
+// FIXME: This setup is temporary until we figure out how to improve this situation.
+//        See <https://github.com/rust-lang/rust/issues/125813#issuecomment-2141953780>.
+include!(concat!(env!("CARGO_MANIFEST_DIR"), "/../compiletest/src/command-list.rs"));
+
 /// Get a list of commands from a file. Does the work of ensuring the commands
 /// are syntactically valid.
 fn get_commands(template: &str) -> Result<Vec<Command>, ()> {
@@ -132,36 +140,22 @@ fn get_commands(template: &str) -> Result<Vec<Command>, ()> {
         };
 
         let negated = cap.name("negated").unwrap().as_str() == "!";
-        let cmd = cap.name("cmd").unwrap().as_str();
 
-        let cmd = match cmd {
+        let cmd = match cap.name("cmd").unwrap().as_str() {
             "has" => CommandKind::Has,
             "count" => CommandKind::Count,
             "is" => CommandKind::Is,
             "ismany" => CommandKind::IsMany,
             "set" => CommandKind::Set,
-            _ => {
-                print_err(&format!("Unrecognized command name `@{}`", cmd), lineno);
+            // FIXME: See the comment above the `include!(...)`.
+            cmd if KNOWN_DIRECTIVE_NAMES.contains(&cmd) => continue,
+            cmd => {
+                print_err(&format!("Unrecognized command name `{cmd}`"), lineno);
                 errors = true;
                 continue;
             }
         };
 
-        if let Some(m) = cap.name("invalid") {
-            if m.as_str() == "!" {
-                print_err(
-                    &format!(
-                        "`!@{0}{1}`, (help: try with `@!{1}`)",
-                        if negated { "!" } else { "" },
-                        cmd,
-                    ),
-                    lineno,
-                );
-                errors = true;
-                continue;
-            }
-        }
-
         let args = cap.name("args").map_or(Some(vec![]), |m| shlex::split(m.as_str()));
 
         let args = match args {
@@ -197,19 +191,19 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> {
     let result = match command.kind {
         CommandKind::Has => {
             match command.args.len() {
-                // @has <jsonpath> = check path exists
+                // `has <jsonpath>`: Check that `jsonpath` exists.
                 1 => {
                     let val = cache.value();
                     let results = select(val, &command.args[0]).unwrap();
                     !results.is_empty()
                 }
-                // @has <jsonpath> <value> = check *any* item matched by path equals value
+                // `has <jsonpath> <value>`: Check *any* item matched by `jsonpath` equals `value`.
                 2 => {
                     let val = cache.value().clone();
                     let results = select(&val, &command.args[0]).unwrap();
                     let pat = string_to_value(&command.args[1], cache);
                     let has = results.contains(&pat.as_ref());
-                    // Give better error for when @has check fails
+                    // Give better error for when `has` check fails.
                     if !command.negated && !has {
                         return Err(CkError::FailedCheck(
                             format!(
@@ -227,16 +221,16 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> {
                 _ => unreachable!(),
             }
         }
+        // `ismany <path> <jsonpath> <value...>`
         CommandKind::IsMany => {
-            // @ismany <path> <jsonpath> <value>...
+            assert!(!command.negated, "`ismany` may not be negated");
             let (query, values) = if let [query, values @ ..] = &command.args[..] {
                 (query, values)
             } else {
                 unreachable!("Checked in CommandKind::validate")
             };
             let val = cache.value();
             let got_values = select(val, &query).unwrap();
-            assert!(!command.negated, "`@!ismany` is not supported");
 
             // Serde json doesn't implement Ord or Hash for Value, so we must
             // use a Vec here. While in theory that makes setwize equality
@@ -265,8 +259,8 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> {
             }
             true
         }
+        // `count <jsonpath> <count>`: Check that `jsonpath` matches exactly `count` times.
         CommandKind::Count => {
-            // @count <jsonpath> <count> = Check that the jsonpath matches exactly [count] times
             assert_eq!(command.args.len(), 2);
             let expected: usize = command.args[1].parse().unwrap();
             let val = cache.value();
@@ -287,8 +281,8 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> {
                 eq
             }
         }
+        // `has <jsonpath> <value>`: Check` *exactly one* item matched by `jsonpath`, and it equals `value`.
         CommandKind::Is => {
-            // @has <jsonpath> <value> = check *exactly one* item matched by path, and it equals value
             assert_eq!(command.args.len(), 2);
             let val = cache.value().clone();
             let results = select(&val, &command.args[0]).unwrap();
@@ -308,16 +302,17 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> {
                 is
             }
         }
+        // `set <name> = <jsonpath>`
         CommandKind::Set => {
-            // @set <name> = <jsonpath>
+            assert!(!command.negated, "`set` may not be negated");
             assert_eq!(command.args.len(), 3);
             assert_eq!(command.args[1], "=", "Expected an `=`");
             let val = cache.value().clone();
             let results = select(&val, &command.args[2]).unwrap();
             assert_eq!(
                 results.len(),
                 1,
-                "Expected 1 match for `{}` (because of @set): matched to {:?}",
+                "Expected 1 match for `{}` (because of `set`): matched to {:?}",
                 command.args[2],
                 results
             );
@@ -330,7 +325,7 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> {
                 }
                 _ => {
                     panic!(
-                        "Got multiple results in `@set` for `{}`: {:?}",
+                        "Got multiple results in `set` for `{}`: {:?}",
                         &command.args[2], results,
                     );
                 }
@@ -341,18 +336,14 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> {
     if result == command.negated {
         if command.negated {
             Err(CkError::FailedCheck(
-                format!(
-                    "`@!{} {}` matched when it shouldn't",
-                    command.kind,
-                    command.args.join(" ")
-                ),
+                format!("`!{} {}` matched when it shouldn't", command.kind, command.args.join(" ")),
                 command,
             ))
         } else {
             // FIXME: In the future, try 'peeling back' each step, and see at what level the match failed
             Err(CkError::FailedCheck(
                 format!(
-                    "`@{} {}` didn't match when it should",
+                    "`{} {}` didn't match when it should",
                     command.kind,
                     command.args.join(" ")
                 ),

Diff for: tests/rustdoc-json/assoc_items.rs

@@ -3,32 +3,32 @@
 pub struct Simple;
 
 impl Simple {
-    // @has "$.index[*][?(@.name=='CONSTANT')].inner.assoc_const"
+    //@ has "$.index[*][?(@.name=='CONSTANT')].inner.assoc_const"
     pub const CONSTANT: usize = 0;
 }
 
 pub trait EasyToImpl {
-    // @has "$.index[*][?(@.docs=='ToDeclare trait')].inner.assoc_type"
-    // @is "$.index[*][?(@.docs=='ToDeclare trait')].inner.assoc_type.default" null
-    // @is "$.index[*][?(@.docs=='ToDeclare trait')].inner.assoc_type.bounds" []
+    //@ has "$.index[*][?(@.docs=='ToDeclare trait')].inner.assoc_type"
+    //@ is "$.index[*][?(@.docs=='ToDeclare trait')].inner.assoc_type.default" null
+    //@ is "$.index[*][?(@.docs=='ToDeclare trait')].inner.assoc_type.bounds" []
     /// ToDeclare trait
     type ToDeclare;
-    // @has "$.index[*][?(@.docs=='AN_ATTRIBUTE trait')].inner.assoc_const"
-    // @is "$.index[*][?(@.docs=='AN_ATTRIBUTE trait')].inner.assoc_const.default" null
-    // @is "$.index[*][?(@.docs=='AN_ATTRIBUTE trait')].inner.assoc_const.type.primitive" '"usize"'
+    //@ has "$.index[*][?(@.docs=='AN_ATTRIBUTE trait')].inner.assoc_const"
+    //@ is "$.index[*][?(@.docs=='AN_ATTRIBUTE trait')].inner.assoc_const.default" null
+    //@ is "$.index[*][?(@.docs=='AN_ATTRIBUTE trait')].inner.assoc_const.type.primitive" '"usize"'
     /// AN_ATTRIBUTE trait
     const AN_ATTRIBUTE: usize;
 }
 
 impl EasyToImpl for Simple {
-    // @has "$.index[*][?(@.docs=='ToDeclare impl')].inner.assoc_type"
-    // @is "$.index[*][?(@.docs=='ToDeclare impl')].inner.assoc_type.default.primitive" \"usize\"
+    //@ has "$.index[*][?(@.docs=='ToDeclare impl')].inner.assoc_type"
+    //@ is "$.index[*][?(@.docs=='ToDeclare impl')].inner.assoc_type.default.primitive" \"usize\"
     /// ToDeclare impl
     type ToDeclare = usize;
 
-    // @has "$.index[*][?(@.docs=='AN_ATTRIBUTE impl')].inner.assoc_const"
-    // @is "$.index[*][?(@.docs=='AN_ATTRIBUTE impl')].inner.assoc_const.type.primitive" \"usize\"
-    // @is "$.index[*][?(@.docs=='AN_ATTRIBUTE impl')].inner.assoc_const.default" \"12\"
+    //@ has "$.index[*][?(@.docs=='AN_ATTRIBUTE impl')].inner.assoc_const"
+    //@ is "$.index[*][?(@.docs=='AN_ATTRIBUTE impl')].inner.assoc_const.type.primitive" \"usize\"
+    //@ is "$.index[*][?(@.docs=='AN_ATTRIBUTE impl')].inner.assoc_const.default" \"12\"
     /// AN_ATTRIBUTE impl
     const AN_ATTRIBUTE: usize = 12;
 }

Diff for: tests/rustdoc-json/assoc_type.rs

@@ -1,9 +1,9 @@
 // Regression test for <https://github.com/rust-lang/rust/issues/98547>.
 
-// @has "$.index[*][?(@.name=='Trait')]"
-// @has "$.index[*][?(@.name=='AssocType')]"
-// @has "$.index[*][?(@.name=='S')]"
-// @has "$.index[*][?(@.name=='S2')]"
+//@ has "$.index[*][?(@.name=='Trait')]"
+//@ has "$.index[*][?(@.name=='AssocType')]"
+//@ has "$.index[*][?(@.name=='S')]"
+//@ has "$.index[*][?(@.name=='S2')]"
 
 pub trait Trait {
     type AssocType;

Diff for: tests/rustdoc-json/blanket_impls.rs

@@ -2,7 +2,7 @@
 
 #![no_std]
 
-// @has "$.index[*][?(@.name=='Error')].inner.assoc_type"
-// @has "$.index[*][?(@.name=='Error')].inner.assoc_type.default.resolved_path"
-// @has "$.index[*][?(@.name=='Error')].inner.assoc_type.default.resolved_path.name" \"Infallible\"
+//@ has "$.index[*][?(@.name=='Error')].inner.assoc_type"
+//@ has "$.index[*][?(@.name=='Error')].inner.assoc_type.default.resolved_path"
+//@ has "$.index[*][?(@.name=='Error')].inner.assoc_type.default.resolved_path.name" \"Infallible\"
 pub struct ForBlanketTryFromImpl;

Diff for: tests/rustdoc-json/doc_hidden_failure.rs

@@ -11,8 +11,8 @@ mod auto {
     }
 }
 
-// @count "$.index[*][?(@.name=='builders')]" 1
-// @has "$.index[*][?(@.name == 'ActionRowBuilder')"]
+//@ count "$.index[*][?(@.name=='builders')]" 1
+//@ has "$.index[*][?(@.name == 'ActionRowBuilder')"]
 pub use auto::*;
 
 pub mod builders {

Diff for: tests/rustdoc-json/enums/discriminant/basic.rs

@@ -1,12 +1,12 @@
 #[repr(i8)]
 pub enum Ordering {
-    // @is "$.index[*][?(@.name=='Less')].inner.variant.discriminant.expr" '"-1"'
-    // @is "$.index[*][?(@.name=='Less')].inner.variant.discriminant.value" '"-1"'
+    //@ is "$.index[*][?(@.name=='Less')].inner.variant.discriminant.expr" '"-1"'
+    //@ is "$.index[*][?(@.name=='Less')].inner.variant.discriminant.value" '"-1"'
     Less = -1,
-    // @is "$.index[*][?(@.name=='Equal')].inner.variant.discriminant.expr" '"0"'
-    // @is "$.index[*][?(@.name=='Equal')].inner.variant.discriminant.value" '"0"'
+    //@ is "$.index[*][?(@.name=='Equal')].inner.variant.discriminant.expr" '"0"'
+    //@ is "$.index[*][?(@.name=='Equal')].inner.variant.discriminant.value" '"0"'
     Equal = 0,
-    // @is "$.index[*][?(@.name=='Greater')].inner.variant.discriminant.expr" '"1"'
-    // @is "$.index[*][?(@.name=='Greater')].inner.variant.discriminant.value" '"1"'
+    //@ is "$.index[*][?(@.name=='Greater')].inner.variant.discriminant.expr" '"1"'
+    //@ is "$.index[*][?(@.name=='Greater')].inner.variant.discriminant.value" '"1"'
     Greater = 1,
 }