-
-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
keywords only exhibit the behaviors they're defined with #1577
base: main
Are you sure you want to change the base?
Conversation
So, what is the result of:
and
and
? |
Good point. The keywords produce no assertions, but the subschemas still need to. Granted, this is true with 2020-12, too. The validation spec doesn't actually define assertion results for any of the annotations, yet it's still considered a pass because there are no constraints. I think this could be stated explicitly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've always thought of all keywords having an assertion. Annotation-only keywords just always assert true
. $defs
is another example of a keyword always returns true although it's not an annotation.
I think the way this is worded is great because it allows for implementations to ignore non-assertions or just make them true
. They can implement it however makes most sense for their implementation.
any boolean logic operation to the assertion results of subschemas, but MUST NOT | ||
introduce new assertion conditions of their own. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but MUST NOT introduce new assertion conditions of their own.
I'm not following this. What is this trying to say? What are we protecting against be forbidding it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That text was already there.
It's just saying that applicator assertions can only be combinatorial on their subschemas. For instance anyOf
only ORs the results of its subschemas without asserting anything else.
I think this is a simplicity guard, kind of a JSON Schema version of SRP. I suppose this means something like requiredProperties
would be in violation of the spec since it's combining subschema results and asserting that all of the properties are there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdesrosiers what do you think of relaxing this to a SHOULD, or maybe even an informative note suggesting simplicity over complexity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's unnecessary for that to be a constraint. Your requiredProperties
example is great. Although I'm not a fan of that keyword, I don't think there should be any spec reason why it shouldn't be allowed.
or maybe even an informative note suggesting simplicity over complexity?
Yes. I think that's a great idea.
Co-authored-by: Jason Desrosiers <[email protected]>
Co-authored-by: Jason Desrosiers <[email protected]>
fit. Applicators apply subschemas to parts of the instance and combine their | ||
results. | ||
JSON Schema keywords may exhibit one or more behaviors. This specification | ||
defines three such behaviors: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to say that all keyword behaviors in the specification are in these three categories, but the spec defines other behaviors for e.g. $id
and $anchor
/$dynamicAnchor
. And $schema
... exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is an interesting point. I was thinking that these keywords don't exhibit a behavior so much as they are merely informative, like maxContains
would be to contains
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great point! My implementation doesn't even treat those as keywords. They're meta-data used to process the schema, but they're not directly part of the validation/annotation process. So, I wouldn't consider them similar to maxContains
that is involved in validation.
I'm not sure what I'd suggest, but I think something needs to be called that these aren't normal keywords.
What kind of change does this PR introduce?
clarification
Issue & Discussion References
Summary
The previous language could imply that all keywords needed to have an assertion result, e.g. annotations would always produce a "true" assertion result.
For
min/maxContains
, I added explicit text that they do not produce an assertion result, emphasizing that the assertion comes fromcontains
and that these keywords are informative.This change clarifies that keywords only exhibit the behaviors they're defined with.
Does this PR introduce a breaking change?
No.