Skip to content

some edits to the spec for the interim update release #1110

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

some edits to the spec for the interim update release #1110

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6df9b8c
mention that annotations from these keywords affect unevaluated*
karenetheridge May 31, 2021
b6d3b09
remove remaining uses of "production"
karenetheridge May 31, 2021
7ec8df1
rewording of sections on "contains", "maxContains", "minContains"
karenetheridge May 31, 2021
f78b65c
"contains" did not annotate before 2020-12
karenetheridge May 31, 2021
dca5361
$recursiveRef -> $dynamicRef was not just a simple keyword renaming
karenetheridge May 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions jsonschema-core.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,8 @@
"items", whose behavior is defined in terms of "prefixItems"
</t>
<t>
"contains", whose behavior is defined in terms of "minContains"
"contains", whose behavior is affected by the presence and value of
"minContains", in the Validation vocabulary
</t>
</list>
</t>
Expand Down Expand Up @@ -2338,15 +2339,18 @@
</t>
<t>
An array instance is valid against "contains" if at least one of
its elements is valid against the given schema. The subschema MUST be
applied to every array element even after the first match has
been found, in order to collect annotations for use by other keywords.
This is to ensure that all possible annotations are collected.
its elements is valid against the given schema, provided that
"minContains" is not present or is a value greater than 0.
</t>
<t>
Logically, the validation result of applying the value subschema to each
item in the array MUST be ORed with "false", resulting in an overall
validation result.
If "minContains" is present and has a value of 0, then an array instance
is valid against this keyword only if NONE of its elements is valid
against the given schema.
</t>
<t>
The subschema MUST be applied to every array element even after the first
match has been found, in order to collect annotations for use by other
keywords. This is to ensure that all possible annotations are collected.
</t>
<t>
This keyword produces an annotation value which is an array of
Expand Down
34 changes: 32 additions & 2 deletions jsonschema-validation.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,23 @@
<eref target="https://json-schema.org/draft/2020-12/meta/validation"/>.
</t>

<section title="Keyword Independence">
<t>
Schema keywords typically operate independently, without
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better not to copy the wording from the core spec- then it will get out of sync, and I kind of like keeping the statements of principles in the core. You can xref it, and/or just note the vocabulary-specific dependencies. But this is not a hill I intend to die on so if you feel strongly feel free to keep it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the Applicator vocabulary itself has this paragraph, it's reasonable for the Validation vocabulary to have it too. How about moving it out into the general section before we start talking about individual vocabularies? Then all the exceptions don't need to be listed explicitly -- we can just give a single example, and direct the reader to pay close attention to individual keyword definitions for the exceptions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that's embarrassing. I thought it was in the general section (don't look at the change log- it's probably my fault it's not 😅 ). If you feel like factoring it out that would be great, but I do worry a bit about not highlighting the interactions sufficiently. I would be totally fine leaving this the way you have it now (with the duplicated language) and factoring it out in the next draft.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also didn't realize this wasn't in the general section. It definitely should be. There's no reason to duplicate documentation of this common principle. I can help with the refactor if needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.
Are you happy to move this to the general section as part of this PR @karenetheridge ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there is a section in each applicable vocabulary (applicator, validation, unevaluated), but the content is all slightly different. Is it better to have similar-but-slightly-different wording in several places, or just one wording in one place (if this is the case, I would propose putting it in section 7.10)?

affecting each other's outcomes.
</t>
<t>
For schema author convenience, there are some exceptions among the
keywords in this vocabulary:
<list>
<t>
"maxContains" and "minContains", whose behavior is affected by the
presence and value of "contains", in the Applicator vocabulary
</t>
</list>
</t>
</section>

<section title="Validation Keywords for Any Instance Type" anchor="general">
<section title="type">
<t>
Expand Down Expand Up @@ -418,6 +435,12 @@
result is a boolean "true" and the instance array length is less than or
equal to the "maxContains" value.
</t>
<t>
If annotations are not being collected, the validation value may be determined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of phrasing this should use MAY, and I feel the wording could be condensed a bit, but I do not feel strongly about that so feel free to ignore. (the condensing part- the "may" should really be a MAY).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph is simply describing the alternative algorithm to use that doesn't involve annotations. MAY in the RFC sense isn't correct here as alternative behaviour is not permitted -- it's only the implementation can vary. How about "can"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just going by this language from the additional* applicators:

Implementations MAY choose to implement or optimize this keyword in
another way that produces the same effect, such as by directly
checking the names in "properties" and the patterns in
"patternProperties" against the instance property set.
Implementations that do not support annotation collection MUST do so.

As long as they end up consistent I'm probably fine with whatever. If making it consistent gets bogged down, I'll live with a little inconsistency. It is probably better to use "can" than lower-case "may", although that is definitely not an absolute rule.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a reaonsable change. Let's go with "can".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm cutting out this entire part of the revision, since (as per other PRs recently submitted) trying to define a parallel implementation using annotations is not going to be identicaly, so we shouldn't suggest it at all.

by considering the number of instance elements that are valid against the
"contains" schema: validation is successful if and only if the number of valid
elements is less than or equal to the "maxContains" value.
</t>
</section>

<section title="minContains">
Expand All @@ -437,10 +460,17 @@
annotation result is a boolean "true" and the instance array length is
greater than or equal to the "minContains" value.
</t>
<t>
If annotations are not being collected, the validation value may be determined
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/may/can/ again here?

by considering the number of instance elements that are valid against the
"contains" schema: validation is successful if and only if the number of valid
elements is greater than or equal to the "minContains" value.
</t>
<t>
A value of 0 is allowed, but is only useful for setting a range
of occurrences from 0 to the value of "maxContains". A value of
0 with no "maxContains" causes "contains" to always pass validation.
of occurrences from 0 to the value of "maxContains". A value of
0 causes "contains" to always pass validation (but validation can still fail
against a "maxContains" keyword).
</t>
<t>
Omitting this keyword has the same behavior as a value of 1.
Expand Down