json-schema-spec
json-schema-spec copied to clipboard
Annotation output for `propertyDependencies`
propertyDependencies was originally proposed as a replacement for a complex and verbose pattern to apply a schema to an instance based on the value of a property, similarly to OpenAPI's discriminator, but with more native JSON-Schema-like functionality. It was added to draft-next (post-2020-12) here.
Proposed as a replacement for this pattern, one could expect that it would work the same in all cases, however I added a test to the Suite recently to cover the interation between propertyDependencies and unevaluatedProperties:
{
"$schema": "https://json-schema.org/draft/next/schema",
"propertyDependencies": {
"foo": {
"foo1": {
"properties": {
"bar": true
}
}
}
},
"unevaluatedProperties": false
}
This is supposed to be analogous to:
{
"$schema": "https://json-schema.org/draft/next/schema",
"if": {
"properties": {
"foo": { "const": "foo1" }
}
},
"then": {
"properties": {
"bar": true
}
},
"unevaluatedProperties": false
}
In the latter schema, unevaluatedProperties can "see into" /if to recognize that the foo property is declared. If the value of foo is foo1, then the /if subschema passes and produces an annotation that foo is a known property, so unevaluatedProperties subsequently ignores the property and passes validation.
However, as it is currently defined, propertyDependencies produces no annotations. That means, for the former schema, unevaluatedProperties wouldn't know about the foo property, so it would produce an error. Therefore the current definition fails to meet the explicit purpose of this keyword: a replacement for this common if/then, discriminator-like pattern.
This could be solved by requiring propertyDependencies to produce an annotation of the properties listed within it (just like properties does). (We would also need to update unevaluatedProperties to consider that annotation.)
If we maintain that propertyDependencies does not produce an annotation, then the author is required to define the property elsewhere:
{
"$schema": "https://json-schema.org/draft/next/schema",
"properties": {
"foo": true // or an enum or something
},
"propertyDependencies": {
"foo": {
"foo1": {
"properties": {
"bar": true
}
}
}
},
"unevaluatedProperties": false
}
I believe this counteracts the verbosity reduction benefit that propertyDependencies provides.
NOTE Interactions with other keywords, including unevaluatedProperties was not explored in the original proposal.
I agree completely.
I don't think transformations are relevant to how propertyDependencies should behave. They are informative in showing the value of the keyword by showing what users would have to do if it didn't exist, but propertyDependencies isn't and shouldn't be defined by any particular transformation. I don't mean that as an argument one way or another, just that I'd like to come to a decision on this without being influenced by how one transformation out of several possible transformations behaves.
I'm not convinced that not producing annotations results in more verbose schemas. It's possible to construct a schema where that's true, but it wouldn't be a very useful schema. The schema from the example says that "foo" can be absolutely any value of any type, but if it's a string and its value is "foo1", then apply the schema. The "any value of any type" part is what I consider unrealistic. In any real schema, "foo" is going to need to be more fully defined anyway. It's going to have valid values, or at least be constrained to a string. So, you're not just defining "foo" for the sake of unevaluatedProperties, it needs to be defined anyway. Even if there is some edge-case where it would make sense to leave "foo" completely unconstrained, I think it would be so rare as to not have much influence on what we decide here. Please enlighten me if you can think of some use-case that shows another perspective.
Something we haven't mentioned yet is that if propertyDependencies produces annotations, it will affect the definition of additionalProperties as well as unevaluatedProperties. If we have good reason, that's probably fine, but so far I don't see a compelling reason and I'd rather not touch those (especially additionalProperties) if we don't have to.
Although I can see how some people might expect to see propertyDependencies produce annotations, I don't see a benefit in doing so. It makes processing propertyDependencies, additionalProperties, and unevaluatedProperties a little more complicated to implement, but doesn't, IMO, provide a benefit that justifies that added complexity.
Something we haven't mentioned yet is that if propertyDependencies produces annotations, it will affect the definition of additionalProperties as well
additionalProperties isn't defined exclusively in terms of annotations right now, but "other properties that are defined in the same schema" can encapsulate the properties in propertyDependencies easily enough, so I see no reason not to do so unless there is a compelling reason? Doing so would be more consistent IMO.
To extend @karenetheridge note, unevaluatedProperties also doesn't need to change in order for propertyDependencies to produce annotations.
The specification says this for unevaluatedProperties:
The behavior of this keyword depends on the annotation results of adjacent keywords that apply to the instance location being validated. Specifically, the annotations from "properties", "patternProperties", and "additionalProperties", which can come from those keywords when they are adjacent to the "unevaluatedProperties" keyword. Those three annotations, as well as "unevaluatedProperties", can also result from any and all adjacent in-place applicator (Section 10.2) keywords. This includes but is not limited to the in-place applicators defined in this document.
unevaluatedProperties is already defined to consider annotations from all other applicator keywords, including ones that aren't in this document. That would even include my data keyword. (I should make a test for that.)
Note: propertyDependencies is being extracted to a separate proposal document. Once that goes through, PRs against the keyword should be made against that document, not Core.