resolve contradictions in the described behaviour of "additionalProperties" and "items"

[karenetheridge]

Wording has been reverted to that of draft7, before annotations existed, and
the behaviour was clearly in terms of the presence of adjacent "properties"
and "patternProperties" keywords, not their evaluation results.

The described alternate behaviour when annotations are not collected is
contradictory to when annotations are collected, due to annotations only
existing when evaluation is successful. Producing errors at these
properties/items will never change the overall evaluation result, as errors
will ahve already been produced from the earlier keywords. Since this is a
changed behaviour between draft7 and draft2019-09, and no one had noticed this
inconsistency before now, it is likely that implementations have continued
with the previous behaviour, so correcting the language now will not actually
result in noticable changes in behaviour.

The inconsistencies in the behaviour of unevaluatedProperties and
unevaluatedItems with respect to evaluated-but-not-successfully
properties/items is greater, so will have to be addressed in a major revision
of the specification.

reference: json-schema-org/community#57

[karenetheridge]

Note: I didn't add anything to the changelog.

[gregsdennis]

Since this is a changed behaviour between draft7 and draft2019-09, and no one had noticed this inconsistency before now, it is likely that implementations have continued with the previous behaviour, so correcting the language now will not actually result in noticable changes in behaviour.

The assumption that implementations don't use the prescribed behavior in 2019-09+ is wrong. Mine uses the annotation behavior; something I had to recently account for when processing a draft 6/7 schema.

I think 2019-09 & 2020-12 need to stay as they are, and this should be corrected (and unevaluated* aligned/resolved) in the next major version.

[Relequestual]

I'm not sure we actually NEED to change the spec.
I think we read it wrong... See #1172 (comment)

[gregsdennis]

Are we sure that we want to push this for the 2020-12 patch? It's a change to a behavior, not a clarification.

I agree with the change, but I think this change is better for the next version.

and no one had noticed this inconsistency before now

I noticed

it is likely that implementations have continued with the previous behaviour,

and my library behaves accordingly.

[gregsdennis]

From the discussion in Slack with @Relequestual I agree with the reinterpretation of the spec that annotations are passed down at the schema level rather than the keyword level. This means that all annotations are maintained within the context of a (sub)schema evaluation, but upon leaving that context, annotation are dropped if that (sub)schema fails validation.

This resolves the issues that I (and likely many other implementors) have been receiving of confusing behavior by causing additionalProperties to behave as it did in draft 7 without having to revert the language.

I think we need a more thorough clarification on how annotations are passed rather than changing this keyword.

(Note that this still means that I have to update my library.)

[Relequestual]

@gregsdennis Given your second comment, do you still feel your previous comment stands?

Are we sure that we want to push this for the 2020-12 patch? It's a change to a behavior, not a clarification.

As in, do you still feel it's a change to behavior or do you agree it's a clarification?

[gregsdennis]

I still think that this reversion is a change to behavior, yes. It may be similar, but it's different than what is currently prescribed.

[Relequestual]

Sorry, I didn't qualify my question.

THIS PR would be a change in behavior, sure.
Do you however believe that a clarification of interpritations (as per #1154 (comment)) would be a change to behavior or not?

[gregsdennis]

It's a change in behavior for my implementation because I implemented dropping annotations at the keyword level.

Based on the premise of the opening comment of this PR

Since this is a changed behaviour between draft7 and draft2019-09, and no one had noticed this inconsistency before now, it is likely that implementations have continued with the previous behaviour, so correcting the language now will not actually result in noticable changes in behaviour.

I think a consensus of the reinterpretation should suffice, and changes represented in this PR are not needed.

[karenetheridge]

I agree, if we go with the reinterpretation that @Relequestual discovered, then this PR is moot, but perhaps different wording would be needed to clarify the situation (and we'll need to update that one 'contains' test in the test suite too).

[karenetheridge]

Okay, given the discussion in #1172, I think this PR can be replaced with a different patch that clarifies the annotation behaviour of additionalProperties and unevaluatedProperties. Whether or not it is more useful to always produce a true annotation (indicating that it evaluated at all properties, regardless of whether the results were valid or invalid) would be worthy of discussion.

I think we might also need an amendment in the spec in the section on output formats: https://json-schema.org/draft/2020-12/json-schema-core.html#rfc.section.12.4 implies that a keyword can only produce an error or annotations, but not both, but we can see that some keywords such as properties or allOf don't produce both, but their subschemas can produce annotations even if the keyword itself produces an error.

[gregsdennis]

Having both errors and annotations is not supported with the current output structure since annotations are included as nested nodes. I think this will need to wait until the next draft and once we've discussed and decided on those changes.

[Relequestual]

Right. I'd really like to keep on topic as possible, opening a new issue for the next feature draft where we want to discuss further other topics.
As an aside, the verbose output structure does sort of support showing "all" annotations and validation results...

The primary difference between [verbose] and the "Detailed" structure is that all results are returned. This includes sub-schema validation results that would otherwise be removed (e.g. annotations for failed validations, successful validations inside a not keyword, etc.).
https://json-schema.org/draft/2020-12/json-schema-core.html#rfc.section.12.4.4

[Relequestual]

Okay, given the discussion in #1172, I think this PR can be replaced with a different patch that clarifies the annotation behaviour of additionalProperties and unevaluatedProperties...

@karenetheridge As such, would you be happy for this PR to be removed from the current patch draft milestone?

[gregsdennis]

Given recent conversations around keyword behaviors & classifications, I'm unsure this PR needs to remain open. I believe the overall intent is to keep with the annotation behavior (though the specifics are still being ironed out).

Can we close this now?

[handrews]

@karenetheridge I'm seconding @gregsdennis 's request that we close this — it seems like the concerns have been addressed elsewhere, or we've shifted enough in our thinking that a new PR would be needed anyway?

[karenetheridge]

could this be tagged with something like "reconsider for next draft"?