Altered behaviour for unevaluatedProperties and unevaluatedItems

[karenetheridge]

related to: #57

I'll start by quoting a discussion I started in the #collaborators slack channel, and then elaborate.

---

Once again I’ve got one small error in my openapi document, but because it has 500+ “paths” entries in it, I get over 1000 errors in the validation result, all from unevaluatedProperties. this would be so much nicer if unevaluatedProperties also considered errors generated from a specific property in its consideration. i.e. making “unevaluated” be truly unevaluated, not “unsuccessfully evaluated”.

Making this change will make a huge difference in bandwidth in my error payloads 🙂

The openapi schema uses unevaluatedProperties everywhere, so any time you have any validation failure, anywhere in the document, you get unevaluatedProperties errors spewing from all locations, and they are totally unhelpful, not to mention very confusing to anyone who doesn’t have intimate knowledge of how this keyword works (which at this point is pretty much constrained to the dozen people in this room).

I’ve always been a bit bugged the way annotations are overloaded from just being informational (e.g. the metadata vocabulary) to also being used for some applicator keywords.

@Relequestual said: "I know a community discussion is coming, but this approach was an attempt to remove specific linking behaviour between keywords, making it easier to implement extensions.
Without it, most libraries likely wouldn't allow new/additional keywords access to information that it may very well need.
I think annotations is probably a poor choice, and really it's "processing scope data" or something "

I replied:

Yes, we may need to introduce an orthogonal thing that is tracked in dynamic scopes — which also happened for $dynamicAnchor+$dynamicRef. Annotations aren’t widely used yet, but it may become confusing that annotations from “properties” etc are mixed in with annotations from “readOnly” — applications would be interested in the latter but not the former, generally. That is — the annotations from the applicator vocabulary are generally only useful internally; the caller doesn’t care about those (but I’d be interested if someone has a counter-observation there).

---

This problem is similar to what's described in this discussion about additionalProperties -- annotations produced by adjacent properties and patternProperties isn't the same as whether that keyword actually exists in the current subschema, as a property in the instance could have been evaluated against one of those keywords and resulted in a failure. So my thought was to redefine unevaluatedProperties as considering both annotations and errors at the data instance location, which I believe would be consistent. (This still isn't truly identical to "was this property ever evaluated by any subschema, anywhere, because of the not keyword, which destroys all errors and annotations from nested subschemas, but I think we can consider this "by design".) This essentially redefines unevaluatedProperties to mean what it says, rather than how it is now which is really unsuccessfullyEvaluatedProperties. I see this as a very positive change, and resolves the problem of an explosion of useless errors from a complicated schema that uses unevaluatedProperties (e.g. openapi).

We also may want to consider whether the annotation concept is being misused by the *properties keywords: keywords like readOnly, title and deprecated are of interest to the user, but annotations from properties etc are not -- so should they be annotations at all? My impression is that the annotation concept was seen as a convenient way to make unevaluatedProperties work, but we could use a totally different mechanism here to track the evaluation status.

[jdesrosiers]

This proposal is how I would have liked to see unevaluatedProperties defined in the first place. But, my intuition was screaming that this proposal would create some kind of breaking change and I finally figured out what it is. When we added object support for contains we neglected to define how it interacts with unevaluatedProperties, but it should be consistent with the behavior defined for unevaluatedItems. That would mean that any property that matches contains would be considered evaluated and any property doesn't match would be considered unevaluated.

Example:

{
  "allOf": [
    {
      "type": "object",
      "contains": { "type": "number" }
    }
  ],
  "unevaluatedProperties": { "type": "boolean" }
}

{ "a": 1, "b": 2, "c": "NaN" }

Currently "unevaluated" means not successfully validated, so that would mean properties "a" and "b" would be considered evaluated and "c" would fail the unevaluatedProperties keyword. But, if we change "unevaluated" to mean "not evaluated", then "c" would be considered evaluated as well (it evaluated to false) and the instance would pass the unevaluatedProperties keyword because none of the properties are unevaluated.

Of course if we change what "unevaluated" means for unevaluatedProperties, we need to change what it means for unevaluatedItems to be consistent. That would mean that contains would define all items in an array to be evaluated, which would effectively undo the change in 2020-12 that gave us the contains/unevaluatedItems behavior we have now. Although I never liked the way that ended up, Henry did succeed in convincing a lot of people that it was a good idea, so if we are going to revert that change, we should have similar levels of buy-in including being comfortable with the flip-flopping of this behavior.

[gregsdennis]

I remember Henry didn't particularly like the word unevaluated but couldn't think of a better word for the behavior a wanted, which he outlined.

... [Looks at the example suspiciously] Does contains apply to objects?

[jdesrosiers]

Does contains apply to objects?

🤣 It was your proposal: json-schema-org/json-schema-spec#1077

[gregsdennis]

🤦‍♂️ hahaha

[karenetheridge]

When we added object support for contains we neglected to define how it interacts with unevaluatedProperties

Nice catch! Thanks for doing that PR.

[karenetheridge]

Of course if we change what "unevaluated" means for unevaluatedProperties, we need to change what it means for unevaluatedItems to be consistent. That would mean that contains would define all items in an array to be evaluated, which would effectively undo the change in 2020-12 that gave us the contains/unevaluatedItems behavior we have now.

It would also mean that the presence of items or additionalProperties would effectively negate the use of unevaluatedItems or unevaluatedProperties at the same data location. unevaluatedProperties has always been redundant when paired with an additionalProperties at the same data location, and contains is generally used to indicate "I don't care what other stuff is in this array, as long as <x> of the items matches this". Also, the contains might be wrapped in an if/then, so it isn't always applied to the data location, making an unevaluatedItems elsewhere in the schema not redundant.

I will continue to mull, but I think I am okay with this tradeoff.