json-schema-spec icon indicating copy to clipboard operation
json-schema-spec copied to clipboard

"Keyword is present" meaning

Open yakimun opened this issue 3 years ago • 5 comments

In the current draft, some keywords behavior depends on the presence and annotation results of keywords from other vocabularies.

Example: https://json-schema.org/draft/2020-12/json-schema-validation.html#rfc.section.6.4.4

If "contains" is not present within the same schema object, then this keyword has no effect.

An instance array is valid against "maxContains" in two ways, depending on the form of the annotation result of an adjacent "contains" keyword. The first way is if the annotation result is an array and the length of that array is less than or equal to the "maxContains" value. The second way is if the annotation result is a boolean "true" and the instance array length is less than or equal to the "maxContains" value.

If the Applicator vocabulary isn't used to process the current schema, but the "contains" property exists in the schema object, should an implementation consider it as presented? If yes, then we will have annotation produced by the unknown keyword "contains" with an arbitrary value. Such a case isn't considered in the description of the "maxContains" keyword.

yakimun avatar Dec 25 '21 22:12 yakimun

If the Applicator vocabulary isn't used to process the current schema, but the "contains" property exists in the schema object, should an implementation consider it as presented?

I think the answer has to be "no". Nothing else would would be reasonable. It's well known that having these keywords in different vocabularies is awkward, but I don't think I've ever considered that case exactly. A similar case would be a vocabulary that uses the keyword name "contains", but isn't the same keyword as the one defined in the applicator vocab.

The spec probably needs to be more clear in this case that it refers to the "contains" in the applicator vocabulary rather than just any keyword called "contains". Or better yet, not split dependent keywords across vocabs. Either way, this is a good case for implementers to be aware of.

jdesrosiers avatar Dec 27 '21 22:12 jdesrosiers

think the answer has to be "no". Nothing else would would be reasonable

I lean towards the other direction, and also believe it to be reasonable :) Not only would it force the implementation of these keywords to use annotations (which I'd really like to get away from, and it hasn't been necessary for situations like this where only the current schema location is relevant), the annotation would have to track the source vocabulary as well.

Consider that some other vocabulary might choose to implement the same keyword but with slightly different semantics, that also produces an annotation that minContains can consume. It would be a shame for this not to be possible.

In either case, the specification could be more clear about how to handle keywords implemented by different vocabularies with the same name. We could insist that any redefinition of a keyword in the core specification must produce annotations of the same type and be compatible with other core keywords that consume those annotations (e.g. prohibit a new vocabulary's contains annotation from being a string, for instance).

karenetheridge avatar Dec 27 '21 22:12 karenetheridge

I think both viewpoints are valid, but I can't argue with @karenetheridge's reasoning. The ability to substitute different semantics for a keyword and still have dependent keywords react is powerful.

prohibit a new vocabulary's contains annotation from being a string, for instance.

We shouldn't need to prohibit annotations being different. If minContains receives from contains an annotation that's a string, then it doesn't understand the annotation, and it should go on as if the annotation didn't exist. (We should be explicit about this behavior, of course.)

gregsdennis avatar Dec 28 '21 02:12 gregsdennis

yes that's reasonable - keywords can just ignore annotations that aren't in the type/format that they expect.

karenetheridge avatar Dec 28 '21 06:12 karenetheridge

I'm sorry I used the word "reasonable". In hind sight it sounds like a passive aggressive way to diminish other ideas. It was just the only reasonable interpretation I could think of at the moment I was responding. I didn't mean to imply that there aren't other reasonable interpretations.

From the perspective of interpreting the current spec, I think this behavior would be considered "undefined" and anything presented here would be fine for implementations to do.

I agree that it's reasonable for us to aim for supporting the scenarios presented. We'd have to build out those concepts in a future release. I think we'd want to decouple annotations from the keywords that generate them. That way vocabularies can define keywords that extend or interact with core keywords without depending on the core vocabulary.

If minContains receives from contains an annotation that's a string, then it doesn't understand the annotation, and it should go on as if the annotation didn't exist.

What if the annotation happens to look like contains annotation but actually represents something other than what minContains expects? I think we would need a way to ensure we have the right annotation in order to avoid this situation.

However, I'd rather we just didn't define anything in terms of annotations at all and revisit the design of contains/minContains/maxContains to avoid this situation entirely.

jdesrosiers avatar Dec 30 '21 05:12 jdesrosiers

This would be fixed by #1312 , which both puts the keywords in the same vocabulary and makes it clear that annotations are not required to be the implementation mechanism. Which is actually already the case in the current spec- annotations have never been mandated as the implementation mechanism for any keyword, only recommended and used to describe the required externally visible behavior, but it seems like most people miss or ignore that statement. (Probably because it ought to be in §7.2 but for some reason it's in §7.3)

handrews avatar Sep 25 '22 18:09 handrews