Report an unresolvable $ref instead of passing silently - #944
Open
Amoifr wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #916.
This takes option 1 from the issue, a validation error, and there is an argument for it beyond it being the least disruptive:
CHECK_MODE_STRICTis currently more permissive than the default mode. Same schema, same data, onmain:The legacy path already refuses to let a broken
$refthrough; it is only the draft-specific constraints that swallow it. So this is not just a missing feature, it is the strict mode being laxer than the mode it is supposed to tighten. After the change, strict mode reports one error and the default mode is untouched.On the error constant
The issue suggests
MISSING_ERRORor a new one.MISSING_ERRORis not available: its message is commented out inConstraintErrorwithUsed for tests; this error is deliberately commented out, so it is reserved. This addsUNRESOLVABLE_REFwith the messageThe $ref "%s" could not be resolved, which also names the pointer that failed rather than leaving the reader to find it.On the offline case
The issue rightly raises unreachable remote schemas. Nothing changes there for anyone on the default mode, which already throws. For strict mode, a remote
$refthat cannot be fetched now produces a validation error rather than a silent pass, which I think is the honest answer: the validator cannot claim a document conforms to a schema it never read. Option 3, gating onCHECK_MODE_EXCEPTIONS, remains available on top of this if you would rather make it configurable.Verification
Two tests per draft: an unresolvable
$refis reported with the new constraint name, and a resolvable one still validates. Each fails without the change.Full suite green at 3166 tests, including the official JSON-Schema-Test-Suite, which is what I would have expected to break had the previous behaviour been load-bearing. PHPStan clean.