Skip to content
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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,18 @@ This project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
writes too. Rendered HTML is unchanged - every affected spelling parses to
the same document either way.

- **An ingest refusal at a typed node union names the admitted types instead of
a field from the first branch.** A payload putting the wrong KIND of node at
`figure.target` was refused, correctly, and then described the wrong problem:
it reported the required property of whichever branch the validator happened
to try first, so a node of an inadmissible type was reported as an `image`
missing its `src`. A producer reading that would have added `src` to a
heading. The message now names the offending type and the admitted set, which is
what carve-js says about the same payload. A node whose type IS admitted and
which is missing a required field still names that field, so the change adds a
story rather than replacing one, and no payload changes from accepted to
refused or back.

- **`carve fmt` writes a bare caret where no inline note can open.** `^[` opens
a note only where a note can form, and PART 9 §16 rules out three positions:
an empty or whitespace-only body, an unclosed run, and anywhere inside a
Expand Down
75 changes: 74 additions & 1 deletion src/Ast/AstSchema.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
use function array_key_exists;
use function dirname;
use function file_get_contents;
use function implode;
use function in_array;
use function is_array;
use function is_bool;
use function is_float;
use function is_int;
use function is_string;
use function json_decode;
use function sort;
use function sprintf;
use function str_starts_with;
use function substr;
Expand Down Expand Up @@ -260,7 +262,7 @@ private static function checkComposition(mixed $value, array $schema, array $roo
// A branch list is never EMPTY here - same assertion - so a failure
// was recorded whenever none matched.
if ($matched === 0 && $first !== null) {
return $first;
return self::typedNodeUnionMismatch($value, $schema[$keyword], $root, $path) ?? $first;
}
}

Expand All @@ -273,6 +275,77 @@ private static function checkComposition(mixed $value, array $schema, array $roo
return null;
}

/**
* Why the value's own TYPE is not admitted here, when that is the reason
* no branch matched. Null whenever it is not, so the caller keeps the
* branch failure it already has.
*
* A union of typed node definitions - `figure.target`, `blockNode`,
* `inlineNode` - fails in two different ways, and the first branch's own
* complaint tells only one of them. A node whose type IS admitted but which
* is missing a field it requires wants that field named. A node of a type
* the position never admits wants the ADMITTED SET named: reporting the
* first branch's missing `src` for a `block_quote` at `figure.target` sends
* a producer to add `src` to a block quote.
*
* Both conditions are required before a message is built: the value has to
* identify itself as a node, and every branch has to pin a type constant.
* Otherwise the union is something else - a union of records, a mixed one -
* and this has nothing to say about it.
*
* @param mixed $value
* @param array<mixed> $branches
* @param array<string, mixed> $root
* @param string $path
*
* @return string|null
*/
private static function typedNodeUnionMismatch(mixed $value, array $branches, array $root, string $path): ?string
{
if (!is_array($value)) {
return null;
}
$type = $value['type'] ?? null;
if (!is_string($type)) {
return null;
}

// THE THREE `return null`s BELOW ARE TYPE NARROWING, not guards against a
// schema this repo ships. Both unions the published schema writes today -
// `figure.target` and `definition_list.items` - are typed node unions, so
// the branch shapes always resolve; the checks exist because the values
// come out of decoded JSON as `mixed` and the function must be total for a
// union some later schema writes differently. They are therefore not
// reachable from any payload, which is why the tests do not cover them.
$admitted = [];
foreach ($branches as $branch) {
/** @var array<mixed> $branch */
if (isset($branch['$ref']) && is_string($branch['$ref'])) {
$branch = self::resolve($branch['$ref'], $root);
}
$properties = $branch['properties'] ?? null;
if (!is_array($properties)) {
return null;
}
$declared = $properties['type'] ?? null;
if (!is_array($declared)) {
return null;
}
$constant = $declared['const'] ?? null;
if (!is_string($constant)) {
return null;
}
$admitted[] = $constant;
}

if (in_array($type, $admitted, true)) {
return null;
}
sort($admitted);

return sprintf('%s holds a "%s" node where the schema admits only %s', $path, $type, implode(', ', $admitted));
}

