Skip to content

Commit

Permalink
Auto merge of #127968 - fmease:upd-jsondocck-directive-style, r=Guill…
Browse files Browse the repository at this point in the history
…aumeGomez

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`
  • Loading branch information
bors committed Jul 19, 2024
2 parents ff4b398 + 633f41d commit 9057c3f
Show file tree
Hide file tree
Showing 131 changed files with 1,095 additions and 1,090 deletions.
1 change: 1 addition & 0 deletions src/etc/htmldocck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'''
Expand Down
31 changes: 22 additions & 9 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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.
///
Expand Down Expand Up @@ -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 `:`)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
67 changes: 29 additions & 38 deletions src/tools/jsondocck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>.*)$
"#,
Expand All @@ -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>, ()> {
Expand All @@ -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 {
Expand Down Expand Up @@ -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!(
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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
);
Expand All @@ -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,
);
}
Expand All @@ -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(" ")
),
Expand Down
24 changes: 12 additions & 12 deletions tests/rustdoc-json/assoc_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
8 changes: 4 additions & 4 deletions tests/rustdoc-json/assoc_type.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
6 changes: 3 additions & 3 deletions tests/rustdoc-json/blanket_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
4 changes: 2 additions & 2 deletions tests/rustdoc-json/doc_hidden_failure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions tests/rustdoc-json/enums/discriminant/basic.rs
Original file line number Diff line number Diff line change
@@ -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,
}
Loading

0 comments on commit 9057c3f

Please sign in to comment.