Conversation
- Add $vocabulary support for vocabulary-based keyword filtering - Implement $recursiveRef and $recursiveAnchor resolution - Add duration and uuid format validators - Fix $ref resolution with embedded $id schemas - Improve unevaluatedProperties handling with dependentSchemas - Support scope isolation for $ref with sibling keywords
|
cc @aeschli |
There was a problem hiding this comment.
Looks good. This is a huge step toward being fully compliant with the JSON Schema spec, but there's definitely a few pieces still missing. I expect that this isn't intended to implement everything, but it's way closer than what we have now.
The following list is all the things I tested. I tested it with these changes running in vscode. Most of the issues are related to identifying which keywords should be recognized/ignored depending on the version of JSON Schema in use. It respects all implemented keywords regardless of what version of JSON Schema the schema uses. That didn't surprise me. I know this implementations doesn't make those distinctions, but I did notice in the code that you tried to add support for those things by implementing the $vocabulary keyword. However, I didn't see any indication that it was working at all. I see there are tests, but when I try those same scenarios in vscode, I don't see $vocabulary being respected.
The other thing I noticed is that in 2019-09, the format keyword is supposed to be an annotation only. You're allowed to provide a way to enable format validation, but it's not supposed to be enabled by default. I understand if the intention is enable this by default, but technically that's not correct.
In 2019-09, you can also enable/disable format using $vocabulary. false means annotation-only and true means validate. This can be overridden by configuration, but those are the defaults. It would be nice to see that working fully.
- ❌ Relative reference with
$id- This has never worked properly, not just with 2019-09 changes. I'm sure it wasn't intended for this to be in scope for these changes, but it would be nice to get this working properly at some point. I don't think it would be too hard compared to everything else you've achieved in this PR.
- ✔️ Reference an
$anchorin the same schema - ✔️ Reference an
$anchorin an external schema - ✔️ Reference an
$anchorin an embedded schema - ❌
$idfragments aren't anchors in a 2020-12 schema - ❌
$anchorisn't an anchor in a draft-07/6/4 schema - ✔️ embedded schemas
- ✔️ nested embedded schemas
- ✔️ recursive references
- ✔️
$refwith siblings - ✔️
dependentRequiredworks in 2019-09 - ❌
dependentRequiredshouldn't work in draft-07 - ❌
dependentSchemasworks in 2019-09- Works, but the error is on the wrong node. It appears on the property key, not the property value.
- ❌
dependentSchemasshouldn't work in draft-07 - ❌
dependenciesshouldn't work in 2019-09 - ✔️
unevaluatedItems - ✔️
unevaluatedProperties - ✔️
unevalautedPropertieswithdependentSchemas - ✔️
minContains/maxContains - ❌
formatdoesn't validate by default - ❌ Enable format validation using
$vocabulary - ❌ Disable vocab using
$vocabulary
One more thing. I'm not sure if there was something missing in my testing setup or what, but I wasn't seeing vscode recognize the changes in my schemas while editing. I had to refresh (Ctrl+R) in order for the schema changes to be recognized in the JSON document that linked it. If that's related to these changes, that could be a concern, but I expect it's just my setup.
Thanks for this work! It will be huge for JSON Schema to get this and 2020-12 support in vscode.
…2019-09 - Change Vocabularies type from Set to Map to track required/optional status - Add isFormatAssertionEnabled for format-annotation vs format-assertion - Handle $ref sibling keywords correctly per draft version - Only recognize $anchor in draft-2019-09 and later schemas - Fix dependencies keyword to only apply in draft-07 and earlier - Fix missing property error location to use object offset
|
|
I think there was some confusion because I wasn't clear enough in my checklist. I haven't had a chance to test this round of changes yet, but I want to make sure there aren't any miscommunications.
That's correct.
|
I'm still seeing {
"$schema": "./my-schema.json",
"date": "not a date" // <-- Should be ok, but has a validation error
}{
"$schema": "https://json-schema.org/draft/2019-09/schema",
"type": "object",
"properties": {
"date": {
"type": "string",
"format": "date" // <-- Should be annotation-only in 2019-09
}
}
}
Mostly looks right except for one case. {
"$schema": "./my-schema.json",
"foo": 42 // <-- This is showing that the value should be a string, but the reference shouldn't have worked.
}{
"$schema": "https://json-schema.org/draft/2019-09/schema",
"type": "object",
"properties": {
"foo": { "$ref": "#foo" } // <-- The reference target shouldn't exist
},
"$defs": {
"": {
"$id": "#foo", // <-- Should not create an anchor in draft-2019-09
"type": "string"
}
}
}
I think this is working better. I'm seeing draft-2019-09 schemas allowing only draft-2019-09 keywords. But, in draft-07 schemas, all keywords seem to work including draft-2019-09 keywords. Also, the vocabulary filtering only seems to work for built-in schemas like draft-2019-09. I'm not seeing it work for custom meta-schemas. Not sure if that was intended or not, but it looks like you wrote tests for it so I expected it to work. {
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$vocabulary": {
"https://json-schema.org/draft/2019-09/vocab/core": true,
"https://json-schema.org/draft/2019-09/vocab/applicator": true
},
"$ref": "https://json-schema.org/draft/2019-09/schema"
}{
"$schema": "./my-dialect.json",
"type": "object",
"properties": {
"name": { "type": "string" },
"age": { "minimum": 0 } // <-- "minimum" should do nothing
},
"required": ["name"] // <-- "required" should do nothing
}{ // <-- I'm seeing an error from "required" for "name" not being included, but "required" should be ignored.
"$schema": "./my-schema.json",
"age": -3 // <-- I'm seeing an error from "minimum" that should be ignored
}My checklist so far:
|
|
Quick update. I will get back to this soon, hopefully by early next week. Using vocabulary this way, doesn't quite work. Will update. |
…a 2019-09+ - Gate dependentRequired, dependentSchemas, unevaluatedProperties, unevaluatedItems, minContains, maxContains to 2019-09+ - Gate prefixItems to 2020-12+ - Gate dependencies to draft-07 and earlier - Apply vocabulary filtering only for 2019-09+ schemas - Make format annotation-only by default for 2019-09+ per spec - Support enabling format assertion via $vocabulary - Stop treating $id fragments as anchors in 2019-09+ - Fix relative $id resolution when reached via JSON pointer $ref - Highlight full object range for required property errors
|
@jdesrosiers - Thanks for the test cases, these helped. |
|
I haven't gone through everything, but I found some regressions this time around. Embedded schemas stopped working. This example get the schema from the embedded location, not try to retrieve it. {
"$schema": "https://json-schema.org/draft/2019-09/schema",
"type": "object",
"properties": {
"subject": { "$ref": "https://example.com/embedded" }
},
"$defs": {
"": {
"$id": "https://example.com/embedded",
"type": "string"
}
}
}{
"$schema": "./example.json", // Error: Unable to load schema from 'https://example.com/embedded': Location https://example.com/embedded is untrusted.
"subject": "foo"
}
{
"$schema": "http://json-schema.org/draft-07/schema#",
"type": "object",
"properties": {
"subject": { "$ref": "#foo" }
},
"$defs": {
"": {
"$anchor": "foo",
"type": "number"
}
}
}{
"$schema": "./example.json", // Expect a reference error reported here
"subject": "foo"
}Here's a weird one I stumbled into because my tests were sloppy. If you reference the same schema using two different URIs, an anchor can get double counted. {
"$schema": "https://json-schema.org/draft/2019-09/schema",
"type": "object",
"properties": {
"a": { "$ref": "#foo" },
"b": { "$ref": "#/$defs/foo" }
},
"$defs": {
"foo": {
"$anchor": "foo",
"type": "string"
}
}
}{
"$schema": "./example.json" // Error: Duplicate anchor declaration: 'foo'
} |
|
Thanks a lot of the work done so far and the testing! |
|
as per https://json-schema.org/draft/2019-09/draft-handrews-json-schema-02#rfc.section.8.2.4.2.2, "The value of the "$recursiveAnchor" property MUST be a boolean." |
This is incorrect. Properties evaluated by subschemas under any keywords adjacent to the unevaluated keyword are considered to be evaluated -- schemas reached via |
|
(from examples) |
That's plain JSON not a JSON Schema. It's the vscode |
… anchor resolution in schemas
…emas with additional tests
|
@jdesrosiers - Thanks! I added these all as tests cases and fixed them. I verified in a local instance of VS Code as well. To get this to work with the ESM work on main I had to add "default": "./lib/esm/jsonLanguageService.js"to the exports in package.json In f822189 I also found an error around scope isolation, fixed and added test cases. |
|
It took some effort to get this running with the esm changes. I had to convert the I took some time to organize my tests. They are very close to all passing at this point. I've put an "Expected Error" comment everywhere an error is expected. If you see an error anywhere else, that's a bug.
I noticed another issue not related to validation. It has to do with the Goto Definition feature. Not sure how much of this you want to consider in scope because some of it doesn't work currently. I put comments in my test cases to show some of the cases. These references all resolve correctly for validation, just not for Goto Definition.
The last one doesn't work currently, so might be considered out of scope. But, jumping to local references is currently supported, so I think since we added support for anchors and embedded schemas, those local references should be considered in scope for these changes. |
…a vocabulary resolution Address PR review feedback for JSON Schema 2019-09 support: - Fix Goto Definition (findLinks) to support plain-name anchor fragments (#foo via $anchor or legacy $id), and embedded schema references by $id URI - Fix vocabulary extraction for custom dialects referenced via relative $schema URI (e.g. "./dialect.jsonc") by resolving against the schema's base URI before fetching - Extract repeated absolute URI scheme regex into hasSchemeRegex constant - Add tests for: anchor/embedded Goto Definition, $ref siblings ignored in draft-07, unevaluatedProperties/unevaluatedItems ignored in draft-07, nested embedded schemas, and vocabulary disable with relative URI
|
@jdesrosiers - I don't quite understand this one: vocabulary-format-enable 5c8674a I added additional tests here for how I understand the spec. From your earlier comment I understood that in 2019-09 this should be annotation only, with an optional ability to enable assertions vs in 2020-12 it is split into format-assertions and format-annotations.
Agreed out of scope for both of these. Good feedback on Goto Definition, agreed it should be part of this feature, added in 5c8674a as well |
That's mostly correct, but The other way to enable assertion is to set the format vocabulary to Using the vocabulary option is a little more strict than the config option. If you enable using the config option, the implementation can have any level of support for different formats. Some formats might not be supported or only partially supported. If you enable using the vocabulary option, the validator must support all formats and should produce an error condition if it doesn't support all of them. Actually, I believe vscode doesn't support all formats and therefore should technically not support enabling by vocab at all. But, that's a dumb rule and it would be reasonable to only error if the schema uses a format that isn't supported. That's what I do in my validator. The draft-2019-09 meta-schema sets the format vocabulary to The way draft-2019-09 uses vocabularies to enable or disable assertion is not how the vocabulary system normally works. Normally, setting a vocabulary to |
Overview
This PR adds further support for JSON Schema draft 2019-09.
The changes focus on three main areas:
$recursiveRefresolution$refhandling with embedded schemasThis is a very large change, happy to break it up however would be easiest to review.
Key Changes
1. Vocabulary Support (
$vocabulary)Files:
vocabularies.ts(new),jsonSchemaService.ts,jsonParser.ts,jsonLanguageTypes.tsJSON Schema 2019-09 introduced vocabularies, which allow meta-schemas to declare which keyword sets are active. This enables custom meta-schemas to disable certain validation keywords.
Implementation
Vocabulariestype (Set<string>) to represent active vocabulary URIsisKeywordEnabled()function that checks if a keyword should be processed based on active vocabularies$schema) and extracts$vocabularydeclarationsenabled(keyword)before processing each keyword$id,$ref,$schema, etc.) are always enabled per the specExample
A meta-schema with only
applicatorvocabulary (novalidationvocabulary) will skipminimum,required,type, etc.2.
$recursiveRefand$recursiveAnchorSupportFiles:
jsonParser.ts,jsonSchema.tsThese keywords (2019-09) enable extensible recursive schemas—a pattern where a base schema can be extended while maintaining recursion to the extending schema.
Recursive Ref Implementation
schemaStackandschemaRootsparameters to thevalidate()function to track schema traversal$recursiveRefis encountered:$id)$recursiveAnchor: true, find the first$recursiveAnchor: truein the stack$recursiveAnchortype fromstringtoboolean | stringto handle both spec-correct and real-world schemas3. Improved
$refResolution with Embedded$idSchemasFiles:
jsonSchemaService.tsSchemas can contain embedded sub-schemas with their own
$id, creating new URI scopes. This PR properly resolves$refagainst the correct base URI.Key Fixes
traverseWithBaseTracking()$idvaluesregisterEmbeddedSchemas()collectAnchors()$idboundaries since anchors are scoped to their document4. Scope Isolation for
$refwith Sibling KeywordsFiles:
jsonSchemaService.tsIn 2019-09+,
$refcan have sibling keywords, butunevaluatedProperties/unevaluatedItemsin the referenced schema shouldn't see properties evaluated by those siblings.Scope Isolation Implementation
needsScopeIsolation()to detect when isolation is neededallOf:{ "allOf": [ { /* $ref'd schema */ }, { /* sibling keywords */ } ] }5.
dependentSchemasIntegration withunevaluatedPropertiesFiles:
jsonParser.tsProperties validated by
dependentSchemaswere not being tracked as "evaluated," causing false positives withunevaluatedProperties: false.Fix: Added
validationResult.mergeProcessedProperties()after validatingdependentSchemas.6.
patternPropertiesHandling FixFiles:
jsonParser.tsPreviously,
patternPropertieswas processed incrementally, which caused issues when a property matched multiple patterns.Fix: Collect all pattern matches first, validate against all matching patterns, then mark as processed.
7. New Format Validators
Files:
jsonParser.tsAdded validators for two new 2019-09 formats:
durationP1Y2M3DT4H5M6Suuid550e8400-e29b-41d4-a716-4466554400008. Type Definition Updates
Files:
jsonSchema.ts$recursiveAnchorstringboolean | string$vocabularyany{ [uri: string]: boolean }Testing
New Test Files
vocabularies.test.ts: Unit tests forisKeywordEnabled()covering all vocabulary combinationsUpdated Test Files
parser.test.ts: Added tests for:$recursiveRefdependentSchemas+unevaluatedPropertiesdurationanduuidformatsschema.test.ts: Added tests for:jsonSchemaTestSuite.test.ts:Tests Now Passing
The following test categories now pass:
$idresolutionpatternPropertiesunevaluatedProperties$recursiveRefUnsupported Features
The following 2020-12 features are detected and produce warnings:
$dynamicRef$dynamicAnchorBreaking Changes
None. All changes are backward compatible.
Remaining Skipped Tests
Most remaining skipped tests fall into two categories:
localhost:1234(partially addressed)$dynamicRef/$dynamicAnchor: 2020-12 features not yet implemented