Skip to content
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

Specify a naming scheme for non-assertion keywords #1399

Closed

Conversation

awwright
Copy link
Member

This is an alternative to #1387 and writes in #1340.

To implement annotation keywords, there's already a section a bit higher up (JSON Schema Objects and Keywords), which defines the usage of keywords, and so any requirements about naming schemes and what may or must not be assertions should probably go here.

Additionally it writes in "disallow unknown keywords" (at least partially with a SHOULD), because I found the purpose of annotation-only keywords becomes more evident.

Please closely review the language, even without "disallow unknown keywords" it may be slightly different than what #1387 is attempting to do. It provides a slightly different rationale for it, at least: While the practical way to discuss may be "an annotation keyword", the technical reason is better understood as a "non-assertion keyword".

@jdesrosiers
Copy link
Member

You're introducing a new keyword classification system in this PR. I think that should be discussed separately and have it's own PR when there's consensus.

@awwright
Copy link
Member Author

JSON Schema already has a keyword classification system, so I think it's appropriate to modify it as needed to accommodate annotation/SVA-only keywords. I mentioned that I made the changes in that section to support this addition, I can (post-hoc) explain further.

If we're going to have a naming scheme for identifying annotation-only keywords, the "$" naming scheme should also be explained here too. So this expands the scope of the "identifiers" category into "Core keywords" in turn.

And if there's going to be an option or requirement to reject unknown keywords, then we must distinguish reserved keywords (which are meaningful by themselves) and arguments (which are only meaningful in the presence of an adjacent keyword that reads it). So, this separates "Reserved locations" into "Reserved keywords" and "Arguments".

These are changes that would not be very convincing by themselves, but I think make more sense when submitted together.

Finally, I'm submitting this as a draft to solicit feedback and recognize that further changes may be necessary. At least, the "annotation-" prefix that's serving as a placeholder to bypass bikeshedding. And other changes, e.g. maybe there's someone who relies on distinguishing "identifiers" from other "core keywords"—I'm not aware of such a thing, but maybe someone wants to argue that.

@jdesrosiers
Copy link
Member

I'm totally fine with changing the classification system or even dropping it altogether (I never found it useful) if that's what people want to do. I just thought it was odd lumped in with the annotation changes.

Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

This is presented as an alternative to #1387, but I think some combination of the two would be best.

The other PR states that x- keywords MUST be collected as annotations. I think that's important to include.

The other PR states that x- keywords MUST NOT be defined in vocabularies. I think that's important to include.

This PR defines the behavior when an unknown keyword is encountered, which the other PR is missing.

Comment on lines +378 to +379
If a validator encounters a keyword whose assertion behavior is unknown (a keyword besides an "annotation-" keyword),
then attempting to compute the validation result SHOULD raise an error.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of "raise an error", would it be better to say the result is "indeterminate"? (Has the "indeterminate" concept made it into the spec yet? It might need to be defined.) I often use "raise an error" as shorthand, but not all languages have a concept of raising an error and even then, it's not required that they use that mechanism to report the indeterminate result. But, I'm probably being overly pedantic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's more specific too.

Copy link
Member

Choose a reason for hiding this comment

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

What does "attempting to compute the validation result" mean? Does it mean "it is incorrect for the evaluator to compute a validation result from this keyword"?

@awwright
Copy link
Member Author

That's a good observation, I was mostly concerned with describing the validation behavior, but you're right, more rigorous annotation behavior can be defined too.

of their subschemas.
Some properties may be classified into multiple categories.
Some keywords are also arguments to other keywords,
and applicator keywords are both assertions and annotations.
Copy link
Member

Choose a reason for hiding this comment

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

Applicator keywords may be assertive and annotative, but not necessarily so. anyOf isn't annotative. Its subschemas produce annotations, but the keyword itself does not.

Comment on lines +373 to +374
A keyword that begins with "$" is a core keyword, unless defined otherwise.
(For example, "$comment" behaves as a reserved keyword.)
Copy link
Member

Choose a reason for hiding this comment

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

See #1388

of five categories:
Object properties that are applied to the instance are called keywords, or schema keywords.
Two properties in the same schema object are referred to as adjacent keywords.
As an authoring convenience, adjacent keywords can sometimes affect each other in a way that a property from a different schema (including sub-schemas) cannot.
Copy link
Member

Choose a reason for hiding this comment

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

Is this an authoring convenience?

Can a keyword from a different schema affect a keyword locally (excluding keywords that explicitly do this like unevaluated*)?

Does this statement add value?

Comment on lines +344 to +348
<dt>Reserved keywords</dt>
<dd>
apply one or more subschemas to a particular location
in the instance, and combine or modify their results
Keywords that are permitted in a schema,
but that do not do anything by themselves,
though their value may be read by or referred to by another keyword.
Copy link
Member

Choose a reason for hiding this comment

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

Do you have an example of a "reserved" keyword? What would the purpose of this be?

Comment on lines +350 to +353
<dt>Arguments</dt>
<dd>
do not directly affect results, but reserve a place
for a specific purpose to ensure interoperability
Arguments are properties in a schema that are used by an adjacent keyword,
and whose meaning may only be defined in the presence of that keyword.
Copy link
Member

Choose a reason for hiding this comment

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

I assume these are keywords like minContains which affect contains but do nothing else.

<dt>Core keywords</dt>
<dd>
Keywords that may configure how the schema is processed,
and can modify the meaning of other keywords in the schema.
Copy link
Member

Choose a reason for hiding this comment

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

What kind of core keyword can modify the meaning of another keyword?

There's maybe $vocabulary, but this defines what the keywords mean; it doesn't modify them.

</t>
<ul>
<li>
A keyword that begins with "annotation-" is never a core keyword or assertion keyword, but may be an annotation or reserved keyword. These keywords MUST NOT affect a validation result.
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @jdesrosiers that this needs to align with the x- decision that we've already made.

@gregsdennis
Copy link
Member

As #1387 has been merged, I'm closing this. If further discussion is desired, let's do that in a new issue/discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants