Make properties/items wording consistent. #111
Conversation
Separate "items" and "additionalItems" for consistency with the separate "properties", "patternProperties", and "additionalProperties". Fix wording bug: non-objects always successfully validate against "additionalProperties". Arrange the wording for each of these to follow this pattern: * keyword value type requirements * value to assume if absent (if relevant) * validation against the instance (independent of child validation) * validation of children (possibly first explaining to which children this keyword applies).
|
@awwright @Relequestual any feedback? I have the use-boolean-values-for-child-schema wording ready to add but it builds on this and I don't want to combine them in case this wording is rejected. |
| </t> | ||
| <t> | ||
| Successful validation of an array instance with regards to these two | ||
| keywords is determined as follows: | ||
| Validation of this keyword against the instance always succeeds. |
There was a problem hiding this comment.
I don't really understand this sentence.
There was a problem hiding this comment.
That's because it's weird :-P
While working out schema algebra rules for the whole "additionalProperties": false" thing, it became clear that it's very useful to have separate notions of whether the immediate instance validates against the keyword and whether any children validate.
For an array, immediate instance validation is just about about the array length.
So in this case, the "items" keyword cannot place restrictions on the length, so the array (as an array, independent of its elements) cannot fail validation based on this keyword. Even if "items" is itself an array, it does not on its own restrict the length of the array- that is only done by "minItems" and "maxItems".
Even "additionalItems": false does not impose length restrictions, it just ensures that child validation fails for all elements after a certain point. Actually, I think the wording can be improved around that now that I've tried to explain it.
| @@ -415,6 +443,14 @@ | |||
| <t> | |||
| If absent, it can be considered the same as an empty object. | |||
| </t> | |||
| <t> | |||
| Validation of this keyword against the instance always succeeds. | |||
There was a problem hiding this comment.
Again I don't understand this =/
There was a problem hiding this comment.
For an object, immediate instance validation is just about what property names are or are not present (as determined by required, minProperties, maxProperties).
properties tells us things about certain property names, but does not restrict the set of property names.
Although now that I think about it, technically even "additionalProperties": false does not restrict the set of property names, it just ensures that all child values for names not otherwise accounted for fail their validation.
Hmm... that may lead to a better phrasing. Let me think about it and I'll submit an update.
A lot of the most confusing language around the "additional" keywords
can be removed if we consider keywords strictly with respect to either
the instance's primitive type or against child instances.
Describing {"additionalItems": false} as failing child validation of
elements beyond the length of "items" is more straightforward and
has the same effect as the more convoluted reasoning about length.
|
@Relequestual I've tried a slightly different approach. I feel like this makes things much more clear by eliminating the most convoluted language around additionalItems and additionalProperties, but we'll see if it's clear to anyone else :-) This also gets rid of the need to talk about "true" and "false" in special ways, as the behavior follows cleanly from how they are applied to child instance values. I'm happy to keep reworking this as needed, as it's been a persistent source of confusion and complexity. |
|
I'm going to merge this as I don't see it makes any changes, just makes a few bits clearer! |
|
Curious. Any idea why this commit doesn't come up in the commits at https://github.com/json-schema-org/json-schema-spec/graphs/contributors ? I don't appear, and you only have one commit |
|
I think those graphs aren't quite real-time. |
| </section> | ||
|
|
||
| <section title="additionalProperties"> | ||
| <t> | ||
| The value of "additionalProperties" MUST be a boolean or a schema. | ||
| The value of "additionalProperties" MUST be a boolean or an | ||
| object. If it is an object, the object MUST be a valid JSON Schema. |
There was a problem hiding this comment.
This has the effect of only permitting schemas that happen to be objects; it's entirely possible schemas might also take the form of a boolean, and in earlier drafts they could be a string, too.
There was a problem hiding this comment.
It says:
MUST be a boolean or an object.
which I think allows for booleans? The separation of "or a schema" into "or an object. If it is..." was done to make it the same as all of the other keywords that specify a boolean or a schema.
I am not aware of anywhere recent, including either the current draft or draft 04, that allowed strings for schemas in general or for this keyword in particular, so I am unclear on your purpose for that comment.
There was a problem hiding this comment.
Ok. Iirc I was trying to make it consistent the other way, because saying "If it's an object, then it's a schema" seems redundant when a schema is already defined to be an object. I'll consider making that change if you don't see any problems with that.
There was a problem hiding this comment.
I was about to put up a pull request for #101 which updates all of this a little bit by no longer requiring the boolean case to be specially explained in two places.
I think my wording for that accomplishes your goal, and if not, definitely feel free to fix it in another way. I'll have the PR up later today.
This is a fix for #103 , although I went a bit further and made an effort to make all of the object/array property/item keywords consistent. I expect the wording might need a bit of tweaking even if the general approach is accepted. Also, I'll happily drop this if a different approach is preferred.
Separate "items" and "additionalItems" for consistency with the separate "properties",
"patternProperties", and "additionalProperties".
Fix wording bug: non-objects always successfully validate against "additionalProperties".
Arrange the wording for each of these to follow this pattern: