diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index b717fc6..a43d694 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -223,6 +223,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"}}, {"examples", assertExamples, []string{"examples"}}, {"docs-summary-desc", assertDocsSummaryDesc, []string{"docs-summary-description"}}, {"extensions-x", assertExtensionsX, []string{"vendor-extensions"}}, diff --git a/compilers/openapi/helpers_test.go b/compilers/openapi/helpers_test.go index 12723a2..b720767 100644 --- a/compilers/openapi/helpers_test.go +++ b/compilers/openapi/helpers_test.go @@ -115,7 +115,7 @@ func newLowerer(doc *load.Document, opts Options) *lowerer { // newRawLowerer builds a lowerer over a hand-constructed document, bypassing the // parser so nil slice/map entries (which the parser panics on) can be exercised. func newRawLowerer(doc *soa.OpenAPI) *lowerer { - return lowererOver(lowering.New(0, doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{})) + return lowererOver(lowering.New(0, doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{})) } // componentID is the stable TypeID of a components-named schema, or of a diff --git a/compilers/openapi/internal/auth/auth.go b/compilers/openapi/internal/auth/auth.go index 55891d2..c9e36aa 100644 --- a/compilers/openapi/internal/auth/auth.go +++ b/compilers/openapi/internal/auth/auth.go @@ -132,7 +132,10 @@ func lowerSecurityScheme(c lowering.Ctx, name string, ss *soa.SecurityScheme, return ir.AuthScheme{}, false, []ir.Diagnostic{mechanismRefusalDiag(c, name, missing, entry)} } diags = preserveUnreadFields(c, &scheme, ss, decl) - return scheme, true, append(diags, applySchemeExtensions(c, &scheme, ss, decl)...) + diags = append(diags, applySchemeExtensions(c, &scheme, ss, decl)...) + // After the extensions, whose entries are what a promotion reads. + return scheme, true, append(diags, + c.PromoteDeprecation(scheme.Unmodeled, scheme.Deprecation, &scheme.Provenance)...) } // applySchemeExtensions keeps the x-* of the securitySchemes entry and, for an diff --git a/compilers/openapi/internal/lowering/limits_test.go b/compilers/openapi/internal/lowering/limits_test.go index 2d76411..e5c655f 100644 --- a/compilers/openapi/internal/lowering/limits_test.go +++ b/compilers/openapi/internal/lowering/limits_test.go @@ -38,7 +38,7 @@ func TestNew_CarriesTheLimits(t *testing.T) { t.Parallel() limits := lowering.Limits{MaxEnumMembers: 12} - c := lowering.New(0, openapitest.DocDeclaring(), ir.SourceInfo{}, lowering.GroupByTags, limits, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, openapitest.DocDeclaring(), ir.SourceInfo{}, lowering.GroupByTags, limits, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) assert.Equal(t, limits, c.Limits) } diff --git a/compilers/openapi/internal/lowering/lowering.go b/compilers/openapi/internal/lowering/lowering.go index 199f796..5445f60 100644 --- a/compilers/openapi/internal/lowering/lowering.go +++ b/compilers/openapi/internal/lowering/lowering.go @@ -45,9 +45,9 @@ type Ctx struct { // Provenance. SrcIndex int // Grouping selects how operations are grouped into OperationGroups. It is one - // of the caller policies the context carries — the budgets and the streaming - // media list are the others; everything else here is a fact about the - // document. + // of the caller policies the context carries — the budgets, the streaming + // media list and the promotion mapping are the others; 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 @@ -70,6 +70,14 @@ type Ctx struct { // thing keeping the other maps here unexported is for. streaming map[string]bool + // 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 @@ -120,21 +128,25 @@ type Ctx struct { // and not another would classify one direction of an operation and not the // other. // +// The promotion policy is normalized here too, and copied rather than shared, +// so no lowering can write through the context into the map 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, limits Limits, streaming StreamingMedia, origin overlay.Origin) Ctx { +func New(srcIndex int, doc *soa.OpenAPI, src ir.SourceInfo, grouping GroupingStrategy, limits Limits, streaming StreamingMedia, promotions ExtensionPromotions, origin overlay.Origin) Ctx { return Ctx{ - Doc: doc, - Source: src, - SrcIndex: srcIndex, - Grouping: grouping, - Limits: limits, - schemas: declaredSchemaNames(doc), - streaming: streamingSet(streaming), - overlay: origin, + Doc: doc, + Source: src, + SrcIndex: srcIndex, + Grouping: grouping, + Limits: limits, + schemas: declaredSchemaNames(doc), + streaming: streamingSet(streaming), + promotions: promotionSet(promotions), + overlay: origin, } } diff --git a/compilers/openapi/internal/lowering/lowering_test.go b/compilers/openapi/internal/lowering/lowering_test.go index f6ce939..dcba0fe 100644 --- a/compilers/openapi/internal/lowering/lowering_test.go +++ b/compilers/openapi/internal/lowering/lowering_test.go @@ -81,7 +81,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{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, tc.doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) for _, n := range tc.declares { assert.True(t, c.DeclaresSchema(n), "%q is declared", n) } @@ -110,7 +110,7 @@ func TestNew_KeepsTheDocumentItWasGiven(t *testing.T) { doc := openapitest.DocDeclaring("User") src := ir.SourceInfo{Format: "openapi@3.1", Path: "spec.yaml", Hash: "abc"} - c := lowering.New(7, doc, src, lowering.GroupByPathPrefix, lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(7, doc, src, lowering.GroupByPathPrefix, lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) assert.Same(t, doc, c.Doc, "the document is referenced, never copied") assert.Equal(t, src, c.Source) @@ -127,7 +127,7 @@ func TestWithAuth_ExtendsACopy(t *testing.T) { t.Parallel() doc := openapitest.DocDeclaring("User") src := ir.SourceInfo{Format: "openapi@3.1", Path: "spec.yaml", Hash: "abc"} - before := lowering.New(7, doc, src, lowering.GroupByPathPrefix, lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + before := lowering.New(7, doc, src, lowering.GroupByPathPrefix, lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) schemes := map[ir.AuthID]ir.AuthScheme{"a/apiKey": {ID: "a/apiKey"}} after := before.WithAuth(schemes) @@ -190,7 +190,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{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{OpenAPI: tc.version}, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) assert.Equal(t, tc.want, c.ExclusiveBoundIsBoolean()) }) } @@ -202,7 +202,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, openapitest.DocDeclaring("User"), ir.SourceInfo{Path: "spec.yaml"}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, openapitest.DocDeclaring("User"), ir.SourceInfo{Path: "spec.yaml"}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) scope := c.RefScope() @@ -268,7 +268,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, openapitest.DocDeclaring(), src, "", lowering.Limits{}, lowering.StreamingMedia{}, origin) + c := lowering.New(0, openapitest.DocDeclaring(), src, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, origin) require.Len(t, c.Sources(), 2) assert.Equal(t, src, c.Sources()[0], "the source being lowered comes first") @@ -283,7 +283,7 @@ func TestSources_ListsOnlyTheSourceWhenNoOverlayApplied(t *testing.T) { t.Parallel() src := ir.SourceInfo{Format: "openapi@3.1", Path: "spec.yaml"} - c := lowering.New(0, openapitest.DocDeclaring(), src, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, openapitest.DocDeclaring(), src, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) assert.Equal(t, []ir.SourceInfo{src}, c.Sources()) } @@ -299,7 +299,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, openapitest.DocDeclaring(), ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, origin) + c := lowering.New(0, openapitest.DocDeclaring(), ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, 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 0000000..a672623 --- /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 0000000..9ec944f --- /dev/null +++ b/compilers/openapi/internal/lowering/promotion_test.go @@ -0,0 +1,294 @@ +package lowering_test + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "strconv" + "strings" + "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{}, "", lowering.Limits{}, lowering.StreamingMedia{}, 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") +} + +// 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() + entries, err := os.ReadDir(".") + require.NoError(t, err) + 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 + } + // 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 { + if lit, ok := stringLiteral(value, typed); ok { + unquoted, err := strconv.Unquote(lit) + 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 +} + +// 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 +// 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) + } +} diff --git a/compilers/openapi/internal/lowering/streaming_test.go b/compilers/openapi/internal/lowering/streaming_test.go index ca9c96b..24bbe0e 100644 --- a/compilers/openapi/internal/lowering/streaming_test.go +++ b/compilers/openapi/internal/lowering/streaming_test.go @@ -14,7 +14,7 @@ import ( // streamingCtx builds a context carrying nothing but the streaming policy. func streamingCtx(policy lowering.StreamingMedia) lowering.Ctx { - return lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.Limits{}, policy, overlay.Origin{}) + return lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.Limits{}, policy, lowering.ExtensionPromotions{}, overlay.Origin{}) } // TestMediaTypeStreams_AnswersFromThePolicy pins every answer the policy gives, diff --git a/compilers/openapi/internal/operation/budgets_test.go b/compilers/openapi/internal/operation/budgets_test.go index 2d20b49..7294e2c 100644 --- a/compilers/openapi/internal/operation/budgets_test.go +++ b/compilers/openapi/internal/operation/budgets_test.go @@ -35,7 +35,7 @@ webhooks: require.NoError(t, err) require.NotNil(t, loadedDoc) c := lowering.New(0, loadedDoc.Doc, loadedDoc.Source, lowering.GroupByTags, - lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) var anchors schema.AnchorIndex ctx, cancel := context.WithCancel(t.Context()) diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index 60ac806..e75ce66 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/compilers/openapi/internal/operation/content.go @@ -555,7 +555,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 c522328..5bcc772 100644 --- a/compilers/openapi/internal/operation/helpers_internal_test.go +++ b/compilers/openapi/internal/operation/helpers_internal_test.go @@ -54,13 +54,13 @@ func loweredFor(t *testing.T, src string) (*lowerer, []ir.Diagnostic) { require.NoError(t, err) require.NotNil(t, loadedDoc, "load returned no document: %+v", diags) return lowererOver(lowering.New(0, loadedDoc.Doc, loadedDoc.Source, - lowering.GroupByTags, lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{})), diags + lowering.GroupByTags, lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{})), diags } // newRawLowerer builds a fixture over a hand-constructed document, bypassing // the parser so nil slice/map entries can be exercised. func newRawLowerer(doc *soa.OpenAPI) *lowerer { - return lowererOver(lowering.New(0, doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{})) + return lowererOver(lowering.New(0, doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{})) } // lowerServiceSpec loads src and runs the phases the service walk needs beneath diff --git a/compilers/openapi/internal/operation/helpers_test.go b/compilers/openapi/internal/operation/helpers_test.go index 8995726..f4287e7 100644 --- a/compilers/openapi/internal/operation/helpers_test.go +++ b/compilers/openapi/internal/operation/helpers_test.go @@ -64,7 +64,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, lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, loadedDoc.Doc, loadedDoc.Source, grouping, lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) var anchors schema.AnchorIndex var acc compile.Diags acc.AppendAll(schema.LowerComponentSchemas(t.Context(), c, types, &anchors)) diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 9080554..698da45 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -378,6 +378,8 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd } op.Bindings = ir.OpBindings{HTTP: []ir.HTTPBinding{hb}} diags = append(diags, applyOperationExtensions(c, &op, src, decl)...) + // After the extensions are on the map, since that is what it reads. + diags = append(diags, c.PromoteDeprecation(op.Unmodeled, op.Deprecation, &op.Provenance)...) 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 94ec981..37a3102 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -268,6 +268,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 01cc8c3..ed7d2b3 100644 --- a/compilers/openapi/internal/schema/compose_internal_test.go +++ b/compilers/openapi/internal/schema/compose_internal_test.go @@ -37,7 +37,7 @@ func TestRefLastSegment(t *testing.T) { func TestMappingTargetID(t *testing.T) { t.Parallel() l := &lowerer{ - ctx: lowering.New(0, openapitest.DocDeclaring("Cat", "Dog", "A/B"), ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}), + ctx: lowering.New(0, openapitest.DocDeclaring("Cat", "Dog", "A/B"), ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}), out: &ir.Document{Types: ir.TypeRegistry{}}, } // A $ref to a declared component. @@ -64,7 +64,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, openapitest.DocDeclaring(""), ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + empty := lowering.New(0, openapitest.DocDeclaring(""), ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, 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 f6a32f7..ddd272c 100644 --- a/compilers/openapi/internal/schema/helpers_internal_test.go +++ b/compilers/openapi/internal/schema/helpers_internal_test.go @@ -83,7 +83,7 @@ func loweredFor(t *testing.T, src string) (*lowerer, []ir.Diagnostic) { require.NoError(t, err) require.NotNil(t, loadedDoc, "load returned no document: %+v", diags) return lowererOver(lowering.New(0, loadedDoc.Doc, loadedDoc.Source, - lowering.GroupByTags, lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{})), diags + lowering.GroupByTags, lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{})), diags } // lowerSpec loads src and lowers its component schemas, returning the document @@ -98,7 +98,7 @@ func lowerSpec(t *testing.T, src string) (*ir.Document, []ir.Diagnostic) { // newRawLowerer builds a fixture over a hand-constructed document, bypassing the // parser so nil slice/map entries (which the parser panics on) can be exercised. func newRawLowerer(doc *soa.OpenAPI) *lowerer { - return lowererOver(lowering.New(0, doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{})) + return lowererOver(lowering.New(0, doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{})) } // assertInternalInvariant requires diags to report a broken internal invariant. diff --git a/compilers/openapi/internal/schema/helpers_test.go b/compilers/openapi/internal/schema/helpers_test.go index 1b1d404..903c96f 100644 --- a/compilers/openapi/internal/schema/helpers_test.go +++ b/compilers/openapi/internal/schema/helpers_test.go @@ -72,7 +72,7 @@ func loweredFor(t *testing.T, src string) (*lowerer, []ir.Diagnostic) { require.NoError(t, err) require.NotNil(t, loadedDoc, "load returned no document: %+v", diags) return lowererOver(lowering.New(0, loadedDoc.Doc, loadedDoc.Source, - lowering.GroupByTags, lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{})), diags + lowering.GroupByTags, lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{})), diags } // lowerSpec loads src and lowers its component schemas, returning the document @@ -87,7 +87,7 @@ func lowerSpec(t *testing.T, src string) (*ir.Document, []ir.Diagnostic) { // newRawLowerer builds a fixture over a hand-constructed document, bypassing the // parser so nil slice/map entries (which the parser panics on) can be exercised. func newRawLowerer(doc *soa.OpenAPI) *lowerer { - return lowererOver(lowering.New(0, doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{})) + return lowererOver(lowering.New(0, doc, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{})) } // componentID is the stable TypeID of a components-named schema, or of a diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index adfb5c5..f344944 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1141,6 +1141,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)...) @@ -1226,6 +1227,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 968f70e..4b32145 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -3366,7 +3366,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{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) proto := ir.Example{Name: "n", Summary: "s", Description: "d"} out, diags := schema.AppendExample(c, nil, proto, openapitest.StrNode("hello"), "/p", "examples", "n") @@ -3384,7 +3384,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{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, 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") @@ -3401,7 +3401,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{}, "", lowering.Limits{}, lowering.StreamingMedia{}, overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, 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 8088daf..441990b 100644 --- a/compilers/openapi/openapi.go +++ b/compilers/openapi/openapi.go @@ -187,5 +187,6 @@ func loadOptions(o Options) load.Options { // loadOptions translates the other two — below here, zero means no budget. func loweringCtx(doc *load.Document, o Options) lowering.Ctx { limits := lowering.Limits{MaxEnumMembers: bounded(o.Limits.MaxEnumMembers)} - return lowering.New(rootSrcIndex, doc.Doc, doc.Source, o.Grouping, limits, o.StreamingMedia, doc.Overlay) + return lowering.New(rootSrcIndex, doc.Doc, doc.Source, o.Grouping, limits, + o.StreamingMedia, o.Promotions, doc.Overlay) } diff --git a/compilers/openapi/options.go b/compilers/openapi/options.go index f80f9d4..b59513e 100644 --- a/compilers/openapi/options.go +++ b/compilers/openapi/options.go @@ -30,8 +30,8 @@ const ( // StreamingMedia is the media-type streaming policy: which media types imply // that a body is a sequence of frames when the document declares nothing that -// says so. It is the second injectable-policy seam (architecture principle 6), -// and it is named here rather than restated for the reason GroupingStrategy is. +// says so. It is another injectable-policy seam (architecture principle 6), and it is +// named here rather than restated for the reason GroupingStrategy is. type StreamingMedia = lowering.StreamingMedia // DefaultStreamingMediaTypes returns the media types StreamingMedia classifies @@ -39,6 +39,32 @@ type StreamingMedia = lowering.StreamingMedia // the list can start from it rather than transcribe it. func DefaultStreamingMediaTypes() []string { return lowering.DefaultStreamingMediaTypes() } +// ExtensionPromotions is the vendor-extension promotion policy: which x-* keys +// are read into which typed IR field. It is another 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. @@ -54,6 +80,11 @@ type Options struct { // the default list, on; a caller who wants only what a document declares // disables it. StreamingMedia StreamingMedia `json:"streamingMedia"` + + // 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 0000000..7505274 --- /dev/null +++ b/compilers/openapi/promotion_test.go @@ -0,0 +1,210 @@ +// 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/compilers/openapi/internal/openapitest" + "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, openapitest.HasDiag(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 3312966..011e565 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1723,6 +1723,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 0000000..76707a1 --- /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 0000000..1455012 --- /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 e3c2634..f6a4bc2 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