Add CREF about ambiguous behaviour of additionalProperties

adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md

@@ -0,0 +1,87 @@
+# Acknowledge ambiguity in additionalProperties behaviour and fix after patch release
+
+* Status: proposed
+* Deciders: @relequestual @gregsdennis, @jdesrosiers, @karenetheridge
+* Date:  2022-04-08
+
+https://github.com/json-schema-org/json-schema-spec/issues/1172
+https://github.com/json-schema-org/community/discussions/57
+https://github.com/json-schema-org/json-schema-spec/pull/1203
+
+## Context and Problem Statement
+
+When we changed the specification to use annotations as the context in which some keywords behave, we included a clause that implementations which didn't use annotations may implement or optimize the processing of `additionalProperties` in another way which produces the same effect as the prior behaviour.
+This section created an ambiguity in terms of the resulting output format, but not validation.
+
+We needed to decide on how to proceed for the patch release of the 2020-12 version of the specification.
+
+The two above links are to a GitHub Discussion and a GitHub Issue detailing the problems.
+Details with an example of the problem can be seen in the Discussion's opening post specifically.
+
+## Decision Drivers <!-- optional -->
+
+* The "patch release" should not change anything functionally
+* Annotations as they are, are confusing to users, implementers, and specification editors alike
+* Patch release is behind schedule
+* There are currently no tests for the output format
+* It's hard to see any immediate consensus on changing the annotation based behaviour
+
+## Considered Options
+
+* [Leaving it "as is" and do nothing](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1413777)
+* [Pick one](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1416683) of the behaviours
+* [Revert back to draft-07 behaviour](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1453723)
+* [Reinterpret how we understand annotation collection](https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1049686478) to allow reading annotations within the same schema object regardless of assertion results
+* [Acknowledge and accept that two approaches and results are allowable](https://github.com/json-schema-org/community/issues/161#issue-1173742930)
+* Redefine annotation collection behaviour and/or how `additionalProperties` works
+
+## Decision Outcome
+
+Chosen option: "Acknowledge and accept that two approaches and results are allowable", because
+
+* Leaving it "as is" will continue to cause confusion
+* The change is non-functional which is required for the patch release
+* The patch release is behind schedule
+* Finding consensus of other solutions proved to be difficult
+* There's no test suite for the output format, so it's not easy to see unintended consequences of a functional change
+* We need to properly re-evaluate annotation collection and how annotations are used by other keywords
+
+### Positive Consequences
+
+* Patch release can move forward
+* Validation result is not impacted
+* Confusion is at least seen and acknowledged
+* Implementations which pick either approach are seen to be compliant
+
+### Negative Consequences
+
+* May have an impact for downstream tools which process full output data
+* A test suite (not yet developed) which covers this situation needs to allow for multiple valid answers
+
+## Pros and Cons of the Options
+
+### Leaving it "as is" and do nothing
+
+Agree to do nothing and hope for the best. There isn't any downstream tooling yet anyway.
+
+* Good, because no functional change
+* Good, because no impact on downstream tooling
+* Bad, because leaves a known ambiguity in the specification
+
+### Pick one / Revert to draft-07 behaviour / Reinterpret annotation collection
+
+* Good, because ambiguity is removed
+* Good, because not many tools will be effected
+* Bad, because it can be seen as a functional change (not really allowed for the patch release)
+* Bad, because it can break existing implementations and downstream tools
+* Bad, because without a test suite it's hard to see unexpected consequences
+
+## Links
+
+* Issue: [Ambiguous behaviour of additionalProperties when invalid](https://github.com/json-schema-org/json-schema-spec/issues/1172)
+* Discussion: [The meaning of "additionalProperties" has changed](https://github.com/json-schema-org/community/discussions/57)
+* Resolving Pull Request: [Add CREF about ambiguous behaviour of additionalProperties](https://github.com/json-schema-org/json-schema-spec/pull/1203)
+* Alternative solution proposal: [Resolve contradictions in the described behaviour of "additionalProperties" and "items"](https://github.com/json-schema-org/json-schema-spec/pull/1154)
+
+* [Result of discussing](https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1063962900) on an Open Community Working Meeting call - @jdesrosiers proposed a less controversial and more agreeable solution to add a comment that both are allowable
+* [Related GitHub Discussion](https://github.com/json-schema-org/community/discussions/67) on alternative behaviour for `unevaluated*` keywords

adr/README.md

@@ -0,0 +1,15 @@
+# Architectural Decision Log
+
+This log lists the architectural decisions for the JSON Schema specification.
+
+<!-- adrlog -- Regenerate the content by using "adr-log -i". You can install it via "npm install -g adr-log" -->
+
+* [ADR-2022-04-08](2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md) - Acknowledge ambiguity in additionalProperties behaviour and fix after patch release
+
+<!-- adrlogstop -->
+
+You can find the ADR for using ADRs in our [community repo ADR log](https://github.com/json-schema-org/community/tree/HEAD/docs/adr).
+
+For new ADRs, please use [template.md](template.md) as basis.
+More information on MADR is available at <https://adr.github.io/madr/>.
+General information about architectural decision records is available at <https://adr.github.io/>.

adr/template.md

@@ -0,0 +1,72 @@
+# [short title of solved problem and solution]
+
+* Status: [proposed | rejected | accepted | deprecated | … | superseded by [ADR-0005](0005-example.md)] <!-- optional -->
+* Deciders: [list everyone involved in the decision] <!-- optional -->
+* Date: [YYYY-MM-DD when the decision was last updated] <!-- optional -->
+
+Technical Story: [description | ticket/issue URL] <!-- optional -->
+
+## Context and Problem Statement
+
+[Describe the context and problem statement, e.g., in free form using two to three sentences. You may want to articulate the problem in form of a question.]
+
+## Decision Drivers <!-- optional -->
+
+* [driver 1, e.g., a force, facing concern, …]
+* [driver 2, e.g., a force, facing concern, …]
+* … <!-- numbers of drivers can vary -->
+
+## Considered Options
+
+* [option 1]
+* [option 2]
+* [option 3]
+* … <!-- numbers of options can vary -->
+
+## Decision Outcome
+
+Chosen option: "[option 1]", because [justification. e.g., only option, which meets k.o. criterion decision driver | which resolves force force | … | comes out best (see below)].
+
+### Positive Consequences <!-- optional -->
+
+* [e.g., improvement of quality attribute satisfaction, follow-up decisions required, …]
+* …
+
+### Negative Consequences <!-- optional -->
+
+* [e.g., compromising quality attribute, follow-up decisions required, …]
+* …
+
+## Pros and Cons of the Options <!-- optional -->
+
+### [option 1]
+
+[example | description | pointer to more information | …] <!-- optional -->
+
+* Good, because [argument a]
+* Good, because [argument b]
+* Bad, because [argument c]
+* … <!-- numbers of pros and cons can vary -->
+
+### [option 2]
+
+[example | description | pointer to more information | …] <!-- optional -->
+
+* Good, because [argument a]
+* Good, because [argument b]
+* Bad, because [argument c]
+* … <!-- numbers of pros and cons can vary -->
+
+### [option 3]
+
+[example | description | pointer to more information | …] <!-- optional -->
+
+* Good, because [argument a]
+* Good, because [argument b]
+* Bad, because [argument c]
+* … <!-- numbers of pros and cons can vary -->
+
+## Links <!-- optional -->
+
+* [Link type] [Link to ADR] <!-- example: Refined by [ADR-0005](0005-example.md) -->
+* … <!-- numbers of links can vary -->

jsonschema-core.xml

@@ -2480,6 +2480,17 @@
                             checking the names in "properties" and the patterns in
                             "patternProperties" against the instance property set.
                             Implementations that do not support annotation collection MUST do so.
+                            <cref>
+                                In defining this option, it seems there is the potential for
+                                ambiguity in the output format. The ambiguity does not affect validation results,
+                                but it does affect the resulting output format.
+                                The ambiguity allows for multiple valid output results depending on whether annotations
+                                are used or a solution that "produces the same effect" as draft-07. It is understood
+                                that annotations from failing schemas are dropped.
+                                See our
+                                [Decision Record](https://github.com/json-schema-org/json-schema-spec/tree/HEAD/adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md)
+                                for further details.
+                            </cref>
                         </t>
                     </section>