Skip to content

Split "unevaluatedItems" and "unevaluatedProperties" into their own vocabulary. #981

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

Merged
merged 6 commits into from
Sep 26, 2020

Conversation

handrews
Copy link
Contributor

@handrews handrews commented Sep 6, 2020

Fixes #853. Fixes #831.

Quite some time ago we agreed to split these out. Now support (or lack of support) is all handled through $vocabulary, without any "the vocabulary is supported except these keywords might error out if the implementation just didn't feel like it" behavior.

If the implementation doesn't support annotations, it won't be able to process anything that requires the unevaluated vocabulary.

This change just moves things to the right new section, which
includes some vocabulary section boilerplate.  Further changes
will modify the contents of the moved sections appropriately.
No longer need exceptional case for individual keywords that
require annotation collection.
This attempts to address various points of confusion noted
in feedback on the most recent draft.
Leaving them as 2019-09 for now as we'll update that all at once.
@handrews handrews added the core label Sep 6, 2020
@handrews handrews added this to the draft-08-patch1 milestone Sep 6, 2020
@handrews

This comment has been minimized.

Not sure why I put this before the section defining the concept
of "keywords", clearly it belongs in that section.
@karenetheridge
Copy link
Member

Now that these keywords depend on annotation results from another vocabulary, the evaluation order of the vocabularies is now significant, so it should be explicitly stated what that ordering should be (e.g. this vocabulary always runs last after all others defined by the core specification).

@Relequestual
Copy link
Member

@karenetheridge Want to file a new issue for that? This PR is about to be merged.

@handrews
Copy link
Contributor Author

Want to file a new issue for that? This PR is about to be merged.

@Relequestual @karenetheridge #995

(which for some reason still has no comments despite addressing something that has been brought up repeatedly in the last few weeks... is this thing on?)

Comment on lines +2488 to +2492
These instance items or properties may have been unsuccessfully evaluated
against one or more adjacent keyword subschemas, such as when an assertion
in a branch of an "anyOf" fails. Such failed evaluations are not considered
to contribute to whether or not the item or property has been evaluated.
Only successful evaluations are considered.
Copy link
Member

Choose a reason for hiding this comment

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

I've been thinking about this section and its phrasing.
It's the first time in the whole document we mention "evaluation", and we don't really define what it means.

People (including myself previously) incorrectly assume that if a location has been evaluated (had something applied to it), then it counts as having been evaluated.

I'm going to riff some ideas...

We say here "successful evaluations", we mean if an application doesn't result in validation errors, and therefore the applied schema or keyword is correctly applicable to the instance location.

For example, if you had a schema...
anyOf [ { title> unicycle, props > wheels > const > 1 }, { title: bicycle, props > wheels > const > 2 } ]

If your instance was a unicycle, the first subschema item in the anyOf would be applied and successfully evaluated. Because it only has one wheel, we know it's a unicycle. It is not a bicycle, so that whole subschema is not successfully evaluated.

The instance is successfully evaluated as (or determined to be) a unicycle.

We can further justify this terminology by the fact that evaluation can be short-circuited, and therefore there's no expectation for a schema to be fully evaluated (although that would be a weak argument, and probably confuse things).


We need to clearly define what successful evaluation means, and what unsuccessful evaluation means, with justification (IMHO). I'm not currently convinced I can do that.

I'm happy for this to be a new issue in order to get this PR merged, but it would be essential to 2020-NN.

Copy link
Member

Choose a reason for hiding this comment

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

This is a good point, and it should be made clear that these keywords really mean "unSucessfullyEvaluatedThing" rather than just "unEvaluatedThing".

@Relequestual
Copy link
Member

Relequestual commented Sep 25, 2020

Want to file a new issue for that? This PR is about to be merged.

@Relequestual @karenetheridge #995

(which for some reason still has no comments despite addressing something that has been brought up repeatedly in the last few weeks... is this thing on?)

yokay, I'm still reading the rest of the issue. If I see no problems, it's merging time.

Edit: waiting for reply on my in code comment

@handrews
Copy link
Contributor Author

@Relequestual

Edit: waiting for reply on my in code comment

Let's make it a new issue to get this merged and then I can reply to that new issue in the next few days.

@handrews
Copy link
Contributor Author

Per conversation with @Relequestual on chat I've file #998 and am going to go ahead and merge this.

@handrews handrews merged commit cba93cb into json-schema-org:master Sep 26, 2020
@handrews handrews deleted the uneval branch September 26, 2020 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split "unevaluated*" into separate vocab for separate support indications JSON Schema Core - 9.3.1.3. unevaluatedItems - clarification suggested
4 participants