/**
* `properties`, `additionalProperties` and `items`.
*
Expand Down
90 changes: 86 additions & 4 deletions tests/TestCase/Ast/PayloadIsValidatedAgainstTheSchemaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,9 @@ static function (array $d): array {
'the AST schema does not name: ',
],
// A `oneOf` that NO branch satisfies. `figure.target` is one of an
// image, a quote, a table, a code block or a paragraph; a heading
// is none of them, and the report has to say something rather than
// fall through as a match.
// image, a table, a code block or a paragraph; a heading is none
// of them, and the report names the admitted node types rather
// than a required field from whichever branch happens to be first.
'a figure target that is none of its alternatives' => [
static function (array $d): array {
$d['children'][0] = [
Expand All @@ -263,7 +263,7 @@ static function (array $d): array {

return $d;
},
'target',
'$.children[0].target holds a "heading" node where the schema admits only block_quote, code_block, image, paragraph, table',
],
'a type the vocabulary does not hold' => [
static function (array $d): array {
Expand All @@ -287,6 +287,88 @@ public function testTheDocumentTheShapesAreBuiltFromIsValid(): void
$this->assertSame("<p>a</p>\n", (new HtmlRenderer())->render($document));
}

/**
* A HEADING is the example on purpose: it is not a captionable host under any
* version of the clause, so this case does not move when the admitted set does.
* A `block_quote` would have read better and was rejected for exactly that
* reason: markup-carve/carve#1161 removed it from the set and
* markup-carve/carve#1213 has since put it back, so a case built on it
* would have asserted the opposite of the pinned schema within days. The
* admitted set in the expectation moves with the pin; the refused type
* does not.
*/
public function testFigureTargetReportsARefusedNodeTypeAndEveryAdmittedType(): void
{
$payload = self::valid();
$payload['children'][0] = [
'type' => 'figure',
'target' => [
'type' => 'heading',
'level' => 1,
'children' => [],
],
'caption' => [],
];

$violation = AstSchema::firstViolation($payload);

$this->assertNotNull($violation);
$this->assertStringContainsString('holds a "heading" node where the schema admits only', (string)$violation);
$this->assertStringContainsString('$.children[0].target', (string)$violation);
$this->assertStringNotContainsString('src', (string)$violation);

try {
(new AstCodec())->decode($payload);
$this->fail('the decoder accepted a heading as a figure target');
} catch (AstDecodeException $e) {
$this->assertStringContainsString((string)$violation, $e->getMessage());
$this->assertStringContainsString('PART 12 §12(d)', $e->getMessage());
$this->assertStringNotContainsString('src', $e->getMessage());
}
}

public function testFigureMayTargetACompleteImage(): void
{
$payload = self::valid();
$payload['children'][0] = [
'type' => 'figure',
'target' => ['type' => 'image', 'src' => 'figure.png', 'alt' => 'Figure'],
'caption' => [],
];

$this->assertNull(AstSchema::firstViolation($payload));
}

public function testFigureTargetReportsAMissingFieldForAnAdmittedNodeType(): void
{
$payload = self::valid();
$payload['children'][0] = [
'type' => 'figure',
'target' => ['type' => 'image'],
'caption' => [],
];

$this->assertSame(
'$.children[0].target is missing `src`, which the schema requires',
AstSchema::firstViolation($payload),
);
}

public function testCompositionWithoutAStringNodeTypeKeepsTheFirstBranchFailure(): void
{
$payload = self::valid();
$payload['children'][0] = [
'type' => 'figure',
'target' => [],
'caption' => [],
];

$this->assertSame(
'$.children[0].target is missing `type`, which the schema requires',
AstSchema::firstViolation($payload),
);
}

/**
* The premise carve#881 signed off on, measured against the copy THIS
* engine reads rather than against the spec repo's.
Expand Down
Loading