From cc59f43c06b8a86b88dfe841e4b23d86a514eb32 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Mon, 7 Sep 2026 23:21:49 +0300 Subject: [PATCH 1/3] feat(ir)!: give Deprecation a RemovalDate and promote x-sunset into it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit x-sunset echoes RFC 8594's Sunset header, which is a date by definition, but the default promotion mapping read it into Deprecation.RemovalVersion — a field whose name, doc comment and sibling all say version. A consumer deciding whether removing a deprecated operation is breaking compares a sunset against a release date, and could not tell which spelling it had been handed without re-parsing the string. Take issue #417's option 1: a distinct RemovalDate beside RemovalVersion, with x-sunset promoting to the date. A version and a date are two facts, not two spellings of one — a document may state both ("gone in 3.0.0", "gone on 2026-08-01"), and neither is derivable from the other without a release calendar the IR does not have. A single field carrying which spelling it holds (option 2) would have to drop whichever fact it read second, so it costs losslessness to buy nothing a second field does not already give: the field a value arrives in is what says which fact it is. Deliberately out of scope, and stated in ir-design.md and at the reading site: RemovalDate is the source's own text, neither parsed nor normalized. No source format defines the field, so none defines its format; and the key→field mapping is caller policy, so a key pointed at the date field is the caller's statement that it holds a date. Morphic records which fact was stated and leaves the calendar to the consumer. BREAKING CHANGE: Deprecation gains removalDate, and x-sunset now fills it instead of removalVersion. A consumer reading removalVersion for a sunset reads an empty field until it moves. No default key names RemovalVersion any more — a document stating a removal version names its own key, per promotion rule 1 — so the corpus stops witnessing that field and it joins unwitnessed.golden.txt. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 --- .../openapi/internal/lowering/promotion.go | 20 ++++-- .../internal/lowering/promotion_test.go | 7 ++- compilers/openapi/options.go | 2 + compilers/openapi/promotion_test.go | 63 +++++++++++++++++-- docs/ir-design.md | 31 ++++++--- ir/docs.go | 19 +++++- ir/docs_test.go | 8 ++- ir/helpers_test.go | 1 + .../openapi/extension-promotion.golden.json | 10 +-- .../openapi/extension-promotion.yaml | 4 +- .../openapi/unwitnessed.golden.txt | 1 + 11 files changed, 134 insertions(+), 32 deletions(-) diff --git a/compilers/openapi/internal/lowering/promotion.go b/compilers/openapi/internal/lowering/promotion.go index a672623f..90866340 100644 --- a/compilers/openapi/internal/lowering/promotion.go +++ b/compilers/openapi/internal/lowering/promotion.go @@ -38,8 +38,13 @@ const ( TargetDeprecationMessage ExtensionTarget = "deprecation.message" // TargetDeprecationSince fills ir.Deprecation.Since. TargetDeprecationSince ExtensionTarget = "deprecation.since" - // TargetDeprecationRemovalVersion fills ir.Deprecation.RemovalVersion. + // TargetDeprecationRemovalVersion fills ir.Deprecation.RemovalVersion. No + // default key names it: the one convention in wide use for a scheduled + // removal, x-sunset, states a date, and a document that spells a removal + // *version* names its own key. TargetDeprecationRemovalVersion ExtensionTarget = "deprecation.removalVersion" + // TargetDeprecationRemovalDate fills ir.Deprecation.RemovalDate. + TargetDeprecationRemovalDate ExtensionTarget = "deprecation.removalDate" ) // ExtensionPromotions is the vendor-extension promotion policy: which x-* keys @@ -71,7 +76,9 @@ func DefaultExtensionPromotions() map[string]ExtensionTarget { return map[string]ExtensionTarget{ "x-deprecated-reason": TargetDeprecationMessage, "x-deprecated-since": TargetDeprecationSince, - "x-sunset": TargetDeprecationRemovalVersion, + // x-sunset echoes the RFC 8594 Sunset header, which is a date by + // definition, so it fills the date field and not the version one. + "x-sunset": TargetDeprecationRemovalDate, } } @@ -127,14 +134,19 @@ func deprecationField(dep *ir.Deprecation, target ExtensionTarget) *string { return &dep.Since case TargetDeprecationRemovalVersion: return &dep.RemovalVersion + case TargetDeprecationRemovalDate: + return &dep.RemovalDate default: return nil } } // extensionText reads a preserved extension value as a string. Every -// Deprecation field is prose or a version, so a value of any other JSON shape -// is a document meaning something else by the key. +// Deprecation field is prose, a version or a date, so a value of any other JSON +// shape is a document meaning something else by the key. Text of the right JSON +// shape is taken as written — a date is not parsed here, because the mapping is +// the caller's policy and a key it points at the date field is its statement +// that the key holds one. func extensionText(raw ir.RawValue) (string, bool) { var text string if err := json.Unmarshal(raw, &text); err != nil { diff --git a/compilers/openapi/internal/lowering/promotion_test.go b/compilers/openapi/internal/lowering/promotion_test.go index 9ec944f9..92dd043b 100644 --- a/compilers/openapi/internal/lowering/promotion_test.go +++ b/compilers/openapi/internal/lowering/promotion_test.go @@ -33,8 +33,10 @@ func vendorExtension(rawJSON string) ir.UnmodeledEntry { } // TestPromoteDeprecation_FillsTheFieldsThePolicyNames pins what each mapping -// writes, one field at a time, because the three share a struct and a promotion -// writing the wrong member of it would still look filled. +// writes, one field at a time, because they share a struct and a promotion +// writing the wrong member of it would still look filled. The removal pair is +// why that matters most: a date written into the version field is the defect +// GitHub #417 records, and it reads as a filled Deprecation either way. func TestPromoteDeprecation_FillsTheFieldsThePolicyNames(t *testing.T) { t.Parallel() tests := []struct { @@ -45,6 +47,7 @@ func TestPromoteDeprecation_FillsTheFieldsThePolicyNames(t *testing.T) { {"message", lowering.TargetDeprecationMessage, ir.Deprecation{Message: "why"}}, {"since", lowering.TargetDeprecationSince, ir.Deprecation{Since: "why"}}, {"removal version", lowering.TargetDeprecationRemovalVersion, ir.Deprecation{RemovalVersion: "why"}}, + {"removal date", lowering.TargetDeprecationRemovalDate, ir.Deprecation{RemovalDate: "why"}}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/compilers/openapi/options.go b/compilers/openapi/options.go index b59513ed..03852b90 100644 --- a/compilers/openapi/options.go +++ b/compilers/openapi/options.go @@ -56,6 +56,8 @@ const ( TargetDeprecationSince = lowering.TargetDeprecationSince // TargetDeprecationRemovalVersion fills ir.Deprecation.RemovalVersion. TargetDeprecationRemovalVersion = lowering.TargetDeprecationRemovalVersion + // TargetDeprecationRemovalDate fills ir.Deprecation.RemovalDate. + TargetDeprecationRemovalDate = lowering.TargetDeprecationRemovalDate ) // DefaultExtensionPromotions returns the extension-to-field mapping applied diff --git a/compilers/openapi/promotion_test.go b/compilers/openapi/promotion_test.go index 32f135b0..b628f510 100644 --- a/compilers/openapi/promotion_test.go +++ b/compilers/openapi/promotion_test.go @@ -84,12 +84,15 @@ func assertExtensionPromotion(t *testing.T, doc *ir.Document, diags []ir.Diagnos op, ok := opByName(doc, "getX") require.True(t, ok) assert.Equal(t, "1.2.0", op.Deprecation.Since) - assert.Equal(t, "2.0.0", op.Deprecation.RemovalVersion) + assert.Equal(t, "2026-08-01", op.Deprecation.RemovalDate, + "x-sunset is a date, so it reaches the date field") + assert.Empty(t, op.Deprecation.RemovalVersion, + "a sunset date does not land in the field a consumer reads as a version") require.Len(t, op.Params, 1) require.NotNil(t, op.Params[0].Deprecation) - assert.Equal(t, "3.0.0", op.Params[0].Deprecation.RemovalVersion, - "the parameter's own x-sunset reaches its own removal version, not the operation's") + assert.Equal(t, "2027-01-15", op.Params[0].Deprecation.RemovalDate, + "the parameter's own x-sunset reaches its own removal date, not the operation's") assertPromotionDeclined(t, doc, diags) } @@ -209,11 +212,59 @@ paths: op, ok := opByName(doc, "getX") require.True(t, ok) require.NotNil(t, op.Deprecation) - for _, got := range map[openapi.ExtensionTarget]string{ + + // Read off the defaults rather than listing the pairs, so a mapping this + // test does not know about fails here instead of going unread. + fields := map[openapi.ExtensionTarget]string{ openapi.TargetDeprecationMessage: op.Deprecation.Message, openapi.TargetDeprecationSince: op.Deprecation.Since, openapi.TargetDeprecationRemovalVersion: op.Deprecation.RemovalVersion, - } { - assert.Equal(t, "filled", got, "every default target is filled by its default key") + openapi.TargetDeprecationRemovalDate: op.Deprecation.RemovalDate, + } + named := map[openapi.ExtensionTarget]bool{} + for key, target := range defaults { + got, known := fields[target] + require.True(t, known, "%s is a default target this test reads no field for", target) + assert.Equal(t, "filled", got, "%s is filled by its default key %s", target, key) + named[target] = true } + for target, got := range fields { + if !named[target] { + assert.Empty(t, got, "%s is filled by no default key, so it stays empty", target) + } + } +} + +// TestPromotion_RemovalDateAndVersionAreSeparateFacts pins why a scheduled +// removal is two fields rather than one field carrying which spelling it holds +// (GitHub #417). A document can state both — a sunset date and the release it +// goes in — and one field would have to drop whichever it read second. +func TestPromotion_RemovalDateAndVersionAreSeparateFacts(t *testing.T) { + t.Parallel() + both := openapi.Options{Promotions: openapi.ExtensionPromotions{ + Targets: map[string]openapi.ExtensionTarget{ + "x-sunset": openapi.TargetDeprecationRemovalDate, + "x-gone-in": openapi.TargetDeprecationRemovalVersion, + }, + }} + spec := `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /x: + get: + operationId: getX + deprecated: true + x-sunset: "2026-08-01" + x-gone-in: "9.0.0" + responses: + "200": + description: ok +` + doc := compilePromotionSpec(t, spec, both) + op, ok := opByName(doc, "getX") + require.True(t, ok) + require.NotNil(t, op.Deprecation) + assert.Equal(t, "2026-08-01", op.Deprecation.RemovalDate) + assert.Equal(t, "9.0.0", op.Deprecation.RemovalVersion, + "both facts survive; neither overwrites the other") } diff --git a/docs/ir-design.md b/docs/ir-design.md index 90036610..b6a12377 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1695,7 +1695,14 @@ type Docs struct { ExternalDocs []Link // {URL, Description} } -type Deprecation struct { Message, Since, RemovalVersion string } +type Deprecation struct { Message, Since, RemovalVersion, RemovalDate string } +// A scheduled removal is two fields because a version and a date are two facts, not two +// spellings of one: a document may state either or both, and neither is derivable from the +// other without a release calendar the IR does not have. A consumer deciding whether removing +// a deprecated entity is breaking compares a removal date against a release date, so it must +// be able to tell which fact it holds without re-parsing the string. RemovalDate is the +// source's own text, unparsed and unnormalized — no source format defines the field, so none +// defines its format either. type Example struct { Name string @@ -1816,12 +1823,13 @@ where the IR expects them, so there is no reason to record and no unmodelled con #### Promoting a vendor extension into the field it is the only spelling for Several typed fields model information no source format gives a keyword for, so the only way a -document can state it is a vendor extension: `Deprecation.Message`/`Since`/`RemovalVersion`, -`Pagination.*`, `LongRunning`, `Idempotency`, `ErrorCase.Retryable`/`Throttling`, `Enum.Flags`, -`EnumMember.Name`, `Sensitive` and `Secret`. Reading such an extension into its field is -**promotion**, and because the format assigns an `x-*` key no semantics at all, promotion is a -heuristic — invariant 6 applies to it in full. Four rules, so that no emitter has to re-derive -this from `Unmodeled` and no two derive it differently: +document can state it is a vendor extension: +`Deprecation.Message`/`Since`/`RemovalVersion`/`RemovalDate`, `Pagination.*`, `LongRunning`, +`Idempotency`, `ErrorCase.Retryable`/`Throttling`, `Enum.Flags`, `EnumMember.Name`, `Sensitive` +and `Secret`. Reading such an extension into its field is **promotion**, and because the format +assigns an `x-*` key no semantics at all, promotion is a heuristic — invariant 6 applies to it +in full. Four rules, so that no emitter has to re-derive this from `Unmodeled` and no two derive +it differently: 1. **The mapping is injectable policy, default-on and disableable**, per compiler. Its default contents are conventions, not standards: nothing in any specification says `x-deprecated-reason` @@ -1841,8 +1849,13 @@ this from `Unmodeled` and no two derive it differently: (§4.4) and `EnumMember` (§4.5) are the instances today: each carries a `Deprecation` and no provenance of its own, so no key maps into either until one of them gains one. -A value the mapped field cannot hold — anything but text, for the three `Deprecation` members — is -reported and not coerced, since the document means something else by the key. +A value the mapped field cannot hold — anything but text, for the four `Deprecation` members — is +reported and not coerced, since the document means something else by the key. Text of the right +JSON shape is taken as written: `x-sunset` fills `RemovalDate` and not `RemovalVersion` because +the header it echoes ([RFC 8594](https://www.rfc-editor.org/rfc/rfc8594)) is a date by +definition, and the mapping is where that reading is stated — the promotion does not then parse +the date to confirm it. No default key names `RemovalVersion`: a document stating a removal +*version* names its own key, per rule 1. ### 12.1 One structural home per declaration diff --git a/ir/docs.go b/ir/docs.go index 02d4997a..21325437 100644 --- a/ir/docs.go +++ b/ir/docs.go @@ -21,13 +21,30 @@ type Link struct { } // Deprecation marks an entity as deprecated with optional migration guidance. +// +// A scheduled removal has two fields because a version and a date are two +// facts, not two spellings of one: a document may state either or both ("gone +// in 3.0.0", "gone on 2026-08-01"), and neither is derivable from the other +// without a release calendar the IR does not have. Keeping them apart is what +// lets a consumer compare a scheduled removal against a release date without +// re-parsing the string to work out which kind it was handed. type Deprecation struct { // Message explains the deprecation and any migration path. Message string `json:"message,omitempty"` // Since is the version in which the entity was deprecated. Since string `json:"since,omitempty"` - // RemovalVersion is the version in which the entity is scheduled for removal. + // RemovalVersion is the version in which the entity is scheduled for + // removal. A removal the source states as a date belongs in RemovalDate. RemovalVersion string `json:"removalVersion,omitempty"` + // RemovalDate is the date on which the entity is scheduled for removal — + // the fact an RFC 8594 Sunset carries, and what the OpenAPI x-sunset + // convention echoing it holds. + // + // It is the source's own text, neither parsed nor normalized: the IR + // records which fact the document stated and leaves the calendar to the + // consumer, since no source format defines the field and so none defines + // its format either. + RemovalDate string `json:"removalDate,omitempty"` } // Example is a documentation example. Field legality is contextual: diff --git a/ir/docs_test.go b/ir/docs_test.go index b1150209..cabc0a21 100644 --- a/ir/docs_test.go +++ b/ir/docs_test.go @@ -24,9 +24,11 @@ func TestLink_JSONContract(t *testing.T) { } // TestDeprecation_JSONContract pins Deprecation's omitempty contract — all -// three fields are optional, so an entity deprecated with no detail at all -// still marshals to an empty object rather than three empty strings — and -// that a fully populated Deprecation round-trips. +// four fields are optional, so an entity deprecated with no detail at all +// still marshals to an empty object rather than four empty strings — and +// that a fully populated Deprecation round-trips, RemovalVersion and +// RemovalDate included, since a consumer tells one from the other by which key +// it arrived under. func TestDeprecation_JSONContract(t *testing.T) { t.Parallel() assertJSONContract(t, ir.Deprecation{}, `{}`, *populatedDeprecation()) diff --git a/ir/helpers_test.go b/ir/helpers_test.go index f562cc08..113e8909 100644 --- a/ir/helpers_test.go +++ b/ir/helpers_test.go @@ -276,6 +276,7 @@ func populatedDeprecation() *ir.Deprecation { Message: "use v2 instead", Since: "1.2.0", RemovalVersion: "2.0.0", + RemovalDate: "2026-08-01", } } diff --git a/testdata/conformance/openapi/extension-promotion.golden.json b/testdata/conformance/openapi/extension-promotion.golden.json index b9f40b8b..8b41116b 100644 --- a/testdata/conformance/openapi/extension-promotion.golden.json +++ b/testdata/conformance/openapi/extension-promotion.golden.json @@ -28,7 +28,7 @@ "deprecation": { "message": "use getY instead", "since": "1.2.0", - "removalVersion": "2.0.0" + "removalDate": "2026-08-01" }, "params": [ { @@ -44,7 +44,7 @@ "docs": {}, "deprecation": { "message": "use filter instead", - "removalVersion": "3.0.0" + "removalDate": "2027-01-15" }, "unmodeled": { "openapi:x-deprecated-reason": { @@ -57,7 +57,7 @@ }, "openapi:x-sunset": { "reason": "vendor_extension", - "value": "3.0.0", + "value": "2027-01-15", "provenance": { "source": 0, "pointer": "/paths/~1x/get/parameters/0/x-sunset" @@ -175,7 +175,7 @@ }, "openapi:x-sunset": { "reason": "vendor_extension", - "value": "2.0.0", + "value": "2026-08-01", "provenance": { "source": 0, "pointer": "/paths/~1x/get/x-sunset" @@ -434,7 +434,7 @@ { "format": "openapi@3.1", "path": "extension-promotion.yaml", - "hash": "aa16a7bd98de256e0feb2bf240903d08db7f651a6b802255d7d89a98a69bcadd" + "hash": "2433e30d378223aa920d20cb51cce28080a3a325c92f36dbbe7662470fbee376" } ] } diff --git a/testdata/conformance/openapi/extension-promotion.yaml b/testdata/conformance/openapi/extension-promotion.yaml index 30e78be2..cd0f88e2 100644 --- a/testdata/conformance/openapi/extension-promotion.yaml +++ b/testdata/conformance/openapi/extension-promotion.yaml @@ -7,13 +7,13 @@ paths: deprecated: true x-deprecated-reason: use getY instead x-deprecated-since: "1.2.0" - x-sunset: "2.0.0" + x-sunset: "2026-08-01" parameters: - name: legacy in: query deprecated: true x-deprecated-reason: use filter instead - x-sunset: "3.0.0" + x-sunset: "2027-01-15" schema: {type: string} responses: "200": diff --git a/testdata/conformance/openapi/unwitnessed.golden.txt b/testdata/conformance/openapi/unwitnessed.golden.txt index f6a4bc2f..88950790 100644 --- a/testdata/conformance/openapi/unwitnessed.golden.txt +++ b/testdata/conformance/openapi/unwitnessed.golden.txt @@ -23,6 +23,7 @@ Content.SchemaFormat CtorValue.Args CtorValue.Name CtorValue.Scalar +Deprecation.RemovalVersion Discriminator.Envelope Discriminator.EnvelopeValueName Discriminator.Index From 8a4ebe49acd86abe8ef35774e4cba2a3b53c4138 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Mon, 7 Sep 2026 23:42:32 +0300 Subject: [PATCH 2/3] feat(compilers/openapi): promote x-extensible-enum onto Enum.Closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ir.Enum has carried a Closed bool since the IR was written, and the OpenAPI compiler set it true at both construction sites unconditionally. So the one key the format has for saying an enum is open, x-extensible-enum, survived only as a generic vendor_extension entry, and every consumer reading typed fields saw a closed enum whatever the document said. Open versus closed decides whether a generator emits a fallback member and whether a differ calls an added value breaking, so this was a wiring gap, not a modelling one. Add TargetEnumOpen to the promotion vocabulary, map x-extensible-enum onto it by default, and apply it in attachDeclaredAnnotations beside the deprecation promotion — the point at which a declaration's extensions have reached the node's map, which is what makes "the extension survives its own promotion" structural here as it is there. Every promotion property holds unchanged: the entry stays put with its vendor_extension reason, the node records extension-promotion in Provenance.Inferred, and a disabled policy writes nothing. The target names the fact rather than the field, which the rest of the vocabulary does not. Openness is the only half of that bool a document ever declares — a schema's `enum` is closed by definition — so a target named for Closed could only ever be written false and would read as its own opposite at every mapping naming it. For the same reason the key's presence is the statement rather than its value: the established spelling writes the member list as the value, and a list of members says nothing about openness the key naming it has not already said. A boolean is the one shape that does state it alone, so an explicit `false` is read as written rather than inverted. Deliberately out of scope, and stated in ir-design.md and at the reading site: a document writing x-extensible-enum *instead* of `enum`, with the members in the extension, lowers to no ir.Enum at all and there is no node to open. Minting one would be reading a member list out of a vendor key rather than promoting a field; the entry survives verbatim for a consumer that wants to. The corpus can now witness the matrix's open-enums row, so its matrixRowsUncovered reason is deleted rather than left to go stale, and extension-promotion.yaml gains the three enums that pin the three answers the reading has: the convention opens one, an explicit false declines to, and an enum naming no such key is untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 --- compilers/openapi/conformance_matrix_test.go | 3 - compilers/openapi/conformance_test.go | 2 +- .../openapi/internal/lowering/promotion.go | 73 ++++++++ .../internal/lowering/promotion_test.go | 144 ++++++++++++++- compilers/openapi/internal/schema/schema.go | 7 + compilers/openapi/options.go | 2 + compilers/openapi/promotion_test.go | 70 +++++++- docs/ir-design.md | 24 ++- docs/ir-spec-matrix.md | 2 +- .../openapi/extension-promotion.golden.json | 170 +++++++++++++++++- .../openapi/extension-promotion.yaml | 11 ++ 11 files changed, 482 insertions(+), 26 deletions(-) diff --git a/compilers/openapi/conformance_matrix_test.go b/compilers/openapi/conformance_matrix_test.go index 912ba948..043d78b7 100644 --- a/compilers/openapi/conformance_matrix_test.go +++ b/compilers/openapi/conformance_matrix_test.go @@ -207,9 +207,6 @@ func TestConformance_MatrixRowNamesResolve(t *testing.T) { // that reads as closable and is not. func matrixRowsUncovered() map[string]string { return map[string]string{ - "open-enums": "OpenAPI has no open-enum keyword; the matrix's ⚠ is the " + - "anyOf: [{enum: [...]}, {type: string}] idiom, which lowers as an ordinary union " + - "and needs a spec pinning that the enum branch survives beside the open one", "pagination": "OpenAPI states it only through links and x-*, and this compiler keeps both " + "verbatim rather than reading either into ir.Pagination — response-links pins that they " + "survive. Invariant 6 puts the inference in a pass rather than in the compiler, so this " + diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 04698533..ae62cc31 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -227,7 +227,7 @@ func conformanceCases() []conformanceCase { {"path-item-docs", assertPathItemDocs, []string{"docs-summary-description"}}, {"path-item-operations", assertPathItemOperations, []string{"http-binding"}}, {"deprecation", assertDeprecation, []string{"deprecation"}}, - {"extension-promotion", assertExtensionPromotion, []string{"deprecation"}}, + {"extension-promotion", assertExtensionPromotion, []string{"deprecation", "open-enums"}}, {"examples", assertExamples, []string{"examples"}}, {"docs-summary-desc", assertDocsSummaryDesc, []string{"docs-summary-description"}}, {"extensions-x", assertExtensionsX, []string{"vendor-extensions"}}, diff --git a/compilers/openapi/internal/lowering/promotion.go b/compilers/openapi/internal/lowering/promotion.go index 90866340..03d5470d 100644 --- a/compilers/openapi/internal/lowering/promotion.go +++ b/compilers/openapi/internal/lowering/promotion.go @@ -45,6 +45,21 @@ const ( TargetDeprecationRemovalVersion ExtensionTarget = "deprecation.removalVersion" // TargetDeprecationRemovalDate fills ir.Deprecation.RemovalDate. TargetDeprecationRemovalDate ExtensionTarget = "deprecation.removalDate" + + // TargetEnumOpen clears ir.Enum.Closed, saying the member set admits values + // the document does not list. + // + // It names the fact rather than the field, which the rest of this vocabulary + // does not, because openness is the only half of that bool a document ever + // declares: a schema's `enum` is closed by definition, so a target named for + // Closed could only ever be written false and would read as its own opposite + // at every mapping that names it. + // + // The key's presence is the statement. The established spelling, + // x-extensible-enum, writes the member list as its value, so there is no flag + // to read there — but a boolean value *is* a statement about openness, and an + // explicit `false` is honoured rather than inverted (extensionOpenness). + TargetEnumOpen ExtensionTarget = "enum.open" ) // ExtensionPromotions is the vendor-extension promotion policy: which x-* keys @@ -79,6 +94,9 @@ func DefaultExtensionPromotions() map[string]ExtensionTarget { // x-sunset echoes the RFC 8594 Sunset header, which is a date by // definition, so it fills the date field and not the version one. "x-sunset": TargetDeprecationRemovalDate, + // x-extensible-enum is the convention for an enum a service may add + // members to, so it says the set is open and not that it is closed. + "x-extensible-enum": TargetEnumOpen, } } @@ -123,6 +141,44 @@ func (c Ctx) PromoteDeprecation(unmodeled ir.Unmodeled, dep *ir.Deprecation, pro return diags } +// PromoteEnumOpenness clears e.Closed when the vendor extensions kept in +// unmodeled include a key the policy maps to TargetEnumOpen, and marks prov +// with the heuristic when it does. +// +// It is PromoteDeprecation at a second carrier, with the same three properties: +// the entry it reads stays where it was, the node records that a heuristic +// wrote the field, and a disabled policy writes nothing. What differs is that +// the fact is stated by the key being present rather than by a value, so this +// reports nothing: the deprecation reading declines a value it cannot hold and +// says so, while here every value shape but an explicit `false` is a key that +// means what its name says (TargetEnumOpen, extensionOpenness). +// +// The order the policy's keys are visited in is not fixed, because it cannot +// matter: a key that states openness writes the same field the same value as +// any other, a key that does not is skipped rather than deciding anything, and +// none of them reports. Two keys disagreeing therefore read the same either +// way round — open, because one of them said so. +// +// Deliberately out of scope: a document that writes x-extensible-enum *instead* +// of `enum`, listing the members in the extension, lowers to no ir.Enum at all, +// so there is no node here to open. Reading a member list out of an extension +// would be minting an enum from a vendor key rather than promoting a field, and +// the entry survives verbatim for a consumer that wants to (GitHub #427). +func (c Ctx) PromoteEnumOpenness(unmodeled ir.Unmodeled, e *ir.Enum, prov *ir.Provenance) { + if e == nil || prov == nil || len(unmodeled) == 0 || len(c.promotions) == 0 { + return + } + for key, target := range c.promotions { + entry, declared := unmodeled[extensionKeyPrefix+key] + if target != TargetEnumOpen || !declared || !extensionOpenness(entry.Value) { + continue + } + e.Closed = false + markInferred(prov, ExtensionPromotionHeuristic) + return + } +} + // deprecationField returns the field target names on dep, or nil when target // names something that is not a deprecation field. A policy may map a key to // any target in the vocabulary, and most carriers answer for only some of it. @@ -155,6 +211,23 @@ func extensionText(raw ir.RawValue) (string, bool) { return text, true } +// extensionOpenness reads a preserved extension value as a statement that an +// enum's member set is open. +// +// The key's presence is the statement, so a value of any shape but a boolean +// reads as open: x-extensible-enum's established spelling writes the *members* +// as its value, and a list of members says nothing about openness that the key +// naming it has not already said. A boolean is the one shape that does state +// openness on its own, so an explicit false is read as written — a document +// saying the set is not extensible, which is not something to invert. +func extensionOpenness(raw ir.RawValue) bool { + var open bool + if err := json.Unmarshal(raw, &open); err != nil { + return true + } + return open +} + // markInferred adds one heuristic's name to a provenance, keeping any already // there. Provenance.Inferred holds a single string and more than one heuristic // can reach a node — an operation grouped by path prefix whose deprecation diff --git a/compilers/openapi/internal/lowering/promotion_test.go b/compilers/openapi/internal/lowering/promotion_test.go index 92dd043b..f0db9930 100644 --- a/compilers/openapi/internal/lowering/promotion_test.go +++ b/compilers/openapi/internal/lowering/promotion_test.go @@ -267,6 +267,30 @@ func stringLiteral(value ast.Expr, typed bool) (string, bool) { return lit.Value, true } +// appliers is one closure per promote function the package exports. Each runs +// c's policy against a carrier of its own — the key "x-k", which every caller +// below maps — and reports whether that carrier changed. +// +// The census is hand-written, so a promote function this list does not know +// about is the same gap one level down: the check beneath it would then declare +// a target unapplied that a real lowering does apply. +func appliers() []func(lowering.Ctx) (bool, []ir.Diagnostic) { + return []func(lowering.Ctx) (bool, []ir.Diagnostic){ + func(c lowering.Ctx) (bool, []ir.Diagnostic) { + var dep ir.Deprecation + var prov ir.Provenance + diags := c.PromoteDeprecation(ir.Unmodeled{"openapi:x-k": vendorExtension(`"v"`)}, &dep, &prov) + return dep != (ir.Deprecation{}), diags + }, + func(c lowering.Ctx) (bool, []ir.Diagnostic) { + enum := ir.Enum{Closed: true} + var prov ir.Provenance + c.PromoteEnumOpenness(ir.Unmodeled{"openapi:x-k": vendorExtension(`"v"`)}, &enum, &prov) + return !enum.Closed, nil + }, + } +} + // TestExtensionTarget_EveryDeclaredTargetHasAnApplier holds the vocabulary to // the appliers, which is the half of "a target is a constant and an applier" // that nothing else checks: the constant alone compiles, maps cleanly, and @@ -277,21 +301,125 @@ func stringLiteral(value ast.Expr, typed bool) (string, bool) { // vocabulary entry that fills nothing is the likeliest way this seam breaks. // // A target belonging to a family this package cannot yet apply fails here on -// purpose: adding one means adding its applier, and teaching this test which -// applier answers for it, exactly as a new census keyword means adding its arm. +// purpose: adding one means adding its applier, and teaching appliers() which +// promote function answers for it, exactly as a new census keyword means adding +// its arm. func TestExtensionTarget_EveryDeclaredTargetHasAnApplier(t *testing.T) { t.Parallel() for _, target := range declaredTargets(t) { c := promotionCtx(lowering.ExtensionPromotions{ Targets: map[string]lowering.ExtensionTarget{"x-k": target}, }) - var dep ir.Deprecation - var prov ir.Provenance - diags := c.PromoteDeprecation(ir.Unmodeled{"openapi:x-k": vendorExtension(`"v"`)}, &dep, &prov) - - assert.Empty(t, diags, "%s: a declared target reports nothing when it is applied", target) - assert.NotEqual(t, ir.Deprecation{}, dep, + var applied bool + for _, apply := range appliers() { + wrote, diags := apply(c) + assert.Empty(t, diags, "%s: a declared target reports nothing when it is applied", target) + applied = applied || wrote + } + assert.True(t, applied, "%s is declared in the vocabulary but no applier fills it, so a policy naming it "+ "promotes nothing and says nothing", target) } } + +// TestPromoteEnumOpenness_ClearsClosedAndMarksTheNode pins the promotion the +// vocabulary's one non-Deprecation target performs. Every enum the compiler +// builds is closed, so the write here is the whole of what x-extensible-enum +// buys a consumer, and the marker is what says a heuristic made it. +func TestPromoteEnumOpenness_ClearsClosedAndMarksTheNode(t *testing.T) { + t.Parallel() + tests := []struct { + name string + value string + }{ + {"the member list the convention writes", `["a","b"]`}, + {"an explicit true", `true`}, + {"a value that states nothing", `"whatever"`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + enum := ir.Enum{Closed: true} + var prov ir.Provenance + promotionCtx(lowering.ExtensionPromotions{}).PromoteEnumOpenness( + ir.Unmodeled{"openapi:x-extensible-enum": vendorExtension(tc.value)}, &enum, &prov) + + assert.False(t, enum.Closed, "the key says the member set is open") + assert.Equal(t, lowering.ExtensionPromotionHeuristic, prov.Inferred) + }) + } +} + +// TestPromoteEnumOpenness_LeavesTheEntryItRead is the losslessness half, for +// the same reason its deprecation twin is: the promotion is a second reading of +// a preserved entry, so a consumer that disagrees still has what was written — +// which for this key is the member list itself. +func TestPromoteEnumOpenness_LeavesTheEntryItRead(t *testing.T) { + t.Parallel() + unmodeled := ir.Unmodeled{"openapi:x-extensible-enum": vendorExtension(`["a","b"]`)} + enum := ir.Enum{Closed: true} + promotionCtx(lowering.ExtensionPromotions{}).PromoteEnumOpenness(unmodeled, &enum, &ir.Provenance{}) + + entry, kept := unmodeled["openapi:x-extensible-enum"] + require.True(t, kept, "the entry survives its own promotion") + assert.Equal(t, ir.ReasonVendorExtension, entry.Reason) + assert.JSONEq(t, `["a","b"]`, string(entry.Value)) +} + +// TestPromoteEnumOpenness_WritesNothing pins every shape that must leave the +// enum closed. The last row is the one that is not an absence: a document +// writing the key with a boolean false says the set is *not* extensible, and +// reading presence alone there would record the opposite of what it said. +func TestPromoteEnumOpenness_WritesNothing(t *testing.T) { + t.Parallel() + filled := ir.Unmodeled{"openapi:x-extensible-enum": vendorExtension(`["a","b"]`)} + tests := []struct { + name string + policy lowering.ExtensionPromotions + unmodeled ir.Unmodeled + }{ + {"promotion disabled", lowering.ExtensionPromotions{Disabled: true}, filled}, + {"no extensions kept", lowering.ExtensionPromotions{}, nil}, + {"a key the document did not write", lowering.ExtensionPromotions{}, ir.Unmodeled{ + "openapi:x-other": vendorExtension(`["a","b"]`), + }}, + { + "a key mapped to another target", + lowering.ExtensionPromotions{Targets: map[string]lowering.ExtensionTarget{ + "x-extensible-enum": lowering.TargetDeprecationMessage, + }}, + filled, + }, + { + "an explicit false", + lowering.ExtensionPromotions{}, + ir.Unmodeled{"openapi:x-extensible-enum": vendorExtension(`false`)}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + enum := ir.Enum{Closed: true} + var prov ir.Provenance + promotionCtx(tc.policy).PromoteEnumOpenness(tc.unmodeled, &enum, &prov) + + assert.True(t, enum.Closed, "the enum stays as the format declared it") + assert.Empty(t, prov.Inferred, "nothing was inferred, so nothing is marked") + }) + } +} + +// TestPromoteEnumOpenness_NonEnumCarrierIsTheWholeAnswer pins the nil cases, +// which are the ordinary shape rather than a guard: most nodes an x-* key can +// sit on are not enums, and a node with no provenance could not record the +// guess (promotion rule 4). +func TestPromoteEnumOpenness_NonEnumCarrierIsTheWholeAnswer(t *testing.T) { + t.Parallel() + c := promotionCtx(lowering.ExtensionPromotions{}) + unmodeled := ir.Unmodeled{"openapi:x-extensible-enum": vendorExtension(`["a","b"]`)} + c.PromoteEnumOpenness(unmodeled, nil, &ir.Provenance{}) + + enum := ir.Enum{Closed: true} + c.PromoteEnumOpenness(unmodeled, &enum, nil) + assert.True(t, enum.Closed, "with nowhere to record the guess, none is made") +} diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index 57b2180a..e3f1c958 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1246,6 +1246,13 @@ func attachDeclaredAnnotations(c lowering.Ctx, ts *compile.Types, anchors *Ancho } common.Unmodeled = annotation.MergeUnmodeled(common.Unmodeled, a.Unmodeled) diags = append(diags, c.PromoteDeprecation(common.Unmodeled, common.Deprecation, &common.Provenance)...) + // The enum-openness promotion is applied here rather than where the Enum is + // built, for the same reason the deprecation one is: a promotion reads the + // preserved Unmodeled entries, and this is the point at which the + // declaration's extensions have reached the node's map. + if enum, isEnum := td.(*ir.Enum); isEnum { + c.PromoteEnumOpenness(common.Unmodeled, enum, &common.Provenance) + } if len(a.Examples) > 0 { common.Examples = a.Examples } diff --git a/compilers/openapi/options.go b/compilers/openapi/options.go index 03852b90..b2521d63 100644 --- a/compilers/openapi/options.go +++ b/compilers/openapi/options.go @@ -58,6 +58,8 @@ const ( TargetDeprecationRemovalVersion = lowering.TargetDeprecationRemovalVersion // TargetDeprecationRemovalDate fills ir.Deprecation.RemovalDate. TargetDeprecationRemovalDate = lowering.TargetDeprecationRemovalDate + // TargetEnumOpen clears ir.Enum.Closed. + TargetEnumOpen = lowering.TargetEnumOpen ) // DefaultExtensionPromotions returns the extension-to-field mapping applied diff --git a/compilers/openapi/promotion_test.go b/compilers/openapi/promotion_test.go index b628f510..15a7cdf3 100644 --- a/compilers/openapi/promotion_test.go +++ b/compilers/openapi/promotion_test.go @@ -94,9 +94,46 @@ func assertExtensionPromotion(t *testing.T, doc *ir.Document, diags []ir.Diagnos assert.Equal(t, "2027-01-15", op.Params[0].Deprecation.RemovalDate, "the parameter's own x-sunset reaches its own removal date, not the operation's") + assertEnumOpenness(t, doc) assertPromotionDeclined(t, doc, diags) } +// assertEnumOpenness is the corpus row for GitHub #427 and the matrix's +// open-enums row. ir.Enum.Closed is exactly the fact x-extensible-enum states, +// and every enum the compiler builds is closed, so without the promotion the +// extension changed nothing an emitter or a differ could read. +// +// The three schemas are the three answers the reading has: the convention's own +// spelling opens the enum, an explicit false declines to, and an enum that +// names no such key is untouched — the last so that the first is a promotion +// rather than a compiler that stopped closing enums. +func assertEnumOpenness(t *testing.T, doc *ir.Document) { + tests := []struct { + schema string + closed bool + inferred string + }{ + {"Size", false, "extension-promotion"}, + {"Shade", true, ""}, + {"Fixed", true, ""}, + } + for _, tc := range tests { + enum, ok := doc.Types[namedID(tc.schema)].(*ir.Enum) + require.True(t, ok, "%s lowers to an enum", tc.schema) + assert.Equal(t, tc.closed, enum.Closed, "%s openness", tc.schema) + assert.Equal(t, tc.inferred, enum.Provenance.Inferred, "%s heuristic marker", tc.schema) + } + + for _, schema := range []string{"Size", "Shade"} { + enum, ok := doc.Types[namedID(schema)].(*ir.Enum) + require.True(t, ok) + entry, kept := enum.Unmodeled["openapi:x-extensible-enum"] + require.True(t, kept, "%s keeps the extension whether or not it was read", schema) + assert.Equal(t, ir.ReasonVendorExtension, entry.Reason, + "%s promotion does not reclassify what it read", schema) + } +} + // assertPromotionDeclined pins the two shapes promotion refuses, both of which // leave the extension exactly where it was: a key on a node that never said it // was deprecated annotates nothing, and a value that is not text is a document @@ -193,6 +230,13 @@ func TestPromotion_DefaultTargetsAreTheOnesApplied(t *testing.T) { defaults := openapi.DefaultExtensionPromotions() require.NotEmpty(t, defaults, "an empty mapping would make this vacuous") + // Every default key is written twice, on a deprecated operation and on an + // enum, because the targets live on two carriers and a key reaching only the + // wrong one would read as a mapping that fills nothing. + keys := "" + for key := range defaults { + keys += " " + key + ": filled\n" + } spec := `openapi: 3.1.0 info: {title: T, version: "1"} paths: @@ -200,18 +244,21 @@ paths: get: operationId: getX deprecated: true -` - for key := range defaults { - spec += " " + key + ": filled\n" - } - spec += ` responses: +` + keys + ` responses: "200": description: ok -` +components: + schemas: + E: + type: string + enum: [a, b] +` + keys doc := compilePromotionSpec(t, spec, openapi.Options{}) op, ok := opByName(doc, "getX") require.True(t, ok) require.NotNil(t, op.Deprecation) + enum, ok := doc.Types[namedID("E")].(*ir.Enum) + require.True(t, ok) // Read off the defaults rather than listing the pairs, so a mapping this // test does not know about fails here instead of going unread. @@ -220,6 +267,7 @@ paths: openapi.TargetDeprecationSince: op.Deprecation.Since, openapi.TargetDeprecationRemovalVersion: op.Deprecation.RemovalVersion, openapi.TargetDeprecationRemovalDate: op.Deprecation.RemovalDate, + openapi.TargetEnumOpen: filledWhen(!enum.Closed), } named := map[openapi.ExtensionTarget]bool{} for key, target := range defaults { @@ -235,6 +283,16 @@ paths: } } +// filledWhen renders a target whose field is not text as the "filled" the text +// ones carry, so one table can read every default target rather than growing an +// arm per field type. +func filledWhen(promoted bool) string { + if promoted { + return "filled" + } + return "" +} + // TestPromotion_RemovalDateAndVersionAreSeparateFacts pins why a scheduled // removal is two fields rather than one field carrying which spelling it holds // (GitHub #417). A document can state both — a sunset date and the release it diff --git a/docs/ir-design.md b/docs/ir-design.md index b6a12377..40f7be3c 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1824,12 +1824,12 @@ where the IR expects them, so there is no reason to record and no unmodelled con Several typed fields model information no source format gives a keyword for, so the only way a document can state it is a vendor extension: -`Deprecation.Message`/`Since`/`RemovalVersion`/`RemovalDate`, `Pagination.*`, `LongRunning`, -`Idempotency`, `ErrorCase.Retryable`/`Throttling`, `Enum.Flags`, `EnumMember.Name`, `Sensitive` -and `Secret`. Reading such an extension into its field is **promotion**, and because the format -assigns an `x-*` key no semantics at all, promotion is a heuristic — invariant 6 applies to it -in full. Four rules, so that no emitter has to re-derive this from `Unmodeled` and no two derive -it differently: +`Deprecation.Message`/`Since`/`RemovalVersion`/`RemovalDate`, `Enum.Closed`, `Pagination.*`, +`LongRunning`, `Idempotency`, `ErrorCase.Retryable`/`Throttling`, `Enum.Flags`, +`EnumMember.Name`, `Sensitive` and `Secret`. Reading such an extension into its field is +**promotion**, and because the format assigns an `x-*` key no semantics at all, promotion is a +heuristic — invariant 6 applies to it in full. Four rules, so that no emitter has to re-derive +this from `Unmodeled` and no two derive it differently: 1. **The mapping is injectable policy, default-on and disableable**, per compiler. Its default contents are conventions, not standards: nothing in any specification says `x-deprecated-reason` @@ -1857,6 +1857,18 @@ definition, and the mapping is where that reading is stated — the promotion do the date to confirm it. No default key names `RemovalVersion`: a document stating a removal *version* names its own key, per rule 1. +`Enum.Closed` is the one target whose fact is stated by a key being *present* rather than by a +value, so nothing is read or reported there. Its default key, `x-extensible-enum`, writes the +member list as its own value, and a list of members says nothing about openness that the key +naming it has not already said; the promotion therefore clears `Closed` on presence. A boolean +value is the one shape that does state openness by itself, and an explicit `false` is read as +written rather than inverted. Only openness is ever promoted: a schema's `enum` is closed by +definition, so a document declares the open case or nothing, and the mapping names that fact +rather than the field's own polarity. A document that writes `x-extensible-enum` *instead* of +`enum` lowers to no `Enum` at all and there is no node to open — minting one from a vendor key +would be a compiler reading a member list out of an extension, not a promotion, so the entry is +left for a consumer that wants to. + ### 12.1 One structural home per declaration Documentation, deprecation, XML hints, examples, vendor extensions, validation-only keywords and diff --git a/docs/ir-spec-matrix.md b/docs/ir-spec-matrix.md index 685d0c33..83d206c4 100644 --- a/docs/ir-spec-matrix.md +++ b/docs/ir-spec-matrix.md @@ -30,7 +30,7 @@ the ones the next compiler will be first to bind to. | `negation` | Negation | ✅ not | — | — | — | — | ✅ not | — | — | | `enums-string` | Enums (string) | ✅ | ✅ | ✅ named members | ✅ enum | ✅ | ✅ | ⚠ | ⚠ atom unions | | `enums-numeric` | Enums (numeric, valued) | ✅ | ✅ | ✅ | ✅ intEnum | — | ✅ | ✅ | ⚠ int unions | -| `open-enums` | Open enums (unknown values allowed) | ⚠ anyOf trick | — | ⚠ union w/ string | ✅ (enums are open by default) | — | ⚠ | ✅ open (proto3/editions) / closed (proto2, per-enum feature) | ⚠ atom() fallback | +| `open-enums` | Open enums (unknown values allowed) | ⚠ anyOf trick, x-extensible-enum | — | ⚠ union w/ string | ✅ (enums are open by default) | — | ⚠ | ✅ open (proto3/editions) / closed (proto2, per-enum feature) | ⚠ atom() fallback | | `custom-scalars` | Custom scalars | ⚠ type+format | ⚠ | ✅ scalar extends | ⚠ traits | ✅ scalar | ⚠ | — | ✅ -type/-opaque | | `encoding-hints` | Wire encoding hints (@encode / format) | ✅ format | ✅ format | ✅ @encode | ✅ timestampFormat | — | ✅ | ✅ fixed/zigzag/packed/delimited | — (ETF fixed) | | `field-wire-ids` | Field wire IDs (numeric tags) | — | — | — | — | — | — | ✅ field numbers | ⚠ tuple positions | diff --git a/testdata/conformance/openapi/extension-promotion.golden.json b/testdata/conformance/openapi/extension-promotion.golden.json index 8b41116b..d4d0c4ee 100644 --- a/testdata/conformance/openapi/extension-promotion.golden.json +++ b/testdata/conformance/openapi/extension-promotion.golden.json @@ -252,6 +252,54 @@ } ], "types": { + "t/openapi/components/schemas/Fixed": { + "kind": "enum", + "id": "t/openapi/components/schemas/Fixed", + "name": { + "source": "Fixed", + "canonical": "fixed" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Fixed" + }, + "valueType": "string", + "members": [ + { + "name": { + "source": "one", + "canonical": "one" + }, + "value": { + "kind": "string", + "str": "one", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + }, + { + "name": { + "source": "two", + "canonical": "two" + }, + "value": { + "kind": "string", + "str": "two", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + } + ], + "closed": true, + "flags": false + }, "t/openapi/components/schemas/Old": { "kind": "model", "id": "t/openapi/components/schemas/Old", @@ -365,6 +413,126 @@ "positional": false, "inputOnly": false }, + "t/openapi/components/schemas/Shade": { + "kind": "enum", + "id": "t/openapi/components/schemas/Shade", + "name": { + "source": "Shade", + "canonical": "shade" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:x-extensible-enum": { + "reason": "vendor_extension", + "value": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Shade/x-extensible-enum" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Shade" + }, + "valueType": "string", + "members": [ + { + "name": { + "source": "light", + "canonical": "light" + }, + "value": { + "kind": "string", + "str": "light", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + }, + { + "name": { + "source": "dark", + "canonical": "dark" + }, + "value": { + "kind": "string", + "str": "dark", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + } + ], + "closed": true, + "flags": false + }, + "t/openapi/components/schemas/Size": { + "kind": "enum", + "id": "t/openapi/components/schemas/Size", + "name": { + "source": "Size", + "canonical": "size" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:x-extensible-enum": { + "reason": "vendor_extension", + "value": [ + "small", + "large" + ], + "provenance": { + "source": 0, + "pointer": "/components/schemas/Size/x-extensible-enum" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Size", + "inferred": "extension-promotion" + }, + "valueType": "string", + "members": [ + { + "name": { + "source": "small", + "canonical": "small" + }, + "value": { + "kind": "string", + "str": "small", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + }, + { + "name": { + "source": "large", + "canonical": "large" + }, + "value": { + "kind": "string", + "str": "large", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + } + ], + "closed": false, + "flags": false + }, "t/prim/string": { "kind": "primitive", "id": "t/prim/string", @@ -434,7 +602,7 @@ { "format": "openapi@3.1", "path": "extension-promotion.yaml", - "hash": "2433e30d378223aa920d20cb51cce28080a3a325c92f36dbbe7662470fbee376" + "hash": "d6cc084d89a9e2626e827271c565b828da3241cbf77c658fd57868a02954ff19" } ] } diff --git a/testdata/conformance/openapi/extension-promotion.yaml b/testdata/conformance/openapi/extension-promotion.yaml index cd0f88e2..af7bc15e 100644 --- a/testdata/conformance/openapi/extension-promotion.yaml +++ b/testdata/conformance/openapi/extension-promotion.yaml @@ -39,6 +39,17 @@ components: properties: p: {type: string, deprecated: true, x-deprecated-reason: field goes away} n: {type: string, deprecated: true, x-deprecated-reason: 7} + Size: + type: string + enum: [small, large] + x-extensible-enum: [small, large] + Shade: + type: string + enum: [light, dark] + x-extensible-enum: false + Fixed: + type: string + enum: [one, two] securitySchemes: k: type: apiKey From aa4a2fb462559f902d069349fd7c8ec033a81648 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:45:16 +0300 Subject: [PATCH 3/3] fix(compilers/openapi): read a bare x-extensible-enum as open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JSON null decodes into a bool without error and leaves it false, so a bare `x-extensible-enum:` — the presence-only spelling the reading exists for — read as the explicit false that is the one way a document declines, with no diagnostic. Decode into *bool so absence of a value is told apart from false, and pin the null shape beside the list, the true and the prose the test already covers. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- compilers/openapi/internal/lowering/promotion.go | 11 ++++++++--- compilers/openapi/internal/lowering/promotion_test.go | 3 +++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/compilers/openapi/internal/lowering/promotion.go b/compilers/openapi/internal/lowering/promotion.go index 03d5470d..05a7b393 100644 --- a/compilers/openapi/internal/lowering/promotion.go +++ b/compilers/openapi/internal/lowering/promotion.go @@ -220,12 +220,17 @@ func extensionText(raw ir.RawValue) (string, bool) { // naming it has not already said. A boolean is the one shape that does state // openness on its own, so an explicit false is read as written — a document // saying the set is not extensible, which is not something to invert. +// +// The target is *bool rather than bool because JSON null decodes into a bool +// without error and leaves it false, so a bare `x-extensible-enum:` — the +// presence-only spelling this reading exists for — would otherwise be read as +// the explicit false that is the one way to decline. func extensionOpenness(raw ir.RawValue) bool { - var open bool - if err := json.Unmarshal(raw, &open); err != nil { + var open *bool + if err := json.Unmarshal(raw, &open); err != nil || open == nil { return true } - return open + return *open } // markInferred adds one heuristic's name to a provenance, keeping any already diff --git a/compilers/openapi/internal/lowering/promotion_test.go b/compilers/openapi/internal/lowering/promotion_test.go index f0db9930..b1444518 100644 --- a/compilers/openapi/internal/lowering/promotion_test.go +++ b/compilers/openapi/internal/lowering/promotion_test.go @@ -335,6 +335,9 @@ func TestPromoteEnumOpenness_ClearsClosedAndMarksTheNode(t *testing.T) { {"the member list the convention writes", `["a","b"]`}, {"an explicit true", `true`}, {"a value that states nothing", `"whatever"`}, + // JSON null decodes into a plain bool as false, so this row is what + // separates the presence-only spelling from an explicit decline. + {"a bare key, which is the presence-only spelling", `null`}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) {