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 a672623f..05a7b393 100644 --- a/compilers/openapi/internal/lowering/promotion.go +++ b/compilers/openapi/internal/lowering/promotion.go @@ -38,8 +38,28 @@ 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" + + // 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 @@ -71,7 +91,12 @@ 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, + // 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, } } @@ -116,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. @@ -127,14 +190,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 { @@ -143,6 +211,28 @@ 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. +// +// 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 || open == 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 9ec944f9..b1444518 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) { @@ -264,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 @@ -274,21 +301,128 @@ 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"`}, + // 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) { + 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 b59513ed..b2521d63 100644 --- a/compilers/openapi/options.go +++ b/compilers/openapi/options.go @@ -56,6 +56,10 @@ const ( TargetDeprecationSince = lowering.TargetDeprecationSince // TargetDeprecationRemovalVersion fills ir.Deprecation.RemovalVersion. 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 32f135b0..15a7cdf3 100644 --- a/compilers/openapi/promotion_test.go +++ b/compilers/openapi/promotion_test.go @@ -84,16 +84,56 @@ 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") + 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 @@ -190,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: @@ -197,23 +244,85 @@ 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) - for _, got := range map[openapi.ExtensionTarget]string{ + 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. + 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, + openapi.TargetEnumOpen: filledWhen(!enum.Closed), + } + 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) + } + } +} + +// 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 +// 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..40f7be3c 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,8 +1823,9 @@ 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`, +document can state it is a vendor extension: +`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 @@ -1841,8 +1849,25 @@ 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. + +`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 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/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..d4d0c4ee 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" @@ -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": "aa16a7bd98de256e0feb2bf240903d08db7f651a6b802255d7d89a98a69bcadd" + "hash": "d6cc084d89a9e2626e827271c565b828da3241cbf77c658fd57868a02954ff19" } ] } diff --git a/testdata/conformance/openapi/extension-promotion.yaml b/testdata/conformance/openapi/extension-promotion.yaml index 30e78be2..af7bc15e 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": @@ -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 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