feat: support unevaluatedProperties in Draft 2019-09 - #931
Conversation
|
@tomatotomata ill be back from |
DannyvdSluijs
left a comment
DannyvdSluijs
left a comment
There was a problem hiding this comment.
Really nice work. I didn't think about this solution direction. Refreshing to see your approach.
I did add some review comments could you perhaps take a look and see if you can contribute?
| use JsonSchema\Entity\JsonPointer; | ||
|
|
||
| /** | ||
| * Proof-of-concept support for unevaluatedProperties. |
There was a problem hiding this comment.
suggestion: Before merging we should remove the proof on concept comment.
| * Proof-of-concept support for unevaluatedProperties. |
| * The current validator does not carry annotations between applicators, so this | ||
| * first implementation derives the evaluated property names from properties, | ||
| * patternProperties, and allOf branches in the current schema. |
There was a problem hiding this comment.
suggestion: The comment doesn't need to inform about this being the first implementation
| * The current validator does not carry annotations between applicators, so this | |
| * first implementation derives the evaluated property names from properties, | |
| * patternProperties, and allOf branches in the current schema. | |
| * The current validator does not carry annotations between applicators, so this | |
| * constraint derives the evaluated property names from properties, | |
| * patternProperties, and allOf branches in the current schema. |
There was a problem hiding this comment.
nitpick: I think this file would belong in test/Constraints/Drafts/Draft2019/UnevaluatedPropertiesConstraintTest.php
| /** | ||
| * @param object $schema | ||
| * @param object $value | ||
| * | ||
| * @return array<int, string> | ||
| */ | ||
| private function collectEvaluatedProperties($schema, object $value): array | ||
| { | ||
| if (!is_object($schema)) { | ||
| return []; | ||
| } | ||
|
|
There was a problem hiding this comment.
suggestion: Since $object was already verified as being of type object on line 43 we can safely narrow the type of the function to object and skip the check as well as the doc types since we use the native typing. This requires a additional check on line 91 where we do a recursive call.
| /** | |
| * @param object $schema | |
| * @param object $value | |
| * | |
| * @return array<int, string> | |
| */ | |
| private function collectEvaluatedProperties($schema, object $value): array | |
| { | |
| if (!is_object($schema)) { | |
| return []; | |
| } | |
| /** | |
| * @return array<int, string> | |
| */ | |
| private function collectEvaluatedProperties(object $schema, object $value): array | |
| { |
There was a problem hiding this comment.
issue: With this keyword now supported we should alos enable the test in
json-schema/tests/JsonSchemaTestSuiteTest.php
Lines 206 to 245 in bbe268a
| if ($schema->unevaluatedProperties === true) { | ||
| return; | ||
| } | ||
|
|
| foreach ($schema->allOf as $branch) { | ||
| $evaluated = array_merge($evaluated, $this->collectEvaluatedProperties($branch, $value)); | ||
| } | ||
| } |
There was a problem hiding this comment.
question: This should include (either now or at a later state) anyOf, oneOf, if/then/else and more keywords right? Curious to learn what would be your suggested approach?
2 participants
Summary
This adds a focused proof of concept for
unevaluatedPropertiesin Draft 2019-09, following the scope discussed in #907.The implementation derives evaluated property names from
properties,patternProperties, and directallOfbranches. That covers the motivating composition while keeping the first patch small. WhenunevaluatedPropertiesis a schema, each remaining property is validated against it. When it isfalse, the validator reports the property with a dedicated constraint error.The validator does not yet carry annotations across every applicator, so this intentionally does not claim complete Draft 2019-09 semantics for
anyOf,oneOf, conditionals, or referenced schemas. I would treat this as a proof of concept for the project to shape further.Tests
php -d extension=mbstring -d extension=openssl vendor/bin/phpunit --filter UnevaluatedPropertiesTest --testdoxpasses locally (2 assertions-backed cases, 2 expected skips for the assoc/type-cast variants).git diff --checkpasses.This patch was prepared with AI assistance and then reviewed and tested in this checkout. The repository's contribution notice asks contributors to have authored 100% of the content, so please let me know if the project needs a different treatment of that requirement before considering the patch.
Updates #907