Conversation
c07108a to
37fb6b3
Compare
8ecd057 to
bf51cde
Compare
bf51cde to
a9721b1
Compare
|
@DannyvdSluijs, I'm interested in If-Then-Else (https://json-schema.org/understanding-json-schema/reference/conditionals#ifthenelse) and it seems draft-7 is supporting it. I would like to help but I'm not so sure about the nature of test failures. Are the validation errors to just not-adapted test expectations? |
c1b1a0a to
d978f28
Compare
|
@claudiu-cristea I just need to find time to make progress with this PR. I you want to help you can check the test output and see which ones are failing and figure out what would be a good approach to fix them. I've just taken some time to fix the tests related to the time formatting and the remaining non-optional tests |
…ema when referenced in schemas
There was a problem hiding this comment.
Pull request overview
This PR adds support for JSON Schema Draft-07 with strict mode validation. It implements a complete set of Draft-07 constraint validators following the same architectural pattern established for Draft-06.
Changes:
- Added Draft-07 constraint implementations with all required keyword validators
- Extended DraftIdentifiers enum to support Draft-07 schema URIs
- Updated test infrastructure to run JSON Schema Test Suite tests for Draft-07
- Added support for Draft-07 specific features: if/then/else, contentMediaType, and contentEncoding
Reviewed changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/JsonSchema/DraftIdentifiers.php | Added DRAFT_7 constant and updated mapping methods to support draft07 constraint name conversion |
| src/JsonSchema/Constraints/Factory.php | Registered draft07 constraint handler in the constraint map |
| src/JsonSchema/Uri/UriRetriever.php | Updated regex pattern to include draft-07 schema URI |
| src/JsonSchema/SchemaStorage.php | Enhanced scanForSubschemas to handle array-type subschemas |
| src/JsonSchema/ConstraintError.php | Added CONTENT_MEDIA_TYPE and CONTENT_ENCODING error constants |
| tests/ValidatorTest.php | Updated test to exclude Draft-07 from non-strict mode validation exceptions |
| tests/JsonSchemaTestSuiteTest.php | Integrated Draft-07 tests and updated test name formatting |
| src/JsonSchema/Constraints/Drafts/Draft07/*.php | Complete set of 30+ Draft-07 constraint validators |
| dist/schema/json-schema-draft-07.json | Added Draft-07 meta-schema definition |
| phpstan-baseline.neon | Updated baseline for new code and PHPStan format changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return; | ||
| } | ||
|
|
||
| if (!is_int($schema->multipleOf) && !is_float($schema->multipleOf) && $schema->multipleOf <= 0.0) { |
There was a problem hiding this comment.
The logical condition here is incorrect. The check (!is_int($schema->multipleOf) && !is_float($schema->multipleOf) && $schema->multipleOf <= 0.0) uses AND operators, which means it will only return early if multipleOf is neither an int nor a float AND is less than or equal to 0.0. However, if it's neither int nor float, comparing it with 0.0 doesn't make logical sense. This should use OR: (!is_int($schema->multipleOf) && !is_float($schema->multipleOf)) || $schema->multipleOf <= 0.0 to properly validate that multipleOf is either not a number type OR is not positive.
| if (!is_int($schema->multipleOf) && !is_float($schema->multipleOf) && $schema->multipleOf <= 0.0) { | |
| if ((!is_int($schema->multipleOf) && !is_float($schema->multipleOf)) || $schema->multipleOf <= 0.0) { |
| $contents = json_decode(file_get_contents($file->getPathname()), false); | ||
| foreach ($contents as $testCase) { | ||
| foreach ($testCase->tests as $test) { | ||
| [,$filename] = explode('/tests/', $file->getRealPath(), 2); |
There was a problem hiding this comment.
The list destructuring on this line assumes that '/tests/' will always be found in the file path. If '/tests/' is not present in the path returned by getRealPath(), explode will return an array with only one element, causing an error when trying to assign to the second element. Consider adding error handling or validation to ensure the path contains '/tests/' before attempting to destructure.
This PR will add support for draft 7 using the strict mode