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

Published 20 hours ago • verified publisher icon json-schema-org

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

Open karenetheridge opened this issue 3 years ago • 15 comments

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: https://github.com/json-schema-org/community/discussions/57

karenetheridge avatar Dec 06 '21 03:12 karenetheridge

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

karenetheridge avatar Feb 04 '22 18:02 karenetheridge

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.

gregsdennis avatar Feb 13 '22 01:02 gregsdennis

I'm not sure we actually NEED to change the spec. I think we read it wrong... See https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1049686478

Relequestual avatar Feb 24 '22 10:02 Relequestual

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 avatar Feb 24 '22 11:02 gregsdennis

@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?

Relequestual avatar Feb 25 '22 10:02 Relequestual

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.

gregsdennis avatar Feb 25 '22 10:02 gregsdennis

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 https://github.com/json-schema-org/json-schema-spec/pull/1154#issuecomment-1049687143) would be a change to behavior or not?

Relequestual avatar Feb 25 '22 11:02 Relequestual

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.

gregsdennis avatar Feb 25 '22 13:02 gregsdennis

~~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 avatar Feb 27 '22 20:02 karenetheridge

Okay, given the discussion in https://github.com/json-schema-org/json-schema-spec/issues/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.

karenetheridge avatar Mar 05 '22 20:03 karenetheridge

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.

gregsdennis avatar Mar 06 '22 04:03 gregsdennis

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 avatar Mar 14 '22 11:03 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?

Relequestual avatar Mar 25 '22 09:03 Relequestual

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?

gregsdennis avatar Jul 31 '22 23:07 gregsdennis

@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?

handrews avatar Aug 14 '22 16:08 handrews

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

karenetheridge avatar Sep 27 '22 01:09 karenetheridge