From d316e5d1132a96e10b352562e8cc793e88647464 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 05:04:30 +0300 Subject: [PATCH 1/4] feat(compilers/openapi): fill typed fields from vendor extensions --- compilers/openapi/conformance_test.go | 1 + compilers/openapi/helpers_test.go | 2 +- compilers/openapi/internal/auth/auth.go | 4 +- .../openapi/internal/lowering/lowering.go | 33 +- .../internal/lowering/lowering_test.go | 16 +- .../openapi/internal/lowering/promotion.go | 179 ++++++++ .../internal/lowering/promotion_test.go | 194 +++++++++ .../openapi/internal/operation/content.go | 2 +- .../operation/helpers_internal_test.go | 4 +- .../internal/operation/helpers_test.go | 2 +- .../openapi/internal/operation/operations.go | 1 + .../openapi/internal/operation/params.go | 6 + .../internal/schema/compose_internal_test.go | 4 +- .../internal/schema/helpers_internal_test.go | 4 +- .../openapi/internal/schema/helpers_test.go | 4 +- compilers/openapi/internal/schema/schema.go | 2 + .../openapi/internal/schema/schema_test.go | 6 +- compilers/openapi/openapi.go | 2 +- compilers/openapi/options.go | 30 ++ compilers/openapi/promotion_test.go | 209 ++++++++++ docs/ir-design.md | 28 ++ .../openapi/extension-promotion.golden.json | 389 ++++++++++++++++++ .../openapi/extension-promotion.yaml | 41 ++ .../openapi/unwitnessed.golden.txt | 4 - 24 files changed, 1129 insertions(+), 38 deletions(-) create mode 100644 compilers/openapi/internal/lowering/promotion.go create mode 100644 compilers/openapi/internal/lowering/promotion_test.go create mode 100644 compilers/openapi/promotion_test.go create mode 100644 testdata/conformance/openapi/extension-promotion.golden.json create mode 100644 testdata/conformance/openapi/extension-promotion.yaml diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 17ae7d91..4ce15d0d 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -192,6 +192,7 @@ func conformanceCases() []conformanceCase { {"webhooks", assertWebhooks}, {"callbacks", assertCallbacks}, {"deprecation", assertDeprecation}, + {"extension-promotion", assertExtensionPromotion}, {"examples", assertExamples}, {"docs-summary-desc", assertDocsSummaryDesc}, {"extensions-x", assertExtensionsX}, diff --git a/compilers/openapi/helpers_test.go b/compilers/openapi/helpers_test.go index f1b32f18..648d6b68 100644 --- a/compilers/openapi/helpers_test.go +++ b/compilers/openapi/helpers_test.go @@ -155,7 +155,7 @@ func newLowerer(doc *load.Document, opts Options) *lowerer { func newRawLowerer(doc *soa.OpenAPI) *lowerer { rawTypes := compile.NewTypes(0) l := &lowerer{ - ctx: lowering.New(0, doc, ir.SourceInfo{}, "", overlay.Origin{}), + ctx: lowering.New(0, doc, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: rawTypes.Registry()}, types: rawTypes, operationIDs: make(map[string]string), diff --git a/compilers/openapi/internal/auth/auth.go b/compilers/openapi/internal/auth/auth.go index b04dbe96..8f4e2c70 100644 --- a/compilers/openapi/internal/auth/auth.go +++ b/compilers/openapi/internal/auth/auth.go @@ -134,7 +134,9 @@ func lowerSecurityScheme(c lowering.Ctx, name string, ss *soa.SecurityScheme, diags = preserveUnreadFields(c, &scheme, ss, decl) ext, extDiags := annotation.ExtensionsFrom(ss.GetExtensions(), c.SrcIndex, decl) scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, ext) - return scheme, true, append(diags, extDiags...) + diags = append(diags, extDiags...) + promoteDiags := c.PromoteDeprecation(scheme.Unmodeled, scheme.Deprecation, &scheme.Provenance) + return scheme, true, append(diags, promoteDiags...) } // mechanismRefusalDiag reports a securitySchemes entry that declares a scheme diff --git a/compilers/openapi/internal/lowering/lowering.go b/compilers/openapi/internal/lowering/lowering.go index df1d0e6e..87b20fe9 100644 --- a/compilers/openapi/internal/lowering/lowering.go +++ b/compilers/openapi/internal/lowering/lowering.go @@ -44,9 +44,9 @@ type Ctx struct { // SrcIndex is this source's index within the compile, stamped into every // Provenance. SrcIndex int - // Grouping selects how operations are grouped into OperationGroups. It is the - // only policy the context carries: everything else here is a fact about the - // document, and this is a fact about the caller. + // Grouping selects how operations are grouped into OperationGroups. It is one + // of the two caller policies the context carries; everything else here is a + // fact about the document. // // It arrives as the caller wrote it, normalized or not — the compiler's // Options fills an unset one in before building a context, but nothing here @@ -55,6 +55,14 @@ type Ctx struct { // than a second spelling of the default to keep in step. Grouping GroupingStrategy + // promotions is the vendor-extension promotion policy, normalized into the + // map PromoteDeprecation reads, and nil when the caller disabled it. + // + // It is the second caller policy, and it is unexported where Grouping is not + // because it holds a map: a struct copy would share it, which is the one + // thing keeping the other maps here unexported is for. + promotions map[string]ExtensionTarget + // schemas is the set of component-schema names the document declares. // // It is unexported and read through DeclaresSchema because a struct copy @@ -99,19 +107,24 @@ type Ctx struct { // document as a valid target. It stays nil for a document that declares no // components, which reads the same as an empty set. // +// The promotion policy is normalized into its map here for a related reason: +// the caller's map is copied once at entry rather than shared, so no lowering +// can write through the context into what the caller passed. +// // The $dynamicAnchor index is deliberately not derived here, though GitHub #172 // asked for it. Building it emits a diagnostic when the walk hits its bounds, so // building it is a lowering action rather than context: done at entry, that // warning would reach documents that never write $dynamicRef, changing what the // compiler reports about them. It stays where it is, built on first use. -func New(srcIndex int, doc *soa.OpenAPI, src ir.SourceInfo, grouping GroupingStrategy, origin overlay.Origin) Ctx { +func New(srcIndex int, doc *soa.OpenAPI, src ir.SourceInfo, grouping GroupingStrategy, promotions ExtensionPromotions, origin overlay.Origin) Ctx { return Ctx{ - Doc: doc, - Source: src, - SrcIndex: srcIndex, - Grouping: grouping, - schemas: declaredSchemaNames(doc), - overlay: origin, + Doc: doc, + Source: src, + SrcIndex: srcIndex, + Grouping: grouping, + promotions: promotionSet(promotions), + schemas: declaredSchemaNames(doc), + overlay: origin, } } diff --git a/compilers/openapi/internal/lowering/lowering_test.go b/compilers/openapi/internal/lowering/lowering_test.go index 707ac74f..254e06a2 100644 --- a/compilers/openapi/internal/lowering/lowering_test.go +++ b/compilers/openapi/internal/lowering/lowering_test.go @@ -94,7 +94,7 @@ func TestNew_DerivesTheDeclaredSchemaNames(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - c := lowering.New(0, tc.doc, ir.SourceInfo{}, "", overlay.Origin{}) + c := lowering.New(0, tc.doc, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}) for _, n := range tc.declares { assert.True(t, c.DeclaresSchema(n), "%q is declared", n) } @@ -123,7 +123,7 @@ func TestNew_KeepsTheDocumentItWasGiven(t *testing.T) { doc := docDeclaring("User") src := ir.SourceInfo{Format: "openapi@3.1", Path: "spec.yaml", Hash: "abc"} - c := lowering.New(7, doc, src, lowering.GroupByPathPrefix, overlay.Origin{}) + c := lowering.New(7, doc, src, lowering.GroupByPathPrefix, lowering.ExtensionPromotions{}, overlay.Origin{}) assert.Same(t, doc, c.Doc, "the document is referenced, never copied") assert.Equal(t, src, c.Source) @@ -140,7 +140,7 @@ func TestWithAuth_ExtendsACopy(t *testing.T) { t.Parallel() doc := docDeclaring("User") src := ir.SourceInfo{Format: "openapi@3.1", Path: "spec.yaml", Hash: "abc"} - before := lowering.New(7, doc, src, lowering.GroupByPathPrefix, overlay.Origin{}) + before := lowering.New(7, doc, src, lowering.GroupByPathPrefix, lowering.ExtensionPromotions{}, overlay.Origin{}) schemes := map[ir.AuthID]ir.AuthScheme{"a/apiKey": {ID: "a/apiKey"}} after := before.WithAuth(schemes) @@ -203,7 +203,7 @@ func TestExclusiveBoundIsBoolean_FollowsTheDialect(t *testing.T) { for _, tc := range tests { t.Run(tc.version, func(t *testing.T) { t.Parallel() - c := lowering.New(0, &soa.OpenAPI{OpenAPI: tc.version}, ir.SourceInfo{}, "", overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{OpenAPI: tc.version}, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}) assert.Equal(t, tc.want, c.ExclusiveBoundIsBoolean()) }) } @@ -215,7 +215,7 @@ func TestExclusiveBoundIsBoolean_FollowsTheDialect(t *testing.T) { // decides whether an internal pointer names anything. func TestRefScope_IsTheContextSeenAsAScope(t *testing.T) { t.Parallel() - c := lowering.New(0, docDeclaring("User"), ir.SourceInfo{Path: "spec.yaml"}, "", overlay.Origin{}) + c := lowering.New(0, docDeclaring("User"), ir.SourceInfo{Path: "spec.yaml"}, "", lowering.ExtensionPromotions{}, overlay.Origin{}) scope := c.RefScope() @@ -281,7 +281,7 @@ func TestSources_ListsTheOverlayAfterTheSourceItPatched(t *testing.T) { "overlay: 1.0.0\ninfo: {title: O, version: \"1\"}\nactions:\n"+ " - target: $.info\n update: {description: d}\n") - c := lowering.New(0, docDeclaring(), src, "", origin) + c := lowering.New(0, docDeclaring(), src, "", lowering.ExtensionPromotions{}, origin) require.Len(t, c.Sources(), 2) assert.Equal(t, src, c.Sources()[0], "the source being lowered comes first") @@ -296,7 +296,7 @@ func TestSources_ListsOnlyTheSourceWhenNoOverlayApplied(t *testing.T) { t.Parallel() src := ir.SourceInfo{Format: "openapi@3.1", Path: "spec.yaml"} - c := lowering.New(0, docDeclaring(), src, "", overlay.Origin{}) + c := lowering.New(0, docDeclaring(), src, "", lowering.ExtensionPromotions{}, overlay.Origin{}) assert.Equal(t, []ir.SourceInfo{src}, c.Sources()) } @@ -312,7 +312,7 @@ func TestProvenanceAt_NamesTheOverlayForThePositionsItIntroduced(t *testing.T) { "overlay: 1.0.0\ninfo: {title: O, version: \"1\"}\nactions:\n"+ " - target: $.info\n update: {description: d}\n") - c := lowering.New(0, docDeclaring(), ir.SourceInfo{}, "", origin) + c := lowering.New(0, docDeclaring(), ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, origin) assert.Equal(t, ir.Provenance{Source: 1, Pointer: "/info/description"}, c.ProvenanceAt("/info/description"), "the overlay introduced this position") diff --git a/compilers/openapi/internal/lowering/promotion.go b/compilers/openapi/internal/lowering/promotion.go new file mode 100644 index 00000000..a672623f --- /dev/null +++ b/compilers/openapi/internal/lowering/promotion.go @@ -0,0 +1,179 @@ +package lowering + +import ( + "encoding/json" + "maps" + "slices" + "strings" + + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/ir" +) + +// ExtensionPromotionHeuristic is the name Provenance.Inferred carries on a node +// whose typed field was read out of a vendor extension. It is a constant +// because the marker is what an auditor greps for, and a spelling written at +// the producing site and again at a reading test can drift. +const ExtensionPromotionHeuristic = "extension-promotion" + +// extensionKeyPrefix is the namespace an x-* key is kept under in Unmodeled. +// Promotion reads the preserved entry rather than the source node, so it has to +// spell the same namespace back. +const extensionKeyPrefix = "openapi:" + +// ExtensionTarget names one typed IR field a vendor extension can be read into. +// +// It is a closed vocabulary rather than a free-form path because a promotion +// has to be applied by code that knows the field's type, and a name nothing +// implements would be a policy that silently does nothing. +type ExtensionTarget string + +// The typed fields promotion can fill today. Every other field an extension is +// the only OpenAPI spelling for — Pagination, LongRunning, Idempotency, +// ErrorCase.Retryable/Throttling, Enum.Flags, EnumMember.Name, Sensitive and +// Secret — is a target this vocabulary is meant to grow, not a decision against +// it (GitHub #252). +const ( + // TargetDeprecationMessage fills ir.Deprecation.Message. + TargetDeprecationMessage ExtensionTarget = "deprecation.message" + // TargetDeprecationSince fills ir.Deprecation.Since. + TargetDeprecationSince ExtensionTarget = "deprecation.since" + // TargetDeprecationRemovalVersion fills ir.Deprecation.RemovalVersion. + TargetDeprecationRemovalVersion ExtensionTarget = "deprecation.removalVersion" +) + +// ExtensionPromotions is the vendor-extension promotion policy: which x-* keys +// are read into which typed IR field. +// +// It is a policy rather than a table in the lowering because OpenAPI assigns an +// x-* key no semantics whatsoever, so reading one as anything is a guess about +// a convention (architecture principle 6). A promoted field is marked +// ExtensionPromotionHeuristic in its node's provenance and the extension stays +// in Unmodeled untouched, which is what makes the guess auditable and +// reversible: a consumer that disagrees can ignore the typed field and read the +// entry itself. +type ExtensionPromotions struct { + // Disabled turns promotion off. Off means off: every extension is kept + // verbatim and no typed field is written from one. + Disabled bool `json:"disabled,omitempty"` + // Targets replaces the default map rather than extending it, so a caller who + // states a mapping gets exactly that mapping. Empty means the default. Keys + // are extension names as the document writes them, x- prefix included. + Targets map[string]ExtensionTarget `json:"targets,omitempty"` +} + +// DefaultExtensionPromotions is the mapping the policy uses when the caller +// states none. It is a default and not a standard: OpenAPI defines none of +// these keys, and each is simply the spelling that has become common for a +// field the format never gave a keyword. A document using another spelling is +// not wrong — it names its own mapping. +func DefaultExtensionPromotions() map[string]ExtensionTarget { + return map[string]ExtensionTarget{ + "x-deprecated-reason": TargetDeprecationMessage, + "x-deprecated-since": TargetDeprecationSince, + "x-sunset": TargetDeprecationRemovalVersion, + } +} + +// PromoteDeprecation fills dep's fields from the vendor extensions kept in +// unmodeled, and marks prov with the heuristic when it writes anything. +// +// It reads the preserved Unmodeled entries rather than the source node, which +// is what makes "the extension survives its own promotion" structural instead +// of a rule each call site has to remember: there is nothing here that could +// consume an entry. +// +// A nil dep is the whole answer for a node that is not deprecated — the field +// describes a deprecation, so an x-deprecated-reason beside no `deprecated: true` +// annotates nothing and stays where it is. +func (c Ctx) PromoteDeprecation(unmodeled ir.Unmodeled, dep *ir.Deprecation, prov *ir.Provenance) []ir.Diagnostic { + if dep == nil || prov == nil || len(unmodeled) == 0 || len(c.promotions) == 0 { + return nil + } + var diags []ir.Diagnostic + var promoted bool + // Sorted, so a policy naming two keys the document writes badly always + // reports the same one first. + for _, key := range slices.Sorted(maps.Keys(c.promotions)) { + field := deprecationField(dep, c.promotions[key]) + entry, declared := unmodeled[extensionKeyPrefix+key] + if field == nil || !declared { + continue + } + text, ok := extensionText(entry.Value) + if !ok { + diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, + entry.Provenance.Pointer, "extension %q is not a string, so it does not fill %s", + key, c.promotions[key])) + continue + } + *field = text + promoted = true + } + if promoted { + markInferred(prov, ExtensionPromotionHeuristic) + } + return diags +} + +// 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. +func deprecationField(dep *ir.Deprecation, target ExtensionTarget) *string { + switch target { + case TargetDeprecationMessage: + return &dep.Message + case TargetDeprecationSince: + return &dep.Since + case TargetDeprecationRemovalVersion: + return &dep.RemovalVersion + 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. +func extensionText(raw ir.RawValue) (string, bool) { + var text string + if err := json.Unmarshal(raw, &text); err != nil { + return "", false + } + return text, true +} + +// 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 +// reason was promoted is reached by two — so they are listed rather than one +// overwriting the other. +// +// Adding a name already listed is a no-op. A node reached by two references is +// annotated once per reference, and a marker repeated as many times as a +// component happens to be used would make the field depend on the document's +// reference count rather than on which heuristics ran. +func markInferred(prov *ir.Provenance, marker string) { + if prov.Inferred == "" { + prov.Inferred = marker + return + } + if slices.Contains(strings.Split(prov.Inferred, ","), marker) { + return + } + prov.Inferred += "," + marker +} + +// promotionSet normalizes a policy into the map PromoteDeprecation reads, or +// nil when promotion is off. The caller's map is copied: the context is passed +// by value and a shared map would be the one part of it a callee could write +// through. +func promotionSet(p ExtensionPromotions) map[string]ExtensionTarget { + if p.Disabled { + return nil + } + if len(p.Targets) == 0 { + return DefaultExtensionPromotions() + } + return maps.Clone(p.Targets) +} diff --git a/compilers/openapi/internal/lowering/promotion_test.go b/compilers/openapi/internal/lowering/promotion_test.go new file mode 100644 index 00000000..70cc59c4 --- /dev/null +++ b/compilers/openapi/internal/lowering/promotion_test.go @@ -0,0 +1,194 @@ +package lowering_test + +import ( + "testing" + + soa "github.com/speakeasy-api/openapi/openapi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/compilers/openapi/internal/overlay" + "github.com/dexpace/morphic/ir" +) + +// promotionCtx builds a context carrying nothing but the promotion policy. +func promotionCtx(policy lowering.ExtensionPromotions) lowering.Ctx { + return lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", policy, overlay.Origin{}) +} + +// vendorExtension is one preserved x-* entry, as ExtensionsFrom writes it. +func vendorExtension(rawJSON string) ir.UnmodeledEntry { + return ir.UnmodeledEntry{ + Reason: ir.ReasonVendorExtension, + Value: ir.RawValue(rawJSON), + Provenance: ir.Provenance{Pointer: "/components/schemas/S"}, + } +} + +// 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. +func TestPromoteDeprecation_FillsTheFieldsThePolicyNames(t *testing.T) { + t.Parallel() + tests := []struct { + name string + target lowering.ExtensionTarget + want ir.Deprecation + }{ + {"message", lowering.TargetDeprecationMessage, ir.Deprecation{Message: "why"}}, + {"since", lowering.TargetDeprecationSince, ir.Deprecation{Since: "why"}}, + {"removal version", lowering.TargetDeprecationRemovalVersion, ir.Deprecation{RemovalVersion: "why"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + c := promotionCtx(lowering.ExtensionPromotions{ + Targets: map[string]lowering.ExtensionTarget{"x-k": tc.target}, + }) + var dep ir.Deprecation + var prov ir.Provenance + diags := c.PromoteDeprecation(ir.Unmodeled{"openapi:x-k": vendorExtension(`"why"`)}, &dep, &prov) + + assert.Empty(t, diags) + assert.Equal(t, tc.want, dep) + assert.Equal(t, lowering.ExtensionPromotionHeuristic, prov.Inferred) + }) + } +} + +// TestPromoteDeprecation_LeavesTheEntryItRead is the losslessness half: the +// promotion is a second reading of a preserved entry, never a move, so a +// consumer that disagrees with the guess can still read what the document +// actually wrote. +func TestPromoteDeprecation_LeavesTheEntryItRead(t *testing.T) { + t.Parallel() + unmodeled := ir.Unmodeled{"openapi:x-deprecated-reason": vendorExtension(`"why"`)} + var dep ir.Deprecation + var prov ir.Provenance + promotionCtx(lowering.ExtensionPromotions{}).PromoteDeprecation(unmodeled, &dep, &prov) + + entry, kept := unmodeled["openapi:x-deprecated-reason"] + require.True(t, kept, "the entry survives its own promotion") + assert.Equal(t, ir.ReasonVendorExtension, entry.Reason) + assert.JSONEq(t, `"why"`, string(entry.Value)) +} + +// TestPromoteDeprecation_WritesNothing pins every shape that must leave the +// node exactly as it was. Each is a different reason, and a promotion that +// answered any of them by writing would put a value in the IR that the document +// did not say. +func TestPromoteDeprecation_WritesNothing(t *testing.T) { + t.Parallel() + filled := ir.Unmodeled{"openapi:x-deprecated-reason": vendorExtension(`"why"`)} + 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(`"why"`), + }}, + { + "a target no deprecation field answers to", + lowering.ExtensionPromotions{Targets: map[string]lowering.ExtensionTarget{ + "x-deprecated-reason": lowering.ExtensionTarget("pagination.items"), + }}, + filled, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var dep ir.Deprecation + var prov ir.Provenance + diags := promotionCtx(tc.policy).PromoteDeprecation(tc.unmodeled, &dep, &prov) + assert.Empty(t, diags) + assert.Equal(t, ir.Deprecation{}, dep) + assert.Empty(t, prov.Inferred, "nothing was inferred, so nothing is marked") + }) + } +} + +// TestPromoteDeprecation_UndeprecatedNodeIsTheWholeAnswer pins the nil cases, +// which are not an oversight to guard against but the ordinary shape: a node +// that never said it was deprecated has no Deprecation to fill, and its +// extension stays where it is. +func TestPromoteDeprecation_UndeprecatedNodeIsTheWholeAnswer(t *testing.T) { + t.Parallel() + c := promotionCtx(lowering.ExtensionPromotions{}) + unmodeled := ir.Unmodeled{"openapi:x-deprecated-reason": vendorExtension(`"why"`)} + assert.Empty(t, c.PromoteDeprecation(unmodeled, nil, &ir.Provenance{})) + + var dep ir.Deprecation + assert.Empty(t, c.PromoteDeprecation(unmodeled, &dep, nil)) + assert.Equal(t, ir.Deprecation{}, dep, "with nowhere to record the guess, none is made") +} + +// TestPromoteDeprecation_ValueThatIsNotTextIsReported pins the one thing a +// promotion reports. Every Deprecation field is prose or a version, so a value +// of another shape means the document uses the key for something else — which +// is a reason to leave the field empty and say so, not to coerce. +func TestPromoteDeprecation_ValueThatIsNotTextIsReported(t *testing.T) { + t.Parallel() + unmodeled := ir.Unmodeled{"openapi:x-deprecated-reason": vendorExtension(`7`)} + var dep ir.Deprecation + var prov ir.Provenance + diags := promotionCtx(lowering.ExtensionPromotions{}).PromoteDeprecation(unmodeled, &dep, &prov) + + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityInfo, diags[0].Severity) + assert.Equal(t, "openapi/degraded-construct", diags[0].Code) + assert.Equal(t, "/components/schemas/S", diags[0].Provenance.Pointer, + "the report names the extension rather than the node holding it") + assert.Equal(t, ir.Deprecation{}, dep) + assert.Empty(t, prov.Inferred) +} + +// TestPromoteDeprecation_MarksOnceBesideWhateverWasAlreadyThere pins the two +// halves of the marker. A heuristic already recorded must survive, since +// Provenance.Inferred holds one string; and a node annotated twice — which a +// component reached by two references is — must not accumulate the same name +// per reference. +func TestPromoteDeprecation_MarksOnceBesideWhateverWasAlreadyThere(t *testing.T) { + t.Parallel() + c := promotionCtx(lowering.ExtensionPromotions{}) + unmodeled := ir.Unmodeled{"openapi:x-deprecated-reason": vendorExtension(`"why"`)} + var dep ir.Deprecation + prov := ir.Provenance{Inferred: "group-path-prefix"} + + c.PromoteDeprecation(unmodeled, &dep, &prov) + assert.Equal(t, "group-path-prefix,extension-promotion", prov.Inferred) + + c.PromoteDeprecation(unmodeled, &dep, &prov) + assert.Equal(t, "group-path-prefix,extension-promotion", prov.Inferred, + "a second reading of the same node adds no second marker") +} + +// TestDefaultExtensionPromotions_IsCopiedNotShared holds the exported default +// to being a fresh map. A caller who starts from it and edits it must not be +// editing what the next compile reads. +func TestDefaultExtensionPromotions_IsCopiedNotShared(t *testing.T) { + t.Parallel() + first := lowering.DefaultExtensionPromotions() + require.NotEmpty(t, first, "an empty mapping would make this vacuous") + delete(first, "x-deprecated-reason") + assert.Contains(t, lowering.DefaultExtensionPromotions(), "x-deprecated-reason") +} + +// TestPromoteDeprecation_PolicyMapIsCopiedIntoTheContext pins the same rule for +// the caller's own map: the context is passed by value, so a map shared rather +// than copied would be the one part of it a lowering could write through. +func TestPromoteDeprecation_PolicyMapIsCopiedIntoTheContext(t *testing.T) { + t.Parallel() + targets := map[string]lowering.ExtensionTarget{"x-k": lowering.TargetDeprecationMessage} + c := promotionCtx(lowering.ExtensionPromotions{Targets: targets}) + delete(targets, "x-k") + + var dep ir.Deprecation + var prov ir.Provenance + c.PromoteDeprecation(ir.Unmodeled{"openapi:x-k": vendorExtension(`"why"`)}, &dep, &prov) + assert.Equal(t, "why", dep.Message, "the policy the context read is the one it was given") +} diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index c202731a..c8781c10 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/compilers/openapi/internal/operation/content.go @@ -512,7 +512,7 @@ func applyHeaderAnnotations(c lowering.Ctx, p *ir.Property, h *soa.Header, hdecl hExt, extDiags := schema.ExtensionsOf(c, h.GetExtensions(), hdecl) diags = append(diags, extDiags...) p.Unmodeled = annotation.MergeUnmodeled(p.Unmodeled, hExt) - return diags + return append(diags, c.PromoteDeprecation(p.Unmodeled, p.Deprecation, &p.Provenance)...) } // exampleList lowers a single example node and a plural example map into value diff --git a/compilers/openapi/internal/operation/helpers_internal_test.go b/compilers/openapi/internal/operation/helpers_internal_test.go index 75bc475a..81ec4a04 100644 --- a/compilers/openapi/internal/operation/helpers_internal_test.go +++ b/compilers/openapi/internal/operation/helpers_internal_test.go @@ -44,7 +44,7 @@ func loweredFor(t *testing.T, src string) (*lowerer, []ir.Diagnostic) { require.NotNil(t, loadedDoc, "load returned no document: %+v", diags) types := compile.NewTypes(0) return &lowerer{ - ctx: lowering.New(0, loadedDoc.Doc, loadedDoc.Source, lowering.GroupByTags, overlay.Origin{}), + ctx: lowering.New(0, loadedDoc.Doc, loadedDoc.Source, lowering.GroupByTags, lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: types.Registry()}, types: types, operationIDs: make(map[string]string), @@ -56,7 +56,7 @@ func loweredFor(t *testing.T, src string) (*lowerer, []ir.Diagnostic) { func newRawLowerer(doc *soa.OpenAPI) *lowerer { types := compile.NewTypes(0) return &lowerer{ - ctx: lowering.New(0, doc, ir.SourceInfo{}, "", overlay.Origin{}), + ctx: lowering.New(0, doc, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: types.Registry()}, types: types, operationIDs: make(map[string]string), diff --git a/compilers/openapi/internal/operation/helpers_test.go b/compilers/openapi/internal/operation/helpers_test.go index b8a0254c..8967bd5b 100644 --- a/compilers/openapi/internal/operation/helpers_test.go +++ b/compilers/openapi/internal/operation/helpers_test.go @@ -200,7 +200,7 @@ func serviceWithGrouping(t *testing.T, src string, grouping lowering.GroupingStr require.NotNil(t, loadedDoc) types := compile.NewTypes(0) - c := lowering.New(0, loadedDoc.Doc, loadedDoc.Source, grouping, overlay.Origin{}) + c := lowering.New(0, loadedDoc.Doc, loadedDoc.Source, grouping, lowering.ExtensionPromotions{}, overlay.Origin{}) var anchors schema.AnchorIndex var acc compile.Diags acc.AppendAll(schema.LowerComponentSchemas(c, types, &anchors)) diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 351fb057..b652c4ec 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -292,6 +292,7 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd if len(ext) > 0 { op.Unmodeled = ext } + diags = append(diags, c.PromoteDeprecation(op.Unmodeled, op.Deprecation, &op.Provenance)...) // After the extensions assignment, which would otherwise overwrite the map. diags = append(diags, applyOperationServers(c, &op, src, decl)...) return op, extra, append(diags, checkOperationIDUnique(c, operationIDs, op, mount)...) diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index e0875ee3..e11342c9 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -260,6 +260,12 @@ func paramHoldsResidue(keyword string) bool { // field is written only when the parameter declares it, so it overlays the // schema-derived annotations fillParamSchema already recorded rather than // erasing them with an unset value. +// +// It is the one carrier of an ir.Deprecation that does not promote a vendor +// extension into it: ir.Parameter has no Provenance, so there is nowhere to +// record that the field was read by a heuristic, and ir-design §12's promotion +// rules require that before the reading. Giving Parameter a provenance is a +// change to that document, not to this file (GitHub #252). func fillParamDetail(c lowering.Ctx, param *ir.Parameter, p *soa.Parameter, pptr string) []ir.Diagnostic { if d := p.GetDescription(); d != "" { param.Docs.Description = d diff --git a/compilers/openapi/internal/schema/compose_internal_test.go b/compilers/openapi/internal/schema/compose_internal_test.go index 75046e87..3c201c38 100644 --- a/compilers/openapi/internal/schema/compose_internal_test.go +++ b/compilers/openapi/internal/schema/compose_internal_test.go @@ -36,7 +36,7 @@ func TestRefLastSegment(t *testing.T) { func TestMappingTargetID(t *testing.T) { t.Parallel() l := &lowerer{ - ctx: lowering.New(0, docDeclaring("Cat", "Dog", "A/B"), ir.SourceInfo{}, "", overlay.Origin{}), + ctx: lowering.New(0, docDeclaring("Cat", "Dog", "A/B"), ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: ir.TypeRegistry{}}, } // A $ref to a declared component. @@ -63,7 +63,7 @@ func TestMappingTargetID(t *testing.T) { // (issue #14, f31). It gets a context of its own rather than being added to the // one above: the declared set is derived from the document now, so saying "and // also this one" means saying it to a document. - empty := lowering.New(0, docDeclaring(""), ir.SourceInfo{}, "", overlay.Origin{}) + empty := lowering.New(0, docDeclaring(""), ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}) id, ok = mappingTargetID(empty, l.types, "") require.True(t, ok) assert.Equal(t, ids.AnonType(ids.Ptr("components", "schemas", "")), id) diff --git a/compilers/openapi/internal/schema/helpers_internal_test.go b/compilers/openapi/internal/schema/helpers_internal_test.go index 8cbc6c17..9145e833 100644 --- a/compilers/openapi/internal/schema/helpers_internal_test.go +++ b/compilers/openapi/internal/schema/helpers_internal_test.go @@ -91,7 +91,7 @@ func loweredFor(t *testing.T, src string) (*lowerer, []ir.Diagnostic) { require.NotNil(t, loadedDoc, "load returned no document: %+v", diags) types := compile.NewTypes(0) return &lowerer{ - ctx: lowering.New(0, loadedDoc.Doc, loadedDoc.Source, lowering.GroupByTags, overlay.Origin{}), + ctx: lowering.New(0, loadedDoc.Doc, loadedDoc.Source, lowering.GroupByTags, lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: types.Registry()}, types: types, }, diags @@ -111,7 +111,7 @@ func lowerSpec(t *testing.T, src string) (*ir.Document, []ir.Diagnostic) { func newRawLowerer(doc *soa.OpenAPI) *lowerer { types := compile.NewTypes(0) return &lowerer{ - ctx: lowering.New(0, doc, ir.SourceInfo{}, "", overlay.Origin{}), + ctx: lowering.New(0, doc, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: types.Registry()}, types: types, } diff --git a/compilers/openapi/internal/schema/helpers_test.go b/compilers/openapi/internal/schema/helpers_test.go index 970699ed..8371c565 100644 --- a/compilers/openapi/internal/schema/helpers_test.go +++ b/compilers/openapi/internal/schema/helpers_test.go @@ -67,7 +67,7 @@ func loweredFor(t *testing.T, src string) (*lowerer, []ir.Diagnostic) { require.NotNil(t, loadedDoc, "load returned no document: %+v", diags) types := compile.NewTypes(0) return &lowerer{ - ctx: lowering.New(0, loadedDoc.Doc, loadedDoc.Source, lowering.GroupByTags, overlay.Origin{}), + ctx: lowering.New(0, loadedDoc.Doc, loadedDoc.Source, lowering.GroupByTags, lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: types.Registry()}, types: types, }, diags @@ -87,7 +87,7 @@ func lowerSpec(t *testing.T, src string) (*ir.Document, []ir.Diagnostic) { func newRawLowerer(doc *soa.OpenAPI) *lowerer { types := compile.NewTypes(0) return &lowerer{ - ctx: lowering.New(0, doc, ir.SourceInfo{}, "", overlay.Origin{}), + ctx: lowering.New(0, doc, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: types.Registry()}, types: types, } diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb9..c3422b9b 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -891,6 +891,7 @@ func fillPropertyAnnotations(c lowering.Ctx, ts *compile.Types, anchors *AnchorI p.Examples = a.Examples } p.Unmodeled = annotation.MergeUnmodeled(p.Unmodeled, a.Unmodeled) + diags = append(diags, c.PromoteDeprecation(p.Unmodeled, p.Deprecation, &p.Provenance)...) // nil node: this arm runs only when the schema lowered to no node of its own, // so nothing here can be carrying an Encoding. diags = append(diags, recordUnplacedContent(c, &p.Unmodeled, ref, nil, pointer)...) @@ -974,6 +975,7 @@ func attachDeclaredAnnotations(c lowering.Ctx, ts *compile.Types, anchors *Ancho common.XML = a.XML } common.Unmodeled = annotation.MergeUnmodeled(common.Unmodeled, a.Unmodeled) + diags = append(diags, c.PromoteDeprecation(common.Unmodeled, common.Deprecation, &common.Provenance)...) if len(a.Examples) > 0 { common.Examples = a.Examples } diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index f4b1c910..8b05aa86 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -3225,7 +3225,7 @@ func TestDynamicRef_NonScalarValueIsKeptNotExpanded(t *testing.T) { // prototype changes, so a site that fills in a name or a description keeps it. func TestAppendExample_ConvertsAndAppends(t *testing.T) { t.Parallel() - c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}) proto := ir.Example{Name: "n", Summary: "s", Description: "d"} out, diags := schema.AppendExample(c, nil, proto, strNode("hello"), "/p", "examples", "n") @@ -3243,7 +3243,7 @@ func TestAppendExample_ConvertsAndAppends(t *testing.T) { // that joins them, so a wrong join shows up nowhere else. func TestAppendExample_UnconvertibleValueIsReported(t *testing.T) { t.Parallel() - c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}) nan := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!float", Value: ".nan"} out, diags := schema.AppendExample(c, nil, ir.Example{}, nan, "/p", "examples", "n") @@ -3260,7 +3260,7 @@ func TestAppendExample_UnconvertibleValueIsReported(t *testing.T) { // it, not at the position that declared it. func TestStampConstraintDiags_RelocatesEveryDiagnosticToTheReadingPointer(t *testing.T) { t.Parallel() - c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.ExtensionPromotions{}, overlay.Origin{}) in := []ir.Diagnostic{ {Code: diag.DegradedConstruct, Provenance: ir.Provenance{Pointer: "/elsewhere"}}, {Code: diag.NumericPrecision, Provenance: ir.Provenance{Source: 9, Pointer: "/other"}}, diff --git a/compilers/openapi/openapi.go b/compilers/openapi/openapi.go index b1feb110..3e10dafc 100644 --- a/compilers/openapi/openapi.go +++ b/compilers/openapi/openapi.go @@ -158,5 +158,5 @@ func loadOptions(o Options) load.Options { // place the loader's result type meets the lowering, so lowering.New can take // the two facts it needs rather than the loader's struct. func loweringCtx(doc *load.Document, o Options) lowering.Ctx { - return lowering.New(rootSrcIndex, doc.Doc, doc.Source, o.Grouping, doc.Overlay) + return lowering.New(rootSrcIndex, doc.Doc, doc.Source, o.Grouping, o.Promotions, doc.Overlay) } diff --git a/compilers/openapi/options.go b/compilers/openapi/options.go index 35c6b812..ce322435 100644 --- a/compilers/openapi/options.go +++ b/compilers/openapi/options.go @@ -19,6 +19,32 @@ const ( GroupByPathPrefix = lowering.GroupByPathPrefix ) +// ExtensionPromotions is the vendor-extension promotion policy: which x-* keys +// are read into which typed IR field. It is the second injectable-policy seam +// (architecture principle 6), and is named here rather than restated for the +// reason GroupingStrategy is. +type ExtensionPromotions = lowering.ExtensionPromotions + +// ExtensionTarget names one typed IR field a promoted extension fills. +type ExtensionTarget = lowering.ExtensionTarget + +// The typed fields promotion can fill today. +const ( + // TargetDeprecationMessage fills ir.Deprecation.Message. + TargetDeprecationMessage = lowering.TargetDeprecationMessage + // TargetDeprecationSince fills ir.Deprecation.Since. + TargetDeprecationSince = lowering.TargetDeprecationSince + // TargetDeprecationRemovalVersion fills ir.Deprecation.RemovalVersion. + TargetDeprecationRemovalVersion = lowering.TargetDeprecationRemovalVersion +) + +// DefaultExtensionPromotions returns the extension-to-field mapping applied +// when the caller names none. It is exported so a caller changing the mapping +// can start from it rather than transcribe it. +func DefaultExtensionPromotions() map[string]ExtensionTarget { + return lowering.DefaultExtensionPromotions() +} + // Options configures the OpenAPI compiler. It is the concrete type this // compiler expects in compilers.Options.FormatOptions; the zero value is valid // and normalized by withDefaults. @@ -30,6 +56,10 @@ const ( type Options struct { // Grouping selects the operation-grouping strategy. Grouping GroupingStrategy `json:"grouping,omitempty"` + // Promotions selects which vendor extensions are read into typed IR fields. + // The zero value is the default mapping, on; a caller who wants extensions + // kept verbatim and nothing more disables it. + Promotions ExtensionPromotions `json:"promotions"` // AllowExternalRefs lets reference resolution leave the source document — // reading files off disk and fetching http(s) URLs. Off by default, because // compilers.Source is the whole input ("the caller loads bytes so compilation diff --git a/compilers/openapi/promotion_test.go b/compilers/openapi/promotion_test.go new file mode 100644 index 00000000..7d1f76bc --- /dev/null +++ b/compilers/openapi/promotion_test.go @@ -0,0 +1,209 @@ +// This file covers promotion: reading a vendor extension into the typed IR +// field it is the only OpenAPI spelling for. It belongs to no single source +// file — the policy is on the compiler's options, the reading is one function +// beneath both walks, and it is applied at every carrier that can record it. +package openapi_test // external test package — exercises only the public API + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/compilers/openapi" + "github.com/dexpace/morphic/ir" +) + +// promotionCarrier is one node the corpus spec deprecates, reduced to the three +// things a promotion touches: the field it fills, the provenance that has to +// record it, and the entries it must not consume. +type promotionCarrier struct { + deprecation *ir.Deprecation + provenance ir.Provenance + unmodeled ir.Unmodeled +} + +// promotionCarriers picks out every deprecated node extension-promotion.yaml +// declares. Naming them here rather than asserting one is what makes this the +// sweep: a construction site that stops promoting fails on its own row instead +// of being covered by a neighbour. +func promotionCarriers(t *testing.T, doc *ir.Document) map[string]promotionCarrier { + t.Helper() + op, ok := opByName(doc, "getX") + require.True(t, ok) + require.Len(t, op.Responses, 1) + require.Len(t, op.Responses[0].Headers, 1) + header := op.Responses[0].Headers[0] + + model, ok := doc.Types[namedID("Old")].(*ir.Model) + require.True(t, ok) + prop, ok := propByWire(model, "p") + require.True(t, ok) + + scheme, ok := doc.Auth["auth/openapi/components/securitySchemes/k"] + require.True(t, ok) + + return map[string]promotionCarrier{ + "operation": {op.Deprecation, op.Provenance, op.Unmodeled}, + "header": {header.Deprecation, header.Provenance, header.Unmodeled}, + "type": {model.Deprecation, model.Provenance, model.Unmodeled}, + "property": {prop.Deprecation, prop.Provenance, prop.Unmodeled}, + "auth scheme": {scheme.Deprecation, scheme.Provenance, scheme.Unmodeled}, + } +} + +// assertExtensionPromotion is the corpus row for GitHub #252: OpenAPI names no +// keyword for why something was deprecated, when, or until when, so an x-* key +// is the only spelling there is — and it used to sit in Unmodeled while +// ir.Deprecation stayed empty at every site the compiler builds one. +func assertExtensionPromotion(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { + want := map[string]string{ + "operation": "use getY instead", + "header": "header goes away", + "type": "replaced by New", + "property": "field goes away", + "auth scheme": "rotate to oauth", + } + for name, carrier := range promotionCarriers(t, doc) { + require.NotNil(t, carrier.deprecation, "%s is deprecated", name) + assert.Equal(t, want[name], carrier.deprecation.Message, "%s message", name) + assert.Equal(t, "extension-promotion", carrier.provenance.Inferred, + "%s records that the field was read by a heuristic", name) + entry, kept := carrier.unmodeled["openapi:x-deprecated-reason"] + require.True(t, kept, "%s keeps the extension it was promoted from", name) + assert.Equal(t, ir.ReasonVendorExtension, entry.Reason, + "%s promotion does not reclassify what it read", name) + } + + 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) + + assertPromotionDeclined(t, doc, diags) +} + +// 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 +// meaning something else by the key. +func assertPromotionDeclined(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { + live, ok := opByName(doc, "getY") + require.True(t, ok) + assert.Nil(t, live.Deprecation, "the operation is not deprecated") + assert.Contains(t, live.Unmodeled, "openapi:x-deprecated-reason") + assert.Empty(t, live.Provenance.Inferred, "nothing was inferred, so nothing is marked") + + model, ok := doc.Types[namedID("Old")].(*ir.Model) + require.True(t, ok) + numeric, ok := propByWire(model, "n") + require.True(t, ok) + require.NotNil(t, numeric.Deprecation) + assert.Empty(t, numeric.Deprecation.Message, "a non-string value fills no field") + assert.Contains(t, numeric.Unmodeled, "openapi:x-deprecated-reason") + assert.Empty(t, numeric.Provenance.Inferred) + assert.True(t, hasDiagCode(diags, "openapi/degraded-construct"), + "declining a value the policy cannot read is reported; got %+v", diags) +} + +// deprecatedOpSpec is one deprecated operation carrying the two extensions the +// default mapping reads, for the tests that vary the policy rather than the +// document. +const deprecatedOpSpec = `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /x: + get: + operationId: getX + deprecated: true + x-deprecated-reason: use getY instead + x-gone-in: "9.0.0" + responses: + "200": + description: ok +` + +// compilePromotionSpec compiles src with opts and requires no error diagnostic. +func compilePromotionSpec(t *testing.T, src string, opts openapi.Options) *ir.Document { + t.Helper() + doc, diags, err := openapi.New().Compile(t.Context(), + []compilers.Source{{Path: "spec.yaml", Data: []byte(src)}}, + compilers.Options{FormatOptions: opts}) + require.NoError(t, err) + require.NotNil(t, doc) + assertNoErrorDiags(t, diags) + return doc +} + +// TestPromotion_DisabledKeepsExtensionsAndNothingElse pins the off switch +// invariant 6 requires. Disabled means the typed field is not written at all, +// and the document compiles to exactly what it did before promotion existed. +func TestPromotion_DisabledKeepsExtensionsAndNothingElse(t *testing.T) { + t.Parallel() + doc := compilePromotionSpec(t, deprecatedOpSpec, + openapi.Options{Promotions: openapi.ExtensionPromotions{Disabled: true}}) + op, ok := opByName(doc, "getX") + require.True(t, ok) + require.NotNil(t, op.Deprecation) + assert.Empty(t, op.Deprecation.Message) + assert.Empty(t, op.Provenance.Inferred) + assert.Contains(t, op.Unmodeled, "openapi:x-deprecated-reason") +} + +// TestPromotion_TargetsReplaceTheDefaults pins that a stated mapping is the +// whole mapping. A caller whose documents spell the key differently gets their +// spelling, and does not silently keep the defaults beside it — which is the +// difference between a default and a standard. +func TestPromotion_TargetsReplaceTheDefaults(t *testing.T) { + t.Parallel() + own := openapi.Options{Promotions: openapi.ExtensionPromotions{ + Targets: map[string]openapi.ExtensionTarget{ + "x-gone-in": openapi.TargetDeprecationRemovalVersion, + }, + }} + doc := compilePromotionSpec(t, deprecatedOpSpec, own) + op, ok := opByName(doc, "getX") + require.True(t, ok) + require.NotNil(t, op.Deprecation) + assert.Equal(t, "9.0.0", op.Deprecation.RemovalVersion, "the caller's key is read") + assert.Empty(t, op.Deprecation.Message, "a default the caller replaced is not read") + assert.Equal(t, "extension-promotion", op.Provenance.Inferred) +} + +// TestPromotion_DefaultTargetsAreTheOnesApplied holds the exported mapping to +// the one the compiler applies. Two transcriptions of one mapping is one of +// them going stale unnoticed, and the exported one is what a caller changing it +// starts from. +func TestPromotion_DefaultTargetsAreTheOnesApplied(t *testing.T) { + t.Parallel() + defaults := openapi.DefaultExtensionPromotions() + require.NotEmpty(t, defaults, "an empty mapping would make this vacuous") + + spec := `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /x: + get: + operationId: getX + deprecated: true +` + for key := range defaults { + spec += " " + key + ": filled\n" + } + spec += ` responses: + "200": + description: ok +` + 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{ + 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") + } +} diff --git a/docs/ir-design.md b/docs/ir-design.md index a72ceea6..4a5ce202 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1629,6 +1629,34 @@ bindings at server, channel, operation and message level; `ProtocolDecl.Options` bare `RawValue`, not `UnmodeledEntry` — those entries are there because the source declared them where the IR expects them, so there is no reason to record and no unmodelled construct to locate. +#### 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: + +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` + means what its name suggests, so a caller may replace the mapping outright. +2. **The extension stays where it was.** A promotion is a second reading of a preserved + `Unmodeled` entry, never a move, and the entry keeps its `vendor_extension` reason. That is what + makes it reversible: a consumer that disagrees with the guess reads the entry instead. It also + makes losslessness independent of the policy — a disabled promotion loses nothing. +3. **The node records that it was inferred**, in its own `Provenance.Inferred`, naming the + heuristic. `Inferred` holds one string and a node can be reached by more than one heuristic, so + the names are listed rather than overwritten, and a name already listed is not repeated. +4. **A node with no `Provenance` is not promoted into.** `Parameter` is today's instance: it + carries a `Deprecation` and no provenance, so a promotion there could not satisfy rule 3, and a + heuristic that cannot be audited is worse than an empty field. Giving such a node a provenance + is a change to this document, and the promotion follows it rather than preceding it. + +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. + ### 12.1 One structural home per declaration Documentation, deprecation, XML hints, examples, vendor extensions, validation-only keywords and diff --git a/testdata/conformance/openapi/extension-promotion.golden.json b/testdata/conformance/openapi/extension-promotion.golden.json new file mode 100644 index 00000000..76707a19 --- /dev/null +++ b/testdata/conformance/openapi/extension-promotion.golden.json @@ -0,0 +1,389 @@ +{ + "irVersion": "0.3.0", + "name": "ExtensionPromotion", + "version": "1.0.0", + "docs": {}, + "services": [ + { + "id": "s/openapi/0", + "name": { + "source": "ExtensionPromotion", + "canonical": "extension_promotion" + }, + "docs": {}, + "groups": [ + { + "name": { + "hint": "default" + }, + "docs": {}, + "operations": [ + { + "id": "op/openapi/paths/~1x/get", + "name": { + "source": "getX", + "canonical": "get_x" + }, + "docs": {}, + "deprecation": { + "message": "use getY instead", + "since": "1.2.0", + "removalVersion": "2.0.0" + }, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "headers": [ + { + "id": "p/openapi/paths/~1x/get/responses/200/headers/X-Legacy", + "name": { + "source": "X-Legacy", + "canonical": "x_legacy" + }, + "wireName": "X-Legacy", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "deprecation": { + "message": "header goes away" + }, + "unmodeled": { + "openapi:x-deprecated-reason": { + "reason": "vendor_extension", + "value": "header goes away", + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/responses/200/headers/X-Legacy/x-deprecated-reason" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/responses/200/headers/X-Legacy", + "inferred": "extension-promotion" + } + } + ], + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "GET", + "uriTemplate": "/x", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "unmodeled": { + "openapi:x-deprecated-reason": { + "reason": "vendor_extension", + "value": "use getY instead", + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/x-deprecated-reason" + } + }, + "openapi:x-deprecated-since": { + "reason": "vendor_extension", + "value": "1.2.0", + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/x-deprecated-since" + } + }, + "openapi:x-sunset": { + "reason": "vendor_extension", + "value": "2.0.0", + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/x-sunset" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get", + "inferred": "extension-promotion" + } + }, + { + "id": "op/openapi/paths/~1y/get", + "name": { + "source": "getY", + "canonical": "get_y" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "GET", + "uriTemplate": "/y", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "unmodeled": { + "openapi:x-deprecated-reason": { + "reason": "vendor_extension", + "value": "annotates nothing", + "provenance": { + "source": 0, + "pointer": "/paths/~1y/get/x-deprecated-reason" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1y/get" + } + } + ] + } + ], + "auth": null, + "provenance": { + "source": 0 + } + } + ], + "types": { + "t/openapi/components/schemas/Old": { + "kind": "model", + "id": "t/openapi/components/schemas/Old", + "name": { + "source": "Old", + "canonical": "old" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "deprecation": { + "message": "replaced by New" + }, + "unmodeled": { + "openapi:x-deprecated-reason": { + "reason": "vendor_extension", + "value": "replaced by New", + "provenance": { + "source": 0, + "pointer": "/components/schemas/Old/x-deprecated-reason" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Old", + "inferred": "extension-promotion" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Old/properties/p", + "name": { + "source": "p", + "canonical": "p" + }, + "wireName": "p", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "deprecation": { + "message": "field goes away" + }, + "unmodeled": { + "openapi:x-deprecated-reason": { + "reason": "vendor_extension", + "value": "field goes away", + "provenance": { + "source": 0, + "pointer": "/components/schemas/Old/properties/p/x-deprecated-reason" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Old/properties/p", + "inferred": "extension-promotion" + } + }, + { + "id": "p/openapi/components/schemas/Old/properties/n", + "name": { + "source": "n", + "canonical": "n" + }, + "wireName": "n", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "deprecation": {}, + "unmodeled": { + "openapi:x-deprecated-reason": { + "reason": "vendor_extension", + "value": 7, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Old/properties/n/x-deprecated-reason" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Old/properties/n" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/prim/string": { + "kind": "primitive", + "id": "t/prim/string", + "name": {}, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0 + }, + "prim": "string" + } + }, + "auth": { + "auth/openapi/components/securitySchemes/k": { + "id": "auth/openapi/components/securitySchemes/k", + "name": { + "source": "k", + "canonical": "k" + }, + "kind": "apiKey", + "docs": {}, + "deprecation": { + "message": "rotate to oauth" + }, + "in": "header", + "keyName": "X-Key", + "unmodeled": { + "openapi:x-deprecated-reason": { + "reason": "vendor_extension", + "value": "rotate to oauth", + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/k/x-deprecated-reason" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/k", + "inferred": "extension-promotion" + } + } + }, + "servers": [ + { + "name": { + "hint": "server" + }, + "urlTemplate": "/", + "description": {}, + "auth": null + } + ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "extension \"x-deprecated-reason\" is not a string, so it does not fill deprecation.message", + "provenance": { + "source": 0, + "pointer": "/components/schemas/Old/properties/n/x-deprecated-reason" + } + } + ], + "sources": [ + { + "format": "openapi@3.1", + "path": "extension-promotion.yaml", + "hash": "1bade65b585c75be141198f5312404923b56b7fbcf36105d20f13e26da63c52f" + } + ] +} diff --git a/testdata/conformance/openapi/extension-promotion.yaml b/testdata/conformance/openapi/extension-promotion.yaml new file mode 100644 index 00000000..1455012b --- /dev/null +++ b/testdata/conformance/openapi/extension-promotion.yaml @@ -0,0 +1,41 @@ +openapi: 3.1.0 +info: {title: ExtensionPromotion, version: "1.0.0"} +paths: + /x: + get: + operationId: getX + deprecated: true + x-deprecated-reason: use getY instead + x-deprecated-since: "1.2.0" + x-sunset: "2.0.0" + responses: + "200": + description: ok + headers: + X-Legacy: + deprecated: true + x-deprecated-reason: header goes away + schema: {type: string} + /y: + get: + operationId: getY + x-deprecated-reason: annotates nothing + responses: + "200": + description: ok +components: + schemas: + Old: + type: object + deprecated: true + x-deprecated-reason: replaced by New + properties: + p: {type: string, deprecated: true, x-deprecated-reason: field goes away} + n: {type: string, deprecated: true, x-deprecated-reason: 7} + securitySchemes: + k: + type: apiKey + name: X-Key + in: header + deprecated: true + x-deprecated-reason: rotate to oauth diff --git a/testdata/conformance/openapi/unwitnessed.golden.txt b/testdata/conformance/openapi/unwitnessed.golden.txt index 0ef3569a..9ba471fc 100644 --- a/testdata/conformance/openapi/unwitnessed.golden.txt +++ b/testdata/conformance/openapi/unwitnessed.golden.txt @@ -23,9 +23,6 @@ Content.SchemaFormat CtorValue.Args CtorValue.Name CtorValue.Scalar -Deprecation.Message -Deprecation.RemovalVersion -Deprecation.Since Discriminator.Envelope Discriminator.EnvelopeValueName Discriminator.Index @@ -170,7 +167,6 @@ Property.WireID Property.WireNameByFormat ProtocolDecl.Name ProtocolDecl.Options -Provenance.Inferred Provenance.Source RPCBinding.FullMethod RPCBinding.IdempotencyLevel From 1361159142c438e555826c7ae3836d7c3d9d2516 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 12:10:29 +0300 Subject: [PATCH 2/4] test(compilers/openapi): hold the promotion vocabulary to its appliers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A target is a constant and an applier, and only the constant was held to anything. Declaring one without the other compiles, maps cleanly and promotes nothing — no field written, no diagnostic, no marker — which is precisely the shape every follow-up target arrives in: Pagination, Idempotency, Sensitive and the rest are constants waiting for an applier apiece. The declared set is read off the source rather than restated, so a constant added to the vocabulary reaches the check without anyone remembering to list it twice, and the read is held to finding something so a moved declaration fails instead of passing vacuously. A target whose 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, the way a new census keyword means adding its arm. --- .../internal/lowering/promotion_test.go | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/compilers/openapi/internal/lowering/promotion_test.go b/compilers/openapi/internal/lowering/promotion_test.go index a6b2840b..b7e6e6f7 100644 --- a/compilers/openapi/internal/lowering/promotion_test.go +++ b/compilers/openapi/internal/lowering/promotion_test.go @@ -1,6 +1,8 @@ package lowering_test import ( + "os" + "regexp" "testing" soa "github.com/speakeasy-api/openapi/openapi" @@ -192,3 +194,48 @@ func TestPromoteDeprecation_PolicyMapIsCopiedIntoTheContext(t *testing.T) { c.PromoteDeprecation(ir.Unmodeled{"openapi:x-k": vendorExtension(`"why"`)}, &dep, &prov) assert.Equal(t, "why", dep.Message, "the policy the context read is the one it was given") } + +// declaredTargets reads the ExtensionTarget constants off the source rather +// than restating them, so a target added to the vocabulary reaches the check +// below without anyone remembering to list it here too. +func declaredTargets(t *testing.T) []lowering.ExtensionTarget { + t.Helper() + src, err := os.ReadFile("promotion.go") + require.NoError(t, err) + matches := regexp.MustCompile(`ExtensionTarget = "([^"]+)"`).FindAllStringSubmatch(string(src), -1) + require.NotEmpty(t, matches, "no targets found; the check below would pass vacuously") + out := make([]lowering.ExtensionTarget, 0, len(matches)) + for _, m := range matches { + out = append(out, lowering.ExtensionTarget(m[1])) + } + return out +} + +// 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 +// promotes nothing — no field written, no diagnostic, no marker. +// +// That is the shape every follow-up target arrives in — Pagination, Idempotency, +// Sensitive and the rest are constants waiting for an applier apiece — so a +// 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. +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, + "%s is declared in the vocabulary but no applier fills it, so a policy naming it "+ + "promotes nothing and says nothing", target) + } +} From bc175440b051a7bdff8846dd42f1ddb821eed392 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 12:14:01 +0300 Subject: [PATCH 3/4] test(compilers/openapi): parse the target vocabulary instead of matching it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard read one file with a regex, so it missed a target declared in another file of the package and one spelled with different spacing — silently, in both cases. That is the failure the guard exists to prevent, reproduced inside the guard: a check that does not reach reads exactly like a check that passes. It parses the package now and takes every const whose declared type is ExtensionTarget, which is independent of file, spacing and grouping. All three spellings that slipped past the regex are caught, and the read is still held to finding something, so renaming the type fails rather than passing vacuously. --- .../internal/lowering/promotion_test.go | 49 +++++++++++++++---- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/compilers/openapi/internal/lowering/promotion_test.go b/compilers/openapi/internal/lowering/promotion_test.go index b7e6e6f7..40b33080 100644 --- a/compilers/openapi/internal/lowering/promotion_test.go +++ b/compilers/openapi/internal/lowering/promotion_test.go @@ -1,8 +1,12 @@ package lowering_test import ( + "go/ast" + "go/parser" + "go/token" "os" - "regexp" + "strconv" + "strings" "testing" soa "github.com/speakeasy-api/openapi/openapi" @@ -195,19 +199,44 @@ func TestPromoteDeprecation_PolicyMapIsCopiedIntoTheContext(t *testing.T) { assert.Equal(t, "why", dep.Message, "the policy the context read is the one it was given") } -// declaredTargets reads the ExtensionTarget constants off the source rather -// than restating them, so a target added to the vocabulary reaches the check -// below without anyone remembering to list it here too. +// declaredTargets returns every ExtensionTarget constant the package declares, +// parsed rather than matched: a regex over one file misses a target declared in +// another file of the package, or spelled with different spacing, and misses it +// silently — which is the failure this check exists to prevent, reproduced in +// the check itself. func declaredTargets(t *testing.T) []lowering.ExtensionTarget { t.Helper() - src, err := os.ReadFile("promotion.go") + entries, err := os.ReadDir(".") require.NoError(t, err) - matches := regexp.MustCompile(`ExtensionTarget = "([^"]+)"`).FindAllStringSubmatch(string(src), -1) - require.NotEmpty(t, matches, "no targets found; the check below would pass vacuously") - out := make([]lowering.ExtensionTarget, 0, len(matches)) - for _, m := range matches { - out = append(out, lowering.ExtensionTarget(m[1])) + var out []lowering.ExtensionTarget + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + file, err := parser.ParseFile(token.NewFileSet(), name, nil, 0) + require.NoError(t, err) + ast.Inspect(file, func(n ast.Node) bool { + spec, ok := n.(*ast.ValueSpec) + if !ok { + return true + } + if ident, ok := spec.Type.(*ast.Ident); !ok || ident.Name != "ExtensionTarget" { + return true + } + for _, value := range spec.Values { + lit, ok := value.(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + continue + } + unquoted, err := strconv.Unquote(lit.Value) + require.NoError(t, err) + out = append(out, lowering.ExtensionTarget(unquoted)) + } + return true + }) } + require.NotEmpty(t, out, "no targets found; the check below would pass vacuously") return out } From e36ee9db5f6d48badfd992a534224cfe087db950 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 12:26:00 +0300 Subject: [PATCH 4/4] test(compilers/openapi): read the conversion spelling of a target too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parse took the declared type only, so `X = ExtensionTarget("...")` — legal Go, and a target like any other — was read as no target at all. A vocabulary entry added that way would have promoted nothing with nothing to say about it, which is the defect this check exists for, missed by the check for the third time in three spellings. It reads both forms now. All four ways a target can be declared are caught: the typed const the file uses, the same with odd spacing, a var in another file of the package, and the conversion. The read is still held to finding something, so renaming the type fails rather than passing on an empty set. --- .../internal/lowering/promotion_test.go | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/compilers/openapi/internal/lowering/promotion_test.go b/compilers/openapi/internal/lowering/promotion_test.go index 40b33080..9ec944f9 100644 --- a/compilers/openapi/internal/lowering/promotion_test.go +++ b/compilers/openapi/internal/lowering/promotion_test.go @@ -221,17 +221,17 @@ func declaredTargets(t *testing.T) []lowering.ExtensionTarget { if !ok { return true } - if ident, ok := spec.Type.(*ast.Ident); !ok || ident.Name != "ExtensionTarget" { - return true - } + // Two spellings declare a target: the declared type carries it + // ("X ExtensionTarget = ..."), or a conversion does ("X = Extension- + // Target(...)"). Reading only the first missed the second silently, + // which is the defect this whole check is about. + typed := isTargetIdent(spec.Type) for _, value := range spec.Values { - lit, ok := value.(*ast.BasicLit) - if !ok || lit.Kind != token.STRING { - continue + if lit, ok := stringLiteral(value, typed); ok { + unquoted, err := strconv.Unquote(lit) + require.NoError(t, err) + out = append(out, lowering.ExtensionTarget(unquoted)) } - unquoted, err := strconv.Unquote(lit.Value) - require.NoError(t, err) - out = append(out, lowering.ExtensionTarget(unquoted)) } return true }) @@ -240,6 +240,30 @@ func declaredTargets(t *testing.T) []lowering.ExtensionTarget { return out } +// isTargetIdent reports whether a declared type names ExtensionTarget. +func isTargetIdent(expr ast.Expr) bool { + ident, ok := expr.(*ast.Ident) + return ok && ident.Name == "ExtensionTarget" +} + +// stringLiteral returns the quoted string a target declaration carries, from +// either spelling: the bare literal when the spec declared the type, or the +// argument of an ExtensionTarget conversion when it did not. +func stringLiteral(value ast.Expr, typed bool) (string, bool) { + if lit, ok := value.(*ast.BasicLit); ok && typed && lit.Kind == token.STRING { + return lit.Value, true + } + call, ok := value.(*ast.CallExpr) + if !ok || !isTargetIdent(call.Fun) || len(call.Args) != 1 { + return "", false + } + lit, ok := call.Args[0].(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + return "", false + } + return lit.Value, true +} + // 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