From 317498c00b2c1c717f732566833ec93673016754 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 04:05:22 +0300 Subject: [PATCH 1/3] fix(compilers/openapi): read x-* at every object that carries it --- compilers/openapi/conformance_test.go | 123 ++- .../openapi/conformance_unmodeled_test.go | 21 +- .../openapi/internal/annotation/annotation.go | 79 +- .../annotation/readers_internal_test.go | 52 ++ compilers/openapi/internal/auth/auth.go | 90 +- .../openapi/internal/operation/content.go | 96 ++- .../operation/content_internal_test.go | 3 +- .../internal/operation/content_test.go | 101 ++- .../openapi/internal/operation/operations.go | 101 ++- .../openapi/internal/schema/accumulate.go | 7 + compilers/openapi/meta.go | 115 ++- compilers/openapi/meta_test.go | 44 +- .../openapi/extensions-x.golden.json | 795 +++++++++++++++++- .../conformance/openapi/extensions-x.yaml | 70 +- .../openapi/multipart-encoding.golden.json | 21 +- .../openapi/multipart-encoding.yaml | 5 +- .../openapi/response-links.golden.json | 39 +- .../conformance/openapi/response-links.yaml | 10 + .../openapi/unwitnessed.golden.txt | 6 - 19 files changed, 1654 insertions(+), 124 deletions(-) diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 17ae7d91..40359acb 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -8,6 +8,7 @@ import ( "encoding/json" "os" "path/filepath" + "reflect" "slices" "strings" "testing" @@ -1677,6 +1678,12 @@ func assertFormPartStyle(t *testing.T, doc *ir.Document) { assert.True(t, *pe.Explode) assert.True(t, pe.Multi, "the structural flag still comes from the part's own schema") } + // allowReserved has no PartEncoding field, and PartEncoding has no Unmodeled + // map, so it is kept on the content keyed by the part it governs. Two + // documents differing only in it used to compile to one IR (GitHub #291). + entry := unmodeledEntry(t, op.Request.Contents[0].Unmodeled, "openapi:encoding/ids/allowReserved") + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, "true", string(entry.Value)) } // assertFileBody pins a binary body: the payload's type degrades to bytes and the @@ -2013,21 +2020,111 @@ func assertExtensionsX(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { assert.JSONEq(t, "100", string(raw.Value)) assert.Equal(t, ir.ReasonVendorExtension, raw.Reason) - // The same rule applies at every object that admits an extension, so the - // document root and an operation each keep their own. - root, ok := doc.Unmodeled["openapi:x-audience"] - require.True(t, ok, "a root extension lands on the document; got %v", doc.Unmodeled) - assert.Equal(t, ir.ReasonVendorExtension, root.Reason) - assert.JSONEq(t, `"public"`, string(root.Value)) + assertEveryObjectKeepsItsExtensions(t, doc) + assertRawPreservedBinary(t, m) +} - op, ok := opByName(doc, "listWidgets") - require.True(t, ok) - entry, ok := op.Unmodeled["openapi:x-internal"] - require.True(t, ok, "an operation extension lands on the operation; got %v", op.Unmodeled) - assert.Equal(t, ir.ReasonVendorExtension, entry.Reason) - assert.JSONEq(t, `true`, string(entry.Value)) +// assertEveryObjectKeepsItsExtensions holds the whole rule rather than the +// positions that happened to be noticed: every OpenAPI object that admits an +// x-* keeps it. The fixture writes `x-mark` on each, with a value naming the +// object, so a row that stops arriving names exactly which lowering stopped +// reading — and a lowering that never read one fails here before it ships. +// +// Carriers are derived from the value graph, not named: the row says which +// Unmodeled map the entry must land on by the path the walk reaches it at, so a +// carrier that moves still matches and an entry written to the wrong one does +// not. +func assertEveryObjectKeepsItsExtensions(t *testing.T, doc *ir.Document) { + t.Helper() + sites := unmodeledSites(doc) + for _, tc := range []struct{ object, key, want, carrier string }{ + {"openapi root", "openapi:x-audience", `"public"`, "doc.Unmodeled"}, + {"info", "openapi:info/x-mark", `"XINFO"`, "doc.Unmodeled"}, + {"contact", "openapi:info/contact/x-mark", `"XCONTACT"`, "doc.Unmodeled"}, + {"license", "openapi:info/license/x-mark", `"XLICENSE"`, "doc.Unmodeled"}, + {"externalDocs", "openapi:externalDocs/x-mark", `"XEXTERNALDOCS"`, "doc.Unmodeled"}, + {"components", "openapi:components/x-mark", `"XCOMPONENTS"`, "doc.Unmodeled"}, + {"tag", "openapi:tags/0/x-mark", `"XTAG"`, "doc.Unmodeled"}, + {"tag externalDocs", "openapi:tags/0/externalDocs/x-mark", `"XTAGEXTERNALDOCS"`, "doc.Unmodeled"}, + {"server", "openapi:x-mark", `"XSERVER"`, "doc.Servers[0].Unmodeled"}, + {"server variable", "openapi:x-mark", `"XSERVERVARIABLE"`, "doc.Servers[0].Variables[0].Unmodeled"}, + {"paths", "openapi:paths/x-mark", `"XPATHS"`, "doc.Services[0].Unmodeled"}, + {"path item", "openapi:pathItem/x-mark", `"XPATHITEM"`, ".Unmodeled"}, + {"operation", "openapi:x-internal", `true`, ".Unmodeled"}, + {"operation externalDocs", "openapi:externalDocs/x-mark", `"XOPERATIONEXTERNALDOCS"`, ".Unmodeled"}, + {"responses", "openapi:responses/x-mark", `"XRESPONSES"`, ".Unmodeled"}, + {"parameter", "openapi:x-mark", `"XPARAMETER"`, ".Params[0].Unmodeled"}, + {"request body", "openapi:x-mark", `"XREQUESTBODY"`, ".Request.Unmodeled"}, + {"media type", "openapi:x-mark", `"XMEDIATYPE"`, ".Contents[0].Unmodeled"}, + {"encoding", "openapi:encoding/f/x-mark", `"XENCODING"`, ".Contents[0].Unmodeled"}, + {"example", "openapi:x-mark", `"XEXAMPLE"`, ".Examples[0].Unmodeled"}, + {"response", "openapi:x-mark", `"XRESPONSE"`, ".Responses[0].Unmodeled"}, + {"error response", "openapi:x-mark", `"XERRORRESPONSE"`, ".Errors[0].Unmodeled"}, + {"header", "openapi:x-mark", `"XHEADER"`, ".Headers[0].Unmodeled"}, + {"callback", "openapi:callbacks/onEvent/x-mark", `"XCALLBACK"`, ".Bindings.HTTP[0].Unmodeled"}, + {"schema xml", "openapi:xml/x-mark", `"XXML"`, "doc.Types[t/openapi/components/schemas/S].Unmodeled"}, + {"schema externalDocs", "openapi:externalDocs/x-mark", `"XSCHEMAEXTERNALDOCS"`, + "doc.Types[t/openapi/components/schemas/S].Unmodeled"}, + {"discriminator", "openapi:discriminator/x-mark", `"XDISCRIMINATOR"`, + "doc.Types[t/openapi/components/schemas/D].Unmodeled"}, + {"security scheme", "openapi:x-mark", `"XSECURITYSCHEME"`, + "doc.Auth[auth/openapi/components/securitySchemes/k].Unmodeled"}, + {"oauth flows", "openapi:flows/x-mark", `"XOAUTHFLOWS"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Unmodeled"}, + {"oauth flow", "openapi:x-mark", `"XOAUTHFLOW"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Flows[0].Unmodeled"}, + } { + site, found := findUnmodeled(sites, tc.key, tc.want) + if !assert.True(t, found, "%s extension is dropped: no %s = %s anywhere in the document", + tc.object, tc.key, tc.want) { + continue + } + assert.Equal(t, ir.ReasonVendorExtension, site.entry.Reason, "%s extension reason", tc.object) + assert.Contains(t, site.path, tc.carrier, "%s extension lands on the wrong carrier", tc.object) + } +} - assertRawPreservedBinary(t, m) +// unmodeledSite is one Unmodeled entry paired with the walk path of the map +// holding it. +type unmodeledSite struct { + key string + path string + entry ir.UnmodeledEntry +} + +// unmodeledSites returns every Unmodeled entry the document holds. It walks the +// value graph rather than naming carriers so that a test asserting "this is kept +// somewhere sensible" cannot pass by looking only where it expected to. +func unmodeledSites(doc *ir.Document) []unmodeledSite { + unmodeledType := reflect.TypeOf(ir.Unmodeled(nil)) + var out []unmodeledSite + ir.WalkValues(doc, ir.DocumentPath, func(v reflect.Value, path string) bool { + if v.Type() != unmodeledType || !v.CanInterface() { + return true + } + u, ok := v.Interface().(ir.Unmodeled) + if !ok { + return true + } + for key, entry := range u { + out = append(out, unmodeledSite{key: key, path: path, entry: entry}) + } + return true + }) + return out +} + +// findUnmodeled returns the site holding key with the given JSON value. The +// value is part of the match because one key spelling occurs at many carriers — +// "openapi:x-mark" is written on a dozen objects in this fixture — so matching +// on the key alone would find a different object's entry and call it a pass. +func findUnmodeled(sites []unmodeledSite, key, wantJSON string) (unmodeledSite, bool) { + for _, site := range sites { + if site.key == key && string(site.entry.Value) == wantJSON { + return site, true + } + } + return unmodeledSite{}, false } // assertRawPreservedBinary pins what a !!binary extension keeps: the base64 the diff --git a/compilers/openapi/conformance_unmodeled_test.go b/compilers/openapi/conformance_unmodeled_test.go index 7244a8c5..f475d1f2 100644 --- a/compilers/openapi/conformance_unmodeled_test.go +++ b/compilers/openapi/conformance_unmodeled_test.go @@ -387,9 +387,15 @@ func assertResidue(t *testing.T, p ir.Unmodeled, want map[string]string) { } } -// assertResponseLinks pins a response's links: ir.Response has no field for the -// link objects OpenAPI declares there, so they are kept verbatim on the response -// rather than dropped while the operation they name lowers normally. +// assertResponseLinks pins a response's links: neither ir.Response nor +// ir.ErrorCase has a field for the link objects OpenAPI declares there, so they +// are kept verbatim rather than dropped while the operation they name lowers +// normally. +// +// Both status ranges, because only the success one used to keep them: the same +// declaration survived on a 2xx and vanished on a 4xx, with no diagnostic either +// way, purely because the error branch had no links rule of its own +// (GitHub #275). func assertResponseLinks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { op, ok := opByName(doc, "createOrder") require.True(t, ok) @@ -399,6 +405,15 @@ func assertResponseLinks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { assert.JSONEq(t, `{"GetOrder":{"operationId":"getOrder","parameters":{"orderId":"$response.body#/id"}}}`, string(entry.Value)) + + require.Len(t, op.Errors, 1) + errEntry := unmodeledEntry(t, op.Errors[0].Unmodeled, "openapi:links") + assert.Equal(t, ir.ReasonNoIRHome, errEntry.Reason) + assert.JSONEq(t, + `{"GetConflicting":{"operationId":"getOrder","parameters":{"orderId":"$response.body#/existingId"}}}`, + string(errEntry.Value), + "an error response keeps its links by the same rule the success one does") + _, ok = opByName(doc, "getOrder") assert.True(t, ok, "the operation a link names is an ordinary operation") } diff --git a/compilers/openapi/internal/annotation/annotation.go b/compilers/openapi/internal/annotation/annotation.go index 886b8f58..518973bc 100644 --- a/compilers/openapi/internal/annotation/annotation.go +++ b/compilers/openapi/internal/annotation/annotation.go @@ -175,9 +175,31 @@ func XMLHints(x *oas3.XML) *ir.XMLHints { // key beneath it and marked ReasonVendorExtension, since the format assigns an // x-* key no semantics at all. func ExtensionsFrom(ext *extensions.Extensions, srcIndex int, owner string) (ir.Unmodeled, []ir.Diagnostic) { + return ExtensionsUnder(ext, srcIndex, owner, "") +} + +// ExtensionsUnder is ExtensionsFrom with every entry keyed beneath scope, for +// the objects whose extensions have no Unmodeled map of their own to land on. +// +// Most OpenAPI objects lower to an IR node that carries one, and those pass an +// empty scope. The rest ride on the nearest node that does — an info object's on +// the document, an encoding's on the content, a path item's on each of its +// operations — and several of them can reach the same map, where "openapi:x-id" +// from two objects is one key and the surviving entry would depend on which +// lowering ran last. scope names which object wrote them: the source path from +// the carrier down to it, or the object's own keyword where it is not beneath +// the carrier at all. +// +// A scoped key cannot collide with an unscoped one on the same map, since only +// an x-* key reaches here and no scope begins with "x-". +func ExtensionsUnder(ext *extensions.Extensions, srcIndex int, owner, scope string) (ir.Unmodeled, []ir.Diagnostic) { if ext == nil || ext.Len() == 0 { return nil, nil } + prefix := "openapi:" + if scope != "" { + prefix += scope + "/" + } out := ir.Unmodeled{} var diags []ir.Diagnostic for name, node := range ext.All() { @@ -188,7 +210,7 @@ func ExtensionsFrom(ext *extensions.Extensions, srcIndex int, owner string) (ir. "extension %q could not be serialized", name)) continue } - out["openapi:"+name] = ir.UnmodeledEntry{ + out[prefix+name] = ir.UnmodeledEntry{ Reason: ir.ReasonVendorExtension, Value: raw, Provenance: ir.Provenance{Source: srcIndex, Pointer: owner + ids.Ptr(name)}, @@ -200,6 +222,30 @@ func ExtensionsFrom(ext *extensions.Extensions, srcIndex int, owner string) (ir. return out, diags } +// ExtensionSite is one object's x-* map paired with where it was written: Owner +// is the object's own source pointer, and Scope is what its entries key under on +// the carrier that ends up holding them (see ExtensionsUnder). +type ExtensionSite struct { + Scope string + Owner string + Ext *extensions.Extensions +} + +// ExtensionsAt folds every site into one Unmodeled map, for the carriers that +// hold more than one object's extensions. Sites are applied in the order given, +// which is source order at every caller; distinct scopes cannot collide, so the +// order decides nothing but is fixed anyway. +func ExtensionsAt(srcIndex int, sites ...ExtensionSite) (ir.Unmodeled, []ir.Diagnostic) { + var out ir.Unmodeled + var diags []ir.Diagnostic + for _, site := range sites { + ext, extDiags := ExtensionsUnder(site.Ext, srcIndex, site.Owner, site.Scope) + out = MergeUnmodeled(out, ext) + diags = append(diags, extDiags...) + } + return out, diags +} + // MergeUnmodeled overlays src onto dst, allocating dst on first write. func MergeUnmodeled(dst, src ir.Unmodeled) ir.Unmodeled { if len(src) == 0 { @@ -498,14 +544,41 @@ func Read(st Site, pointer string, srcIndex int) (Set, []ir.Diagnostic) { out.Examples = examples ext, extDiags := ExtensionsFrom(st.Node.GetExtensions(), srcIndex, pointer) + sub, subDiags := subObjectExtensions(st.Node, pointer, srcIndex) kept, keptDiags := unmodeledAt(st.Node, pointer, srcIndex) - diags := make([]ir.Diagnostic, 0, len(exDiags)+len(extDiags)+len(keptDiags)) + diags := make([]ir.Diagnostic, 0, len(exDiags)+len(extDiags)+len(subDiags)+len(keptDiags)) diags = append(diags, exDiags...) diags = append(diags, extDiags...) + diags = append(diags, subDiags...) diags = append(diags, keptDiags...) - out.Unmodeled = MergeUnmodeled(ext, kept) + out.Unmodeled = MergeUnmodeled(MergeUnmodeled(ext, sub), kept) + return out, diags +} + +// subObjectExtensions collects the x-* the sub-objects of a schema declare — +// its xml, its discriminator and its externalDocs. Each is an OpenAPI object +// that admits extensions, and none of ir.XMLHints, ir.Discriminator or ir.Link +// holds an Unmodeled map, so the entries ride on the node the schema itself +// lowers to; the keyword each was written under is what keeps three objects' +// extensions apart on that one map. +func subObjectExtensions(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, []ir.Diagnostic) { + subs := []struct { + keyword string + ext *extensions.Extensions + }{ + {"xml", s.GetXML().GetExtensions()}, + {"discriminator", s.GetDiscriminator().GetExtensions()}, + {"externalDocs", s.GetExternalDocs().GetExtensions()}, + } + var out ir.Unmodeled + var diags []ir.Diagnostic + for _, sub := range subs { + ext, extDiags := ExtensionsUnder(sub.ext, srcIndex, pointer+ids.Ptr(sub.keyword), sub.keyword) + out = MergeUnmodeled(out, ext) + diags = append(diags, extDiags...) + } return out, diags } diff --git a/compilers/openapi/internal/annotation/readers_internal_test.go b/compilers/openapi/internal/annotation/readers_internal_test.go index 4ffaf5f1..30bb7bc1 100644 --- a/compilers/openapi/internal/annotation/readers_internal_test.go +++ b/compilers/openapi/internal/annotation/readers_internal_test.go @@ -303,6 +303,58 @@ func TestExtensionsFrom_UnserializableIsWarnedNotKept(t *testing.T) { assert.Contains(t, diags[0].Message, `"x-bad"`) } +// TestExtensionsUnder_KeysBeneathTheScope pins the scoped spelling: an object +// with no Unmodeled map of its own keys its entries under the path that says +// which object wrote them, while the entry's provenance still points at the +// extension itself. +func TestExtensionsUnder_KeysBeneathTheScope(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: string\nx-a: 1\n") + + got, diags := ExtensionsUnder(s.GetExtensions(), 2, "/info/contact", "info/contact") + + assert.Empty(t, diags) + require.Len(t, got, 1) + require.Contains(t, got, "openapi:info/contact/x-a") + assert.Equal(t, ir.Provenance{Source: 2, Pointer: "/info/contact/x-a"}, + got["openapi:info/contact/x-a"].Provenance) +} + +// TestExtensionsAt_FoldsEverySiteWithoutCollision is the reason the scope +// exists: two objects writing the same x-* key onto one carrier must both +// survive, which one unscoped key cannot do. +func TestExtensionsAt_FoldsEverySiteWithoutCollision(t *testing.T) { + t.Parallel() + info := schemaFromYAML(t, "type: string\nx-a: 1\n") + license := schemaFromYAML(t, "type: string\nx-a: 2\n") + + got, diags := ExtensionsAt(0, + ExtensionSite{Scope: "info", Owner: "/info", Ext: info.GetExtensions()}, + ExtensionSite{Scope: "info/license", Owner: "/info/license", Ext: license.GetExtensions()}, + ExtensionSite{Scope: "components", Owner: "/components", Ext: nil}, + ) + + assert.Empty(t, diags) + require.Len(t, got, 2, "one key spelling, two objects, two entries") + assert.Equal(t, ir.RawValue("1"), got["openapi:info/x-a"].Value) + assert.Equal(t, ir.RawValue("2"), got["openapi:info/license/x-a"].Value) +} + +// TestExtensionsAt_ReportsEverySiteThatFailed holds the diagnostics to the same +// completeness as the entries: a site whose extension cannot be serialized keeps +// nothing, and the fold must still carry its warning out (GitHub #144's rule, +// applied to the multi-site form). +func TestExtensionsAt_ReportsEverySiteThatFailed(t *testing.T) { + t.Parallel() + bad := schemaFromYAML(t, "type: string\nx-bad: .nan\n") + + got, diags := ExtensionsAt(0, ExtensionSite{Scope: "info", Owner: "/info", Ext: bad.GetExtensions()}) + + assert.Nil(t, got, "nothing was kept, so there is no map to emit") + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityWarning, diags[0].Severity) +} + // TestMergeUnmodeled_AllocatesOnlyOnFirstWrite pins the overlay: merging nothing // leaves the destination exactly as it was — including nil, which must not // become an empty map. diff --git a/compilers/openapi/internal/auth/auth.go b/compilers/openapi/internal/auth/auth.go index b04dbe96..e701ca3f 100644 --- a/compilers/openapi/internal/auth/auth.go +++ b/compilers/openapi/internal/auth/auth.go @@ -132,9 +132,42 @@ 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) - ext, extDiags := annotation.ExtensionsFrom(ss.GetExtensions(), c.SrcIndex, decl) + return scheme, true, append(diags, applySchemeExtensions(c, &scheme, ss, decl)...) +} + +// applySchemeExtensions keeps the x-* of the securitySchemes entry and, for an +// oauth2 scheme, of the flows object and of each flow inside it. ir.OAuthFlow +// carries an Unmodeled map of its own; the flows object does not lower to a node +// at all, so its own extensions are kept on the scheme under the keyword. +// +// Only oauth2 reads inside `flows`: on any other type the whole node is kept +// verbatim by preserveUnreadFields, extensions and all, and no ir.OAuthFlow was +// lowered for a flow's own to land on. +func applySchemeExtensions(c lowering.Ctx, scheme *ir.AuthScheme, ss *soa.SecurityScheme, decl string) []ir.Diagnostic { + ext, diags := annotation.ExtensionsFrom(ss.GetExtensions(), c.SrcIndex, decl) scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, ext) - return scheme, true, append(diags, extDiags...) + if scheme.Kind != ir.AuthKindOAuth2 { + return diags + } + flowsPtr := decl + ids.Ptr("flows") + flowsExt, flowsDiags := annotation.ExtensionsUnder(ss.GetFlows().GetExtensions(), c.SrcIndex, flowsPtr, "flows") + scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, flowsExt) + diags = append(diags, flowsDiags...) + return append(diags, applyFlowExtensions(c, scheme.Flows, ss.GetFlows(), flowsPtr)...) +} + +// applyFlowExtensions writes each declared flow's own x-* onto the ir.OAuthFlow +// it lowered to. Both lists are one ordered reading of the same object — +// scheme.Flows came from oauthFlows, which walks presentFlows — so the i-th +// lowered flow is the i-th declared one and the two cannot differ in length. +func applyFlowExtensions(c lowering.Ctx, lowered []ir.OAuthFlow, flows *soa.OAuthFlows, flowsPtr string) []ir.Diagnostic { + var diags []ir.Diagnostic + for i, f := range presentFlows(flows) { + ext, extDiags := annotation.ExtensionsFrom(f.src.GetExtensions(), c.SrcIndex, flowsPtr+ids.Ptr(f.keyword)) + lowered[i].Unmodeled = annotation.MergeUnmodeled(lowered[i].Unmodeled, ext) + diags = append(diags, extDiags...) + } + return diags } // mechanismRefusalDiag reports a securitySchemes entry that declares a scheme @@ -312,28 +345,47 @@ func preserveUnreadFields(c lowering.Ctx, scheme *ir.AuthScheme, ss *soa.Securit return diags } +// sourceFlow is one declared OAuth2 flow: the IR kind it lowers to, the keyword +// it is written under, and the object itself. +type sourceFlow struct { + kind string + keyword string + src *soa.OAuthFlow +} + +// presentFlows returns the flows an OAuthFlows object declares, in a fixed, +// deterministic order. It is the single ordered reading of that object: the +// lowering and the extension reader both walk it, which is what lets the i-th +// ir.OAuthFlow be matched back to the i-th declaration without a second, and +// possibly divergent, enumeration. +func presentFlows(flows *soa.OAuthFlows) []sourceFlow { + candidates := []sourceFlow{ + {"authorization_code", "authorizationCode", flows.GetAuthorizationCode()}, + {"client_credentials", "clientCredentials", flows.GetClientCredentials()}, + {"implicit", "implicit", flows.GetImplicit()}, + {"password", "password", flows.GetPassword()}, + {"device", "deviceAuthorization", flows.GetDeviceAuthorization()}, + } + var out []sourceFlow + for _, cand := range candidates { + if cand.src != nil { + out = append(out, cand) + } + } + return out +} + // oauthFlows lowers each present OAuth2 flow in a fixed, deterministic order. // The device flow's deviceAuthorizationUrl rides OAuthFlow.AuthorizationURL // (ir-design §9). func oauthFlows(flows *soa.OAuthFlows) []ir.OAuthFlow { - if flows == nil { - return nil - } var out []ir.OAuthFlow - if f := flows.GetAuthorizationCode(); f != nil { - out = append(out, oauthFlow("authorization_code", f)) - } - if f := flows.GetClientCredentials(); f != nil { - out = append(out, oauthFlow("client_credentials", f)) - } - if f := flows.GetImplicit(); f != nil { - out = append(out, oauthFlow("implicit", f)) - } - if f := flows.GetPassword(); f != nil { - out = append(out, oauthFlow("password", f)) - } - if f := flows.GetDeviceAuthorization(); f != nil { - out = append(out, deviceFlow(f)) + for _, f := range presentFlows(flows) { + if f.kind == "device" { + out = append(out, deviceFlow(f.src)) + continue + } + out = append(out, oauthFlow(f.kind, f.src)) } return out } diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index c202731a..67753d68 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/compilers/openapi/internal/operation/content.go @@ -76,11 +76,12 @@ func lowerContent(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex content.File = &ir.FileInfo{IsText: false, ContentTypes: []string{mt}} content.Type = ts.PrimRef(ir.PrimBytes) case isFormContent(mt): - enc, encDiags := partEncodings(c, ts, anchors, media, mediaPtr, content.Type.Target) + enc, encUnmodeled, encDiags := partEncodings(c, ts, anchors, media, mediaPtr, content.Type.Target) diags = append(diags, encDiags...) if len(enc) > 0 { content.Encoding = enc } + content.Unmodeled = annotation.MergeUnmodeled(content.Unmodeled, encUnmodeled) } diags = append(diags, fillSequential(c, ts, anchors, &content, media, mediaPtr, hint)...) ext, extDiags := schema.ExtensionsOf(c, media.GetExtensions(), mediaPtr) @@ -113,9 +114,10 @@ func fillSequential(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd if enc == nil { return diags } - pe, encDiags := encodingConfig(c, ts, anchors, enc, mediaPtr+ids.Ptr("itemEncoding")) + pe, unmodeled, encDiags := encodingConfig(c, ts, anchors, enc, mediaPtr+ids.Ptr("itemEncoding"), "itemEncoding") pe.Multi = true content.ItemEncoding = &pe + content.Unmodeled = annotation.MergeUnmodeled(content.Unmodeled, unmodeled) return append(diags, encDiags...) } @@ -154,10 +156,10 @@ func positionalEncoding(c lowering.Ctx, content *ir.Content, media *soa.MediaTyp // body-model property's PropID. A part is included when it carries an explicit // encoding entry or is itself a repeated (array) or file (binary) part. body is // the TypeID the content's own schema position lowered to. -func partEncodings(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, media *soa.MediaType, mediaPtr string, body ir.TypeID) (map[ir.PropID]ir.PartEncoding, []ir.Diagnostic) { +func partEncodings(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, media *soa.MediaType, mediaPtr string, body ir.TypeID) (map[ir.PropID]ir.PartEncoding, ir.Unmodeled, []ir.Diagnostic) { parts := bodyParts(media.GetSchema(), 0) if len(parts) == 0 { - return nil, nil + return nil, nil, nil } // Key by the pointer the body model was interned at, not by mediaPtr, so the // keys align with that model's property IDs (invariant 2/3). Asking the IR is @@ -168,20 +170,25 @@ func partEncodings(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde schemaPtr = bodySchemaPointer(c, media.GetSchema(), mediaPtr+ids.Ptr("schema")) } var diags []ir.Diagnostic + var unmodeled ir.Unmodeled encMap := media.GetEncoding() out := map[ir.PropID]ir.PartEncoding{} for _, part := range parts { - pe, partDiags := buildPartEncoding(c, ts, anchors, part.name, part.schema, encMap, mediaPtr) + pe, partUnmodeled, partDiags := buildPartEncoding(c, ts, anchors, part.name, part.schema, encMap, mediaPtr) diags = append(diags, partDiags...) + // Before the emptiness check, not after it: an entry declaring only + // allowReserved or only x-* lowers to an empty PartEncoding that is left out + // of the map, and what it did declare would go with it. + unmodeled = annotation.MergeUnmodeled(unmodeled, partUnmodeled) if partEncodingEmpty(pe) { continue } out[partPropID(ts, body, part.name, schemaPtr)] = pe } if len(out) == 0 { - return nil, diags + return nil, unmodeled, diags } - return out, diags + return out, unmodeled, diags } // bodyPart is one multipart part: the name keying its encoding entry, and the @@ -304,28 +311,35 @@ func propIDInComposition(ts *compile.Types, m *ir.Model, wire string, depth int) // buildPartEncoding assembles one part's PartEncoding: explicit encoding config // (content types, headers, style, explode) merged with the structural flags Multi // (array part) and Filename (binary/file part). -func buildPartEncoding(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, name string, pjs *oas3.JSONSchema[oas3.Referenceable], encMap *sequencedmap.Map[string, *soa.Encoding], mediaPtr string) (ir.PartEncoding, []ir.Diagnostic) { +func buildPartEncoding(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, name string, pjs *oas3.JSONSchema[oas3.Referenceable], encMap *sequencedmap.Map[string, *soa.Encoding], mediaPtr string) (ir.PartEncoding, ir.Unmodeled, []ir.Diagnostic) { pe := ir.PartEncoding{} + var unmodeled ir.Unmodeled var diags []ir.Diagnostic if encMap != nil { if enc, ok := encMap.Get(name); ok { - pe, diags = encodingConfig(c, ts, anchors, enc, mediaPtr+ids.Ptr("encoding", name)) + pe, unmodeled, diags = encodingConfig(c, ts, anchors, enc, + mediaPtr+ids.Ptr("encoding", name), "encoding/"+name) } } if part := schemaOf(pjs); part != nil { pe.Multi = schemaIsArray(part) pe.Filename = schemaIsFilePart(part) } - return pe, diags + return pe, unmodeled, diags } // encodingConfig lowers one Encoding object's declared wire config: content // types, per-part headers, and form-style serialization. The structural flags // (Multi, Filename) come from the part's own schema, not from here. -func encodingConfig(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, enc *soa.Encoding, encPtr string) (ir.PartEncoding, []ir.Diagnostic) { +// +// It returns what the object declared that PartEncoding has no field for as a +// second value rather than writing it, because PartEncoding carries no +// Unmodeled map of its own; scope is where that belongs on the owning Content +// (see encodingUnmodeled). +func encodingConfig(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, enc *soa.Encoding, encPtr, scope string) (ir.PartEncoding, ir.Unmodeled, []ir.Diagnostic) { pe := ir.PartEncoding{} if enc == nil { - return pe, nil + return pe, nil, nil } pe.ContentTypes = splitContentTypes(enc.GetContentTypeValue()) headers, diags := lowerHeaders(c, ts, anchors, enc.GetHeaders(), encPtr) @@ -334,7 +348,36 @@ func encodingConfig(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd pe.Style = string(*enc.Style) } pe.Explode = enc.Explode - return pe, diags + unmodeled, encDiags := encodingUnmodeled(c, enc, encPtr, scope) + return pe, unmodeled, append(diags, encDiags...) +} + +// encodingUnmodeled keeps what an Encoding Object declares that nothing in the +// IR holds: `allowReserved`, which ir.PartEncoding has no field for even though +// its neighbours style and explode do, and the object's own x-*. Neither had +// reached an IR field, an Unmodeled entry or a diagnostic, so two documents +// differing only in them compiled to one IR (GitHub #291). +// +// Both ride on the owning ir.Content, since PartEncoding carries no Unmodeled +// map, keyed under scope — "encoding/" or "itemEncoding". One content can +// hold an entry per multipart part plus a sequential item's, and they reach the +// same map, so the part is what tells them apart. +// +// allowReserved carries ReasonNoIRHome and announces itself only when something +// was written, the shape preserveHeaderSerialization already uses for the pair +// beside it. +func encodingUnmodeled(c lowering.Ctx, enc *soa.Encoding, encPtr, scope string) (ir.Unmodeled, []ir.Diagnostic) { + var out ir.Unmodeled + at := encPtr + ids.Ptr("allowReserved") + kept, diags := schema.PreserveNode(c, &out, "openapi:"+scope+"/allowReserved", + annotation.RawChildNode(enc.GetRootNode(), "allowReserved"), ir.ReasonNoIRHome, at) + if kept { + diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, at, + "encoding allowReserved has no ir.PartEncoding home; kept verbatim under Unmodeled")) + } + ext, extDiags := schema.ExtensionsIn(c, enc.GetExtensions(), encPtr, scope) + out = annotation.MergeUnmodeled(out, ext) + return out, append(diags, extDiags...) } // lowerHeaders lowers a header map into Properties in source order. Each @@ -547,16 +590,32 @@ func exampleList(c lowering.Ctx, single *yaml.Node, plural *sequencedmap.Map[str // is de-referenced: an enclosing $ref'd response or parameter is already // flattened into pointer. func appendPluralExample(c lowering.Ctx, out []ir.Example, re *soa.ReferencedExample, pointer, name string) ([]ir.Example, []ir.Diagnostic) { - ex := resolve.Object[soa.Example](re) + // The declaration pointer, not the entry's, is where an Example Object's own + // keywords are written: a $ref entry holds none of them. ir.Example carries an + // Unmodeled map, so the object's x-* need no scope. + ex, decl := resolve.ObjectAt[soa.Example](c.RefScope(), re, pointer+ids.Ptr("examples", name)) if ex == nil { return out, nil } + ext, diags := schema.ExtensionsOf(c, ex.GetExtensions(), decl) proto := ir.Example{ Name: name, Summary: ex.GetSummary(), Description: ex.GetDescription(), ExternalURL: ex.GetExternalValue(), + Unmodeled: ext, } + out, exDiags := appendExampleValue(c, out, proto, ex, re, pointer, name) + return out, append(diags, exDiags...) +} + +// appendExampleValue appends the entry's value under the annotations proto +// already carries, stamping the failure pointer where the value is written: at +// the reference site for a $ref entry, which holds no `value` node of its own, +// and at its own `value` for an inline one. +func appendExampleValue(c lowering.Ctx, out []ir.Example, proto ir.Example, ex *soa.Example, + re *soa.ReferencedExample, pointer, name string, +) ([]ir.Example, []ir.Diagnostic) { node := ex.GetValue() if node == nil { return appendValuelessExample(c, out, proto, pointer, name) @@ -604,6 +663,15 @@ func lowerRequestBody(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, bodyPtr, "request body is not required; optionality kept under Unmodeled")) } + // soa.RequestBody exposes no GetExtensions at this library version, so the + // field is read directly — as XMLHints already reads its own. The read sits + // after the payload guard because ir.Payload is the body's only carrier: a + // request body declaring no content lowers to nothing to hang them on, and + // OpenAPI makes content REQUIRED there, so such a body is a defect in the + // document rather than a shape this compiler has to place. + bodyExt, bodyExtDiags := schema.ExtensionsOf(c, rb.Extensions, bodyPtr) + payload.Unmodeled = annotation.MergeUnmodeled(payload.Unmodeled, bodyExt) + diags = append(diags, bodyExtDiags...) op.Request = payload hb.RequestContentTypes = contentTypeKeys(rb.GetContent()) return diags diff --git a/compilers/openapi/internal/operation/content_internal_test.go b/compilers/openapi/internal/operation/content_internal_test.go index 966050a7..ceeebc5f 100644 --- a/compilers/openapi/internal/operation/content_internal_test.go +++ b/compilers/openapi/internal/operation/content_internal_test.go @@ -102,8 +102,9 @@ func TestFillSequential_EmptyItemEncoding(t *testing.T) { func TestEncodingConfig_NilEncoding(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{}) - pe, diags := encodingConfig(l.ctx, l.types, &l.anchors, nil, "/mp") + pe, unmodeled, diags := encodingConfig(l.ctx, l.types, &l.anchors, nil, "/mp", "itemEncoding") assert.Equal(t, ir.PartEncoding{}, pe) + assert.Nil(t, unmodeled) assert.Empty(t, diags) } diff --git a/compilers/openapi/internal/operation/content_test.go b/compilers/openapi/internal/operation/content_test.go index 4273d5d7..1c5eb9d9 100644 --- a/compilers/openapi/internal/operation/content_test.go +++ b/compilers/openapi/internal/operation/content_test.go @@ -386,7 +386,7 @@ paths: "200": description: ok content: - application/jsonl: + multipart/mixed: itemSchema: {type: object, properties: {a: {type: string}}} itemEncoding: {contentType: application/json} x-note: streamy @@ -474,6 +474,105 @@ func TestContent_SequentialAndEmptyBody(t *testing.T) { assert.Nil(t, empty.Request) } +// encodingSpec declares one form part per position an Encoding Object reaches: +// a multipart part, and a 3.2 sequential itemEncoding. Each writes the two +// things ir.PartEncoding has no field for, so both positions are exercised by +// the one fixture. +const encodingSpec = ` /form: + post: + operationId: postForm + requestBody: + required: true + content: + application/x-www-form-urlencoded: + schema: {type: object, properties: {q: {type: string}}} + encoding: + q: {style: form, allowReserved: true, x-vendor: vvv} + responses: {"200": {description: ok}} +` + +// TestEncoding_AllowReservedAndExtensionsKeptOnTheContent pins both halves of +// GitHub #291. Neither allowReserved nor an encoding's x-* reached an IR field, +// an Unmodeled entry or a diagnostic, so two documents differing only in them +// compiled to the same IR. PartEncoding carries no Unmodeled map, so both land +// on the owning Content keyed by the part they govern. +func TestEncoding_AllowReservedAndExtensionsKeptOnTheContent(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, pathsSpecVer("3.1.0", encodingSpec)) + requireNoErrorDiags(t, diags) + op := firstOp(t, svc) + require.NotNil(t, op.Request) + require.Len(t, op.Request.Contents, 1) + kept := op.Request.Contents[0].Unmodeled + + at := "/paths/~1form/post/requestBody/content/application~1x-www-form-urlencoded/encoding/q" + entry, ok := kept["openapi:encoding/q/allowReserved"] + require.True(t, ok, "allowReserved is kept; got %v", kept) + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, "true", string(entry.Value)) + assert.Equal(t, at+"/allowReserved", entry.Provenance.Pointer) + assertInfoDiagAt(t, diags, at+"/allowReserved") + + ext, ok := kept["openapi:encoding/q/x-vendor"] + require.True(t, ok, "the encoding's own x-* is kept; got %v", kept) + assert.Equal(t, ir.ReasonVendorExtension, ext.Reason) + assert.Equal(t, at+"/x-vendor", ext.Provenance.Pointer) +} + +// TestEncoding_AbsentAllowReservedRecordsNothing is the control: preservation +// keys off what the encoding declares, so an entry that omits allowReserved +// records neither an entry nor a diagnostic — rather than recording the OpenAPI +// default the accessor would hand back and calling it a source fact. +func TestEncoding_AbsentAllowReservedRecordsNothing(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, pathsSpecVer("3.1.0", ` /form: + post: + operationId: postForm + requestBody: + required: true + content: + application/x-www-form-urlencoded: + schema: {type: object, properties: {q: {type: string}}} + encoding: + q: {style: form} + responses: {"200": {description: ok}} +`)) + requireNoErrorDiags(t, diags) + op := firstOp(t, svc) + require.NotNil(t, op.Request) + require.Len(t, op.Request.Contents, 1) + assert.Empty(t, op.Request.Contents[0].Unmodeled, "nothing was declared, so nothing is kept") + assert.Empty(t, diags) +} + +// TestEncoding_ItemEncodingKeepsItsOwnUnderItsOwnKey is the other position the +// same reader serves. A content can hold both a per-part encoding map and a +// sequential itemEncoding, and both reach one Unmodeled map, so the key has to +// tell them apart. +func TestEncoding_ItemEncodingKeepsItsOwnUnderItsOwnKey(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, pathsSpecVer("3.2.0", ` /stream: + get: + operationId: streamEvents + responses: + "200": + description: ok + content: + multipart/mixed: + itemSchema: {type: object, properties: {n: {type: string}}} + itemEncoding: {contentType: application/json, allowReserved: true, x-vendor: vvv} +`)) + requireNoErrorDiags(t, diags) + op := firstOp(t, svc) + require.NotEmpty(t, op.Responses) + require.NotNil(t, op.Responses[0].Payload) + require.Len(t, op.Responses[0].Payload.Contents, 1) + kept := op.Responses[0].Payload.Contents[0].Unmodeled + + assert.Contains(t, kept, "openapi:itemEncoding/allowReserved") + assert.Contains(t, kept, "openapi:itemEncoding/x-vendor") +} + // multipartEncoding returns the part-encoding map of an operation's request. func multipartEncoding(t *testing.T, op ir.Operation) map[ir.PropID]ir.PartEncoding { t.Helper() diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 351fb057..d9f007dd 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -59,6 +59,13 @@ func LowerService(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex } svcAuth, diags := auth.LowerSecurityRequirements(c, c.Doc.GetSecurity(), "") svc.Auth = svcAuth + // The Paths Object admits x-* of its own, distinct from any path item's, and + // lowers to no node — its entries become operations. The service is the + // nearest node holding an Unmodeled map, so they are kept there under the + // keyword they were written at. + pathsExt, pathsDiags := schema.ExtensionsIn(c, c.Doc.GetPaths().GetExtensions(), ids.Ptr("paths"), "paths") + svc.Unmodeled = annotation.MergeUnmodeled(svc.Unmodeled, pathsExt) + diags = append(diags, pathsDiags...) groups := newServiceGroups() diags = append(diags, lowerPaths(c, ts, anchors, operationIDs, groups)...) diags = append(diags, lowerWebhooks(c, ts, anchors, operationIDs, groups)...) @@ -135,7 +142,7 @@ func lowerPathItem(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde } op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, declPtr)...) + diags = append(diags, applyPathItem(c, &op, pi, declPtr)...) grp := groups.group(key, func() ir.OperationGroup { return ir.OperationGroup{Name: name, Docs: docs} }) grp.Operations = append(grp.Operations, op) grp.Operations = append(grp.Operations, extra...) @@ -173,7 +180,7 @@ func lowerWebhooks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde } op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, declPtr)...) + diags = append(diags, applyPathItem(c, &op, pi, declPtr)...) grp := groups.group("webhook", func() ir.OperationGroup { // A hint, not a source name: no document declares this group. The // compiler synthesizes it to hold webhook operations, exactly as it @@ -282,21 +289,39 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd diags = append(diags, lowerRequestBody(c, ts, anchors, &op, &hb, src, decl)...) var extra []ir.Operation if opCtx.withCallbacks { + var cbExt ir.Unmodeled var cbDiags []ir.Diagnostic - hb.Callbacks, extra, cbDiags = lowerCallbacks(c, ts, anchors, operationIDs, src, opCtx.ptrs, opCtx.inferred) + hb.Callbacks, extra, cbExt, cbDiags = lowerCallbacks(c, ts, anchors, operationIDs, src, opCtx.ptrs, opCtx.inferred) + hb.Unmodeled = annotation.MergeUnmodeled(hb.Unmodeled, cbExt) diags = append(diags, cbDiags...) } op.Bindings = ir.OpBindings{HTTP: []ir.HTTPBinding{hb}} - ext, extDiags := schema.ExtensionsOf(c, src.GetExtensions(), decl) - diags = append(diags, extDiags...) - if len(ext) > 0 { - op.Unmodeled = ext - } - // After the extensions assignment, which would otherwise overwrite the map. + diags = append(diags, applyOperationExtensions(c, &op, src, decl)...) diags = append(diags, applyOperationServers(c, &op, src, decl)...) return op, extra, append(diags, checkOperationIDUnique(c, operationIDs, op, mount)...) } +// applyOperationExtensions keeps the operation's own x-* and those of the two +// objects beneath it that lower to no node of their own: its externalDocs, +// since ir.Link holds no Unmodeled map, and its Responses Object, whose +// extensions are the map's own rather than any one response's. +// +// It merges rather than assigns. The operation's map already carries whatever +// its parameters or callbacks wrote by the time this runs, and an assignment +// here would drop them — which is why the servers preservation used to have to +// run after it. +func applyOperationExtensions(c lowering.Ctx, op *ir.Operation, src *soa.Operation, decl string) []ir.Diagnostic { + ext, diags := annotation.ExtensionsAt(c.SrcIndex, + annotation.ExtensionSite{Owner: decl, Ext: src.GetExtensions()}, + annotation.ExtensionSite{Scope: "externalDocs", Owner: decl + ids.Ptr("externalDocs"), + Ext: src.GetExternalDocs().GetExtensions()}, + annotation.ExtensionSite{Scope: "responses", Owner: decl + ids.Ptr("responses"), + Ext: src.GetResponses().GetExtensions()}, + ) + op.Unmodeled = annotation.MergeUnmodeled(op.Unmodeled, ext) + return diags +} + // applyOperationServers preserves an operation's own `servers` verbatim under // Unmodeled, for the same reason applyPathServers preserves the path item's: // §10 scopes servers by index list at service and channel, and ir.Operation has @@ -376,6 +401,23 @@ func fillOperationDocs(d *ir.Docs, src *soa.Operation) { } } +// applyPathItem keeps what a path item declares that its operations have no +// home for: its servers and its own x-* extensions. Both belong to the path +// item rather than to any one operation on it, so both are written onto every +// operation the item declares. +// +// The two are applied together, through this one entry point, because every +// route that lowers a path item — a path, a webhook, a callback expression — +// must reach both, and a second call beside the first is a second chance to +// forget one on a route added later. That is exactly how the servers half came +// to be missing on two of its three routes (GitHub #39). +func applyPathItem(c lowering.Ctx, op *ir.Operation, pi *soa.PathItem, declPtr string) []ir.Diagnostic { + diags := applyPathServers(c, op, pi, declPtr) + ext, extDiags := schema.ExtensionsIn(c, pi.GetExtensions(), declPtr, "pathItem") + op.Unmodeled = annotation.MergeUnmodeled(op.Unmodeled, ext) + return append(diags, extDiags...) +} + // applyPathServers preserves path-item-level servers verbatim under Unmodeled // on the operation. §10 models servers as Document.Servers with per-scope index // lists (Service.Servers, Channel.Servers); ir.Operation just has no such list @@ -463,9 +505,27 @@ func lowerResponse(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde Headers: headers, } resp.Docs.Description = r.GetDescription() - _, linkDiags := schema.PreserveNode(c, &resp.Unmodeled, "openapi:links", + return resp, append(diags, preserveResponseExtras(c, &resp.Unmodeled, r, rptr)...) +} + +// preserveResponseExtras keeps what a Response Object declares that has no home +// on the node it lowered to: its links map, and its own x-* extensions. +// +// One helper for both branches on purpose. ir.Response and ir.ErrorCase are two +// lowerings of the same source object, and each construct kept on only one of +// them makes a declaration survive or vanish on nothing but its status code: +// links were kept on a 2xx and dropped on a 4xx, and extensions were read at +// neither (GitHub #275). Adding a construct here reaches both by construction. +// +// The links entry carries ReasonNoIRHome and no diagnostic, as it always has: +// nothing is degraded, the map is in the document, and the gap is one the IR can +// close by growing a links field. +func preserveResponseExtras(c lowering.Ctx, p *ir.Unmodeled, r *soa.Response, rptr string) []ir.Diagnostic { + _, diags := schema.PreserveNode(c, p, "openapi:links", annotation.RawChildNode(r.GetRootNode(), "links"), ir.ReasonNoIRHome, rptr+ids.Ptr("links")) - return resp, append(diags, linkDiags...) + ext, extDiags := schema.ExtensionsOf(c, r.GetExtensions(), rptr) + *p = annotation.MergeUnmodeled(*p, ext) + return append(diags, extDiags...) } // responseName builds a success response's neutral naming. OpenAPI names no @@ -497,7 +557,8 @@ func lowerErrorCase(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd } ec.Docs.Description = r.GetDescription() diags := fillErrorType(c, ts, anchors, &ec, r, rptr) - return ec, append(diags, preserveErrorHeaders(c, &ec, r, rptr)...) + diags = append(diags, preserveErrorHeaders(c, &ec, r, rptr)...) + return ec, append(diags, preserveResponseExtras(c, &ec.Unmodeled, r, rptr)...) } // preserveErrorHeaders keeps an error response's headers from being dropped: @@ -578,19 +639,27 @@ func errorContentMessage(n int) string { // parent.mount roots callback operation identity, so two parents sharing one // $ref'd callback keep distinct callback operations; parent.decl is the base a // $ref'd callback or path item resolves against (issue #107). -func lowerCallbacks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, operationIDs map[string]string, src *soa.Operation, parent opPointers, inferred string) ([]ir.Callback, []ir.Operation, []ir.Diagnostic) { +func lowerCallbacks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, operationIDs map[string]string, src *soa.Operation, parent opPointers, inferred string) ([]ir.Callback, []ir.Operation, ir.Unmodeled, []ir.Diagnostic) { cbMap := src.GetCallbacks() if cbMap == nil || cbMap.Len() == 0 { - return nil, nil, nil + return nil, nil, nil, nil } var callbacks []ir.Callback var ops []ir.Operation + var ext ir.Unmodeled var diags []ir.Diagnostic for cbName, rcb := range cbMap.All() { cb, cbDecl := resolve.ObjectAt[soa.Callback](c.RefScope(), rcb, parent.decl+ids.Ptr("callbacks", cbName)) if cb == nil { continue } + // A Callback Object's own x-* describe the callback rather than any + // expression's path item, and ir.Callback holds no Unmodeled map. The HTTP + // binding does, and is where the callbacks themselves live, so they are kept + // there under the name the callback is mapped by. + cbExt, cbExtDiags := schema.ExtensionsIn(c, cb.GetExtensions(), cbDecl, "callbacks/"+cbName) + ext = annotation.MergeUnmodeled(ext, cbExt) + diags = append(diags, cbExtDiags...) for expr, rp := range cb.All() { exprStr := string(expr) pi, piDecl := resolve.ObjectAt[soa.PathItem](c.RefScope(), rp, cbDecl+ids.Ptr(exprStr)) @@ -604,7 +673,7 @@ func lowerCallbacks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd ops = append(ops, cbOps...) } } - return callbacks, ops, diags + return callbacks, ops, ext, diags } // lowerCallbackOps lowers a callback expression's path-item operations. Callback @@ -632,7 +701,7 @@ func lowerCallbackOps(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI } op, _, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, cb.decl)...) + diags = append(diags, applyPathItem(c, &op, pi, cb.decl)...) opIDs = append(opIDs, op.ID) ops = append(ops, op) } diff --git a/compilers/openapi/internal/schema/accumulate.go b/compilers/openapi/internal/schema/accumulate.go index bf2a5fac..34f0262d 100644 --- a/compilers/openapi/internal/schema/accumulate.go +++ b/compilers/openapi/internal/schema/accumulate.go @@ -123,3 +123,10 @@ func lowerArray(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth i func ExtensionsOf(c lowering.Ctx, ext *extensions.Extensions, owner string) (ir.Unmodeled, []ir.Diagnostic) { return annotation.ExtensionsFrom(ext, c.SrcIndex, owner) } + +// ExtensionsIn is ExtensionsOf for an object with no Unmodeled map of its own, +// whose entries ride on an enclosing node's under scope — see +// annotation.ExtensionsUnder for what scope names and why it is needed. +func ExtensionsIn(c lowering.Ctx, ext *extensions.Extensions, owner, scope string) (ir.Unmodeled, []ir.Diagnostic) { + return annotation.ExtensionsUnder(ext, c.SrcIndex, owner, scope) +} diff --git a/compilers/openapi/meta.go b/compilers/openapi/meta.go index 23191d28..a282d889 100644 --- a/compilers/openapi/meta.go +++ b/compilers/openapi/meta.go @@ -1,10 +1,13 @@ package openapi import ( + "strconv" + soa "github.com/speakeasy-api/openapi/openapi" "github.com/dexpace/morphic/compilers/compile" "github.com/dexpace/morphic/compilers/openapi/internal/annotation" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" "github.com/dexpace/morphic/ir" ) @@ -35,14 +38,70 @@ type docMeta struct { } // lowerMeta lowers the document-level metadata that is not part of the type or -// service graph: info, servers, and top-level extensions (ir-design §10, §12). +// service graph: info, servers, and the extensions of every object around them +// that lowers to no node of its own (ir-design §10, §12). func lowerMeta(c lowering.Ctx) (docMeta, []ir.Diagnostic) { m := lowerInfo(c) - m.Servers = lowerServers(c) - - ext, diags := annotation.ExtensionsFrom(c.Doc.GetExtensions(), c.SrcIndex, "") + ext, diags := documentExtensions(c) m.Unmodeled = ext - return m, diags + + servers, serverDiags := lowerServers(c) + m.Servers = servers + return m, append(diags, serverDiags...) +} + +// documentExtensions collects the x-* of every object that lowers to no IR node +// of its own: the document root, the info block and the contact and license +// inside it, the root externalDocs, the components object, and each declared +// tag with its own externalDocs. ir.Document is the nearest node with an +// Unmodeled map for all of them, so each object's entries are keyed under the +// source path it was written at — see annotation.ExtensionsUnder for why one +// unscoped key for all of them would not do. +func documentExtensions(c lowering.Ctx) (ir.Unmodeled, []ir.Diagnostic) { + return annotation.ExtensionsAt(c.SrcIndex, append(rootExtensions(c), tagExtensions(c)...)...) +} + +// rootExtensions returns the extension sites a document has exactly one of. The +// root's own take no scope, since ir.Document stands for the OpenAPI Object +// itself; the rest are keyed by the path from it down to the object that wrote +// them. +func rootExtensions(c lowering.Ctx) []annotation.ExtensionSite { + info := c.Doc.GetInfo() + infoPtr := ids.Ptr("info") + return []annotation.ExtensionSite{ + {Scope: "", Owner: "", Ext: c.Doc.GetExtensions()}, + {Scope: "info", Owner: infoPtr, Ext: info.GetExtensions()}, + {Scope: "info/contact", Owner: infoPtr + ids.Ptr("contact"), Ext: info.GetContact().GetExtensions()}, + {Scope: "info/license", Owner: infoPtr + ids.Ptr("license"), Ext: info.GetLicense().GetExtensions()}, + {Scope: "externalDocs", Owner: ids.Ptr("externalDocs"), Ext: c.Doc.GetExternalDocs().GetExtensions()}, + {Scope: "components", Owner: ids.Ptr("components"), Ext: c.Doc.GetComponents().GetExtensions()}, + } +} + +// tagExtensions returns the extension sites each declared tag contributes: its +// own, and its externalDocs object's. ir.TagDef holds no Unmodeled map and +// neither does ir.Link, so both ride on the document. +// +// Keyed by the tag's index rather than its name, which is the pointer the tag +// is written at. A name would read better and is what TagDefs are found by, but +// OpenAPI's requirement that tag names be unique is the document's to keep, not +// this compiler's to rely on: two tags spelled alike would silently leave one +// entry. +func tagExtensions(c lowering.Ctx) []annotation.ExtensionSite { + tags := c.Doc.GetTags() + out := make([]annotation.ExtensionSite, 0, 2*len(tags)) + for i, t := range tags { + if t == nil { + continue + } + scope := "tags/" + strconv.Itoa(i) + ptr := ids.Ptr("tags", strconv.Itoa(i)) + out = append(out, + annotation.ExtensionSite{Scope: scope, Owner: ptr, Ext: t.GetExtensions()}, + annotation.ExtensionSite{Scope: scope + "/externalDocs", Owner: ptr + ids.Ptr("externalDocs"), + Ext: t.GetExternalDocs().GetExtensions()}) + } + return out } // lowerInfo maps info onto the document identity, docs, contact, and license. @@ -79,31 +138,38 @@ func infoDocs(c lowering.Ctx, info *soa.Info) ir.Docs { // template, description, and templated variables (ir-design §10). It returns nil // rather than an empty slice when every entry was skipped, so a document // declaring no usable server leaves the field unset. -func lowerServers(c lowering.Ctx) []ir.Server { +func lowerServers(c lowering.Ctx) ([]ir.Server, []ir.Diagnostic) { // GetServers never returns an empty slice — it injects a default "/" server // when none are declared — so the loop always runs at least once. servers := c.Doc.GetServers() out := make([]ir.Server, 0, len(servers)) - for _, s := range servers { + var diags []ir.Diagnostic + for i, s := range servers { if s == nil { continue } - out = append(out, lowerServer(s)) + one, serverDiags := lowerServer(c, s, ids.Ptr("servers", strconv.Itoa(i))) + diags = append(diags, serverDiags...) + out = append(out, one) } if len(out) == 0 { - return nil + return nil, diags } - return out + return out, diags } -// lowerServer lowers one server, named by serverName. -func lowerServer(s *soa.Server) ir.Server { +// lowerServer lowers one server, named by serverName, keeping its x-* on the +// ir.Server itself; sptr is the server's own pointer in the servers list. +func lowerServer(c lowering.Ctx, s *soa.Server, sptr string) (ir.Server, []ir.Diagnostic) { + vars, diags := serverVariables(c, s, sptr) + ext, extDiags := annotation.ExtensionsFrom(s.GetExtensions(), c.SrcIndex, sptr) return ir.Server{ Name: serverName(s), URLTemplate: s.GetURL(), Description: ir.Docs{Description: s.GetDescription()}, - Variables: serverVariables(s), - } + Variables: vars, + Unmodeled: ext, + }, append(diags, extDiags...) } // serverName builds a server's neutral naming: the declared name when the source @@ -134,22 +200,29 @@ func serverName(s *soa.Server) ir.Naming { // serverVariables lowers a server's URL template variables in source order, or // nil when it declares none. -func serverVariables(s *soa.Server) []ir.ServerVariable { +func serverVariables(c lowering.Ctx, s *soa.Server, sptr string) ([]ir.ServerVariable, []ir.Diagnostic) { vars := s.GetVariables() if vars == nil || vars.Len() == 0 { - return nil + return nil, nil } out := make([]ir.ServerVariable, 0, vars.Len()) + var diags []ir.Diagnostic for name, v := range vars.All() { if v == nil { continue } + // ServerVariable exposes no GetExtensions at this library version, so the + // field is read directly — as XMLHints already reads its own. + ext, extDiags := annotation.ExtensionsFrom(v.Extensions, c.SrcIndex, + sptr+ids.Ptr("variables", name)) + diags = append(diags, extDiags...) out = append(out, ir.ServerVariable{ - Name: name, - Default: v.GetDefault(), - Enum: v.GetEnum(), - Docs: ir.Docs{Description: v.GetDescription()}, + Name: name, + Default: v.GetDefault(), + Enum: v.GetEnum(), + Docs: ir.Docs{Description: v.GetDescription()}, + Unmodeled: ext, }) } - return out + return out, diags } diff --git a/compilers/openapi/meta_test.go b/compilers/openapi/meta_test.go index 528f6243..b37c2d84 100644 --- a/compilers/openapi/meta_test.go +++ b/compilers/openapi/meta_test.go @@ -36,9 +36,9 @@ paths: // TestMeta_UnserializableExtensionStillWarns pins the document-level twin of // TestAuth_UnserializableExtensionStillWarns: when every top-level x-* extension -// fails to serialize, lowerMeta's "if len(ext) > 0" guard is false, so the -// warning must already have been recorded by the extension reader rather than appended -// inside the guard — the shape that used to drop it silently. +// fails to serialize, lowerMeta ends up with an empty Unmodeled map, so the +// warning must come from the extension reader rather than from a branch guarded +// on what was kept — the shape that used to drop it silently. func TestMeta_UnserializableExtensionStillWarns(t *testing.T) { t.Parallel() spec := `openapi: 3.1.0 @@ -70,6 +70,20 @@ func TestMeta_FullDocumentMetadata(t *testing.T) { assert.Equal(t, []string{"us", "eu"}, doc.Servers[0].Variables[0].Enum) } +// TestTagExtensions_NilEntrySkipped pins the same guard lowerTagDefs keeps on +// the other side of the tag list: a nil entry contributes no extension site, so +// nothing dereferences it and no site is keyed at an index holding no tag. +func TestTagExtensions_NilEntrySkipped(t *testing.T) { + t.Parallel() + doc := &soa.OpenAPI{Tags: []*soa.Tag{nil, {Name: "kept"}}} + + got := tagExtensions(lowering.Ctx{Doc: doc}) + + require.Len(t, got, 2, "one surviving tag contributes its own site and its externalDocs one") + assert.Equal(t, "tags/1", got[0].Scope, "the site is keyed at the tag's own index, not its position") + assert.Equal(t, "/tags/1", got[0].Owner) +} + func TestMeta_NoInfoNoServers(t *testing.T) { t.Parallel() // With no info block the title is empty; with no servers the library injects @@ -103,7 +117,7 @@ func TestServerName_DerivedFromURLWhenUnnamed(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tc.want, lowerServer(&soa.Server{URL: tc.url}).Name) + assert.Equal(t, tc.want, serverName(&soa.Server{URL: tc.url})) }) } } @@ -114,9 +128,9 @@ func TestServerName_DerivedFromURLWhenUnnamed(t *testing.T) { // both servers the same. func TestServerName_DistinguishesServersDifferingOnlyInPath(t *testing.T) { t.Parallel() - v1 := lowerServer(&soa.Server{URL: "https://api.example.com/v1"}) - v2 := lowerServer(&soa.Server{URL: "https://api.example.com/v2"}) - assert.NotEqual(t, v1.Name.Hint, v2.Name.Hint, "two distinct servers get two distinct hints") + v1 := serverName(&soa.Server{URL: "https://api.example.com/v1"}) + v2 := serverName(&soa.Server{URL: "https://api.example.com/v2"}) + assert.NotEqual(t, v1.Hint, v2.Hint, "two distinct servers get two distinct hints") } // TestServerName_CollidesOnPunctuationAlone bounds that claim, which must not be @@ -126,17 +140,18 @@ func TestServerName_DistinguishesServersDifferingOnlyInPath(t *testing.T) { // known bound rather than a later discovery. func TestServerName_CollidesOnPunctuationAlone(t *testing.T) { t.Parallel() - dotted := lowerServer(&soa.Server{URL: "https://api.example.com/v1"}) - dashed := lowerServer(&soa.Server{URL: "https://api.example.com/v-1"}) - assert.Equal(t, dotted.Name.Hint, dashed.Name.Hint, + dotted := serverName(&soa.Server{URL: "https://api.example.com/v1"}) + dashed := serverName(&soa.Server{URL: "https://api.example.com/v-1"}) + assert.Equal(t, dotted.Hint, dashed.Hint, "neutral words carry no punctuation, so these two collide") } func TestLowerServers_NilEntrySkipped(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Servers: []*soa.Server{nil, {URL: "https://x.example.com"}}} - got := lowerServers(lowering.Ctx{Doc: doc}) + got, diags := lowerServers(lowering.Ctx{Doc: doc}) + assert.Empty(t, diags) require.Len(t, got, 1, "nil server entry skipped, valid one lowered") assert.Equal(t, "https://x.example.com", got[0].URLTemplate) } @@ -147,7 +162,8 @@ func TestServerVariables_NilEntrySkipped(t *testing.T) { sequencedmap.NewElem("skip", (*soa.ServerVariable)(nil)), sequencedmap.NewElem("keep", &soa.ServerVariable{}), ) - srv := lowerServer(&soa.Server{URL: "https://x", Variables: vars}) + srv, diags := lowerServer(lowering.Ctx{}, &soa.Server{URL: "https://x", Variables: vars}, "/servers/0") + assert.Empty(t, diags) require.Len(t, srv.Variables, 1, "nil variable entry skipped") assert.Equal(t, "keep", srv.Variables[0].Name) } @@ -159,5 +175,7 @@ func TestLowerServers_EveryEntrySkippedIsNil(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Servers: []*soa.Server{nil, nil}} - assert.Nil(t, lowerServers(lowering.Ctx{Doc: doc})) + got, diags := lowerServers(lowering.Ctx{Doc: doc}) + assert.Nil(t, got) + assert.Empty(t, diags) } diff --git a/testdata/conformance/openapi/extensions-x.golden.json b/testdata/conformance/openapi/extensions-x.golden.json index ce499e59..c67df892 100644 --- a/testdata/conformance/openapi/extensions-x.golden.json +++ b/testdata/conformance/openapi/extensions-x.golden.json @@ -2,7 +2,19 @@ "irVersion": "0.3.0", "name": "ExtensionsX", "version": "1.0.0", - "docs": {}, + "docs": { + "externalDocs": [ + { + "url": "https://d.example" + } + ] + }, + "contact": { + "name": "n" + }, + "license": { + "name": "MIT" + }, "services": [ { "id": "s/openapi/0", @@ -14,9 +26,16 @@ "groups": [ { "name": { - "hint": "default" + "source": "t1", + "canonical": "t_1" + }, + "docs": { + "externalDocs": [ + { + "url": "https://t.example" + } + ] }, - "docs": {}, "operations": [ { "id": "op/openapi/paths/~1widgets/get", @@ -24,7 +43,37 @@ "source": "listWidgets", "canonical": "list_widgets" }, - "docs": {}, + "docs": { + "externalDocs": [ + { + "url": "https://o.example" + } + ] + }, + "params": [ + { + "name": { + "source": "shape", + "canonical": "shape" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "docs": {}, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XPARAMETER", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/parameters/0/x-mark" + } + } + } + } + ], "responses": [ { "name": { @@ -38,26 +87,184 @@ } ] }, + "headers": [ + { + "id": "p/openapi/paths/~1widgets/get/responses/200/headers/X-Rate", + "name": { + "source": "X-Rate", + "canonical": "x_rate" + }, + "wireName": "X-Rate", + "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": {}, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XHEADER", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/200/headers/X-Rate/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/200/headers/X-Rate" + } + } + ], "docs": { "description": "ok" + }, + "unmodeled": { + "openapi:links": { + "reason": "no_ir_home", + "value": { + "self": { + "operationId": "listWidgets" + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/200/links" + } + }, + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XRESPONSE", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/200/x-mark" + } + } + } + } + ], + "errors": [ + { + "type": { + "target": "", + "nullable": false + }, + "conditions": { + "statusCodes": [ + { + "from": 404, + "to": 404 + } + ] + }, + "fault": "client", + "docs": { + "description": "gone" + }, + "unmodeled": { + "openapi:links": { + "reason": "no_ir_home", + "value": { + "retry": { + "operationId": "listWidgets" + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/404/links" + } + }, + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XERRORRESPONSE", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/404/x-mark" + } + } } } ], "oneWay": false, "idempotency": {}, "auth": null, + "tags": [ + "t1" + ], "bindings": { "http": [ { "method": "GET", "uriTemplate": "/widgets", "sharedRoute": false, + "paramBindings": [ + { + "param": "shape", + "location": "query", + "wireName": "shape", + "style": "form", + "explode": true, + "allowReserved": false + } + ], "checksumRequired": false, - "isWebhook": false + "isWebhook": false, + "callbacks": [ + { + "expression": "{$request.body#/u}", + "operations": [ + "op/openapi/paths/~1widgets/get/callbacks/onEvent/{$request.body#~1u}/post" + ] + } + ], + "unmodeled": { + "openapi:callbacks/onEvent/x-mark": { + "reason": "vendor_extension", + "value": "XCALLBACK", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/callbacks/onEvent/x-mark" + } + } + } } ] }, "unmodeled": { + "openapi:externalDocs/x-mark": { + "reason": "vendor_extension", + "value": "XOPERATIONEXTERNALDOCS", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/externalDocs/x-mark" + } + }, + "openapi:pathItem/x-mark": { + "reason": "vendor_extension", + "value": "XPATHITEM", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/x-mark" + } + }, + "openapi:responses/x-mark": { + "reason": "vendor_extension", + "value": "XRESPONSES", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/x-mark" + } + }, "openapi:x-internal": { "reason": "vendor_extension", "value": true, @@ -71,17 +278,384 @@ "source": 0, "pointer": "/paths/~1widgets/get" } + }, + { + "id": "op/openapi/paths/~1widgets/get/callbacks/onEvent/{$request.body#~1u}/post", + "name": { + "source": "onEventPost", + "canonical": "on_event_post" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "POST", + "uriTemplate": "{$request.body#/u}", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/callbacks/onEvent/{$request.body#~1u}/post" + } + } + ] + }, + { + "name": { + "hint": "default" + }, + "docs": {}, + "operations": [ + { + "id": "op/openapi/paths/~1widgets/post", + "name": { + "source": "createWidget", + "canonical": "create_widget" + }, + "docs": {}, + "params": [ + { + "name": { + "source": "shape", + "canonical": "shape" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "docs": {}, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XPARAMETER", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/parameters/0/x-mark" + } + } + } + } + ], + "request": { + "contents": [ + { + "mediaType": "multipart/form-data", + "type": { + "target": "t/anon/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema", + "nullable": false + }, + "encoding": { + "p/openapi/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema/properties/f": { + "contentTypes": [ + "text/plain" + ], + "multi": false, + "filename": false + } + }, + "examples": [ + { + "name": "e1", + "value": { + "kind": "string", + "str": "v", + "bytes": null, + "list": null, + "object": null + }, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XEXAMPLE", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/examples/e1/x-mark" + } + } + } + } + ], + "unmodeled": { + "openapi:encoding/f/allowReserved": { + "reason": "no_ir_home", + "value": true, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/encoding/f/allowReserved" + } + }, + "openapi:encoding/f/x-mark": { + "reason": "vendor_extension", + "value": "XENCODING", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/encoding/f/x-mark" + } + }, + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XMEDIATYPE", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/x-mark" + } + } + } + } + ], + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XREQUESTBODY", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/x-mark" + } + } + } + }, + "responses": [ + { + "name": { + "hint": "201" + }, + "conditions": { + "statusCodes": [ + { + "from": 201, + "to": 201 + } + ] + }, + "docs": { + "description": "created" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "POST", + "uriTemplate": "/widgets", + "sharedRoute": false, + "paramBindings": [ + { + "param": "shape", + "location": "query", + "wireName": "shape", + "style": "form", + "explode": true, + "allowReserved": false + } + ], + "requestContentTypes": [ + "multipart/form-data" + ], + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "unmodeled": { + "openapi:pathItem/x-mark": { + "reason": "vendor_extension", + "value": "XPATHITEM", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post" + } } ] } ], "auth": null, + "unmodeled": { + "openapi:paths/x-mark": { + "reason": "vendor_extension", + "value": "XPATHS", + "provenance": { + "source": 0, + "pointer": "/paths/x-mark" + } + } + }, "provenance": { "source": 0 } } ], "types": { + "t/anon/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema": { + "kind": "model", + "id": "t/anon/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema", + "name": { + "hint": "createWidget_request" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema" + }, + "properties": [ + { + "id": "p/openapi/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema/properties/f", + "name": { + "source": "f", + "canonical": "f" + }, + "wireName": "f", + "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": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/schema/properties/f" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/A": { + "kind": "model", + "id": "t/openapi/components/schemas/A", + "name": { + "source": "A", + "canonical": "a" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/A" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/A/properties/k", + "name": { + "source": "k", + "canonical": "k" + }, + "wireName": "k", + "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": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/A/properties/k" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/D": { + "kind": "union", + "id": "t/openapi/components/schemas/D", + "name": { + "source": "D", + "canonical": "d" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:discriminator/x-mark": { + "reason": "vendor_extension", + "value": "XDISCRIMINATOR", + "provenance": { + "source": 0, + "pointer": "/components/schemas/D/discriminator/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/D" + }, + "variants": [ + { + "name": { + "hint": "A" + }, + "type": { + "target": "t/openapi/components/schemas/A", + "nullable": false + }, + "docs": {} + } + ], + "exclusive": true, + "wireTagged": false, + "discriminator": { + "propertyName": "k", + "inferred": false + } + }, "t/openapi/components/schemas/S": { "kind": "model", "id": "t/openapi/components/schemas/S", @@ -90,9 +664,27 @@ "canonical": "s" }, "anonymous": false, - "docs": {}, + "docs": { + "externalDocs": [ + { + "url": "https://s.example" + } + ] + }, "sensitive": false, + "xml": { + "name": "s", + "wrapped": false + }, "unmodeled": { + "openapi:externalDocs/x-mark": { + "reason": "vendor_extension", + "value": "XSCHEMAEXTERNALDOCS", + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/externalDocs/x-mark" + } + }, "openapi:x-blob": { "reason": "vendor_extension", "value": "aGVsbG8=", @@ -124,6 +716,14 @@ "source": 0, "pointer": "/components/schemas/S/x-wrapped" } + }, + "openapi:xml/x-mark": { + "reason": "vendor_extension", + "value": "XXML", + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/xml/x-mark" + } } }, "provenance": { @@ -176,17 +776,181 @@ "prim": "string" } }, + "auth": { + "auth/openapi/components/securitySchemes/k": { + "id": "auth/openapi/components/securitySchemes/k", + "name": { + "source": "k", + "canonical": "k" + }, + "kind": "apiKey", + "docs": {}, + "in": "header", + "keyName": "K", + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XSECURITYSCHEME", + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/k/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/k" + } + }, + "auth/openapi/components/securitySchemes/o": { + "id": "auth/openapi/components/securitySchemes/o", + "name": { + "source": "o", + "canonical": "o" + }, + "kind": "oauth2", + "docs": {}, + "flows": [ + { + "kind": "implicit", + "authorizationURL": "https://a.example", + "scopes": { + "read": "r" + }, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XOAUTHFLOW", + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/o/flows/implicit/x-mark" + } + } + } + } + ], + "unmodeled": { + "openapi:flows/x-mark": { + "reason": "vendor_extension", + "value": "XOAUTHFLOWS", + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/o/flows/x-mark" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/securitySchemes/o" + } + } + }, "servers": [ { "name": { - "hint": "server" + "hint": "https_e_example_v" }, - "urlTemplate": "/", + "urlTemplate": "https://e.example/{v}", "description": {}, - "auth": null + "variables": [ + { + "name": "v", + "default": "1", + "docs": {}, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XSERVERVARIABLE", + "provenance": { + "source": 0, + "pointer": "/servers/0/variables/v/x-mark" + } + } + } + } + ], + "auth": null, + "unmodeled": { + "openapi:x-mark": { + "reason": "vendor_extension", + "value": "XSERVER", + "provenance": { + "source": 0, + "pointer": "/servers/0/x-mark" + } + } + } + } + ], + "tagDefs": [ + { + "name": "t1", + "docs": { + "externalDocs": [ + { + "url": "https://t.example" + } + ] + } } ], "unmodeled": { + "openapi:components/x-mark": { + "reason": "vendor_extension", + "value": "XCOMPONENTS", + "provenance": { + "source": 0, + "pointer": "/components/x-mark" + } + }, + "openapi:externalDocs/x-mark": { + "reason": "vendor_extension", + "value": "XEXTERNALDOCS", + "provenance": { + "source": 0, + "pointer": "/externalDocs/x-mark" + } + }, + "openapi:info/contact/x-mark": { + "reason": "vendor_extension", + "value": "XCONTACT", + "provenance": { + "source": 0, + "pointer": "/info/contact/x-mark" + } + }, + "openapi:info/license/x-mark": { + "reason": "vendor_extension", + "value": "XLICENSE", + "provenance": { + "source": 0, + "pointer": "/info/license/x-mark" + } + }, + "openapi:info/x-mark": { + "reason": "vendor_extension", + "value": "XINFO", + "provenance": { + "source": 0, + "pointer": "/info/x-mark" + } + }, + "openapi:tags/0/externalDocs/x-mark": { + "reason": "vendor_extension", + "value": "XTAGEXTERNALDOCS", + "provenance": { + "source": 0, + "pointer": "/tags/0/externalDocs/x-mark" + } + }, + "openapi:tags/0/x-mark": { + "reason": "vendor_extension", + "value": "XTAG", + "provenance": { + "source": 0, + "pointer": "/tags/0/x-mark" + } + }, "openapi:x-audience": { "reason": "vendor_extension", "value": "public", @@ -196,11 +960,22 @@ } } }, + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "encoding allowReserved has no ir.PartEncoding home; kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/post/requestBody/content/multipart~1form-data/encoding/f/allowReserved" + } + } + ], "sources": [ { "format": "openapi@3.1", "path": "extensions-x.yaml", - "hash": "bdd4e78c821cd4ff1e418d0036d0f44c67120aa29ecd201a8d9a2cbc5a825ae9" + "hash": "50a3f85f94845ca391a312360b2c49c55938271c370d5ccc682ded1ab6f35f17" } ] } diff --git a/testdata/conformance/openapi/extensions-x.yaml b/testdata/conformance/openapi/extensions-x.yaml index dfbcee4a..7304f57b 100644 --- a/testdata/conformance/openapi/extensions-x.yaml +++ b/testdata/conformance/openapi/extensions-x.yaml @@ -1,15 +1,65 @@ openapi: 3.1.0 -info: {title: ExtensionsX, version: "1.0.0"} x-audience: public +info: + title: ExtensionsX + version: "1.0.0" + x-mark: XINFO + contact: {name: n, x-mark: XCONTACT} + license: {name: MIT, x-mark: XLICENSE} +externalDocs: {url: https://d.example, x-mark: XEXTERNALDOCS} +servers: + - url: https://e.example/{v} + x-mark: XSERVER + variables: {v: {default: "1", x-mark: XSERVERVARIABLE}} +tags: + - name: t1 + x-mark: XTAG + externalDocs: {url: https://t.example, x-mark: XTAGEXTERNALDOCS} paths: + x-mark: XPATHS /widgets: + x-mark: XPATHITEM + parameters: [{name: shape, in: query, schema: {type: string}, x-mark: XPARAMETER}] get: operationId: listWidgets + tags: [t1] x-internal: true + externalDocs: {url: https://o.example, x-mark: XOPERATIONEXTERNALDOCS} responses: + x-mark: XRESPONSES "200": description: ok + x-mark: XRESPONSE + headers: {X-Rate: {schema: {type: string}, x-mark: XHEADER}} + links: {self: {operationId: listWidgets}} + "404": + description: gone + x-mark: XERRORRESPONSE + links: {retry: {operationId: listWidgets}} + callbacks: + onEvent: + x-mark: XCALLBACK + "{$request.body#/u}": + post: + operationId: onEventPost + responses: {"200": {description: ok}} + post: + operationId: createWidget + requestBody: + x-mark: XREQUESTBODY + required: true + content: + multipart/form-data: + x-mark: XMEDIATYPE + schema: {type: object, properties: {f: {type: string}}} + encoding: + f: {contentType: text/plain, allowReserved: true, x-mark: XENCODING} + examples: + e1: {value: v, x-mark: XEXAMPLE} + responses: + "201": {description: created} components: + x-mark: XCOMPONENTS schemas: S: type: object @@ -24,5 +74,23 @@ components: x-wrapped: !!binary | aGVs bG8= + xml: {name: s, x-mark: XXML} + externalDocs: {url: https://s.example, x-mark: XSCHEMAEXTERNALDOCS} properties: a: {type: string} + D: + oneOf: [{$ref: '#/components/schemas/A'}] + discriminator: {propertyName: k, x-mark: XDISCRIMINATOR} + A: + type: object + properties: {k: {type: string}} + securitySchemes: + k: {type: apiKey, name: K, in: header, x-mark: XSECURITYSCHEME} + o: + type: oauth2 + flows: + x-mark: XOAUTHFLOWS + implicit: + authorizationUrl: https://a.example + scopes: {read: r} + x-mark: XOAUTHFLOW diff --git a/testdata/conformance/openapi/multipart-encoding.golden.json b/testdata/conformance/openapi/multipart-encoding.golden.json index fde8c823..c424e100 100644 --- a/testdata/conformance/openapi/multipart-encoding.golden.json +++ b/testdata/conformance/openapi/multipart-encoding.golden.json @@ -269,6 +269,16 @@ "style": "form", "explode": true } + }, + "unmodeled": { + "openapi:encoding/ids/allowReserved": { + "reason": "no_ir_home", + "value": true, + "provenance": { + "source": 0, + "pointer": "/paths/~1submit/post/requestBody/content/application~1x-www-form-urlencoded/encoding/ids/allowReserved" + } + } } } ], @@ -721,6 +731,15 @@ "pointer": "/paths/~1upload-composed/post/requestBody" } }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "encoding allowReserved has no ir.PartEncoding home; kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1submit/post/requestBody/content/application~1x-www-form-urlencoded/encoding/ids/allowReserved" + } + }, { "severity": "info", "code": "openapi/degraded-construct", @@ -735,7 +754,7 @@ { "format": "openapi@3.1", "path": "multipart-encoding.yaml", - "hash": "3fc8c047afb3b7a847d3875de98e50a9b09052a495631f6f884ae197e2a17423" + "hash": "b15abb2581ba82770d20ef9459dd63c4871f3ab3e2f7a84f83dc146f26314e1e" } ] } diff --git a/testdata/conformance/openapi/multipart-encoding.yaml b/testdata/conformance/openapi/multipart-encoding.yaml index 3f5508f3..fabfa657 100644 --- a/testdata/conformance/openapi/multipart-encoding.yaml +++ b/testdata/conformance/openapi/multipart-encoding.yaml @@ -62,7 +62,10 @@ paths: type: array items: {type: string} encoding: - ids: {style: form, explode: true} + # allowReserved sits beside style and explode in the source and has + # no ir.PartEncoding field, where those two do; it is kept verbatim + # on the content instead of vanishing (GitHub #291). + ids: {style: form, explode: true, allowReserved: true} responses: "200": description: ok diff --git a/testdata/conformance/openapi/response-links.golden.json b/testdata/conformance/openapi/response-links.golden.json index 5478dd7a..57bcfa4e 100644 --- a/testdata/conformance/openapi/response-links.golden.json +++ b/testdata/conformance/openapi/response-links.golden.json @@ -60,6 +60,43 @@ } } ], + "errors": [ + { + "type": { + "target": "", + "nullable": false + }, + "conditions": { + "statusCodes": [ + { + "from": 409, + "to": 409 + } + ] + }, + "fault": "client", + "docs": { + "description": "conflict" + }, + "unmodeled": { + "openapi:links": { + "reason": "no_ir_home", + "value": { + "GetConflicting": { + "operationId": "getOrder", + "parameters": { + "orderId": "$response.body#/existingId" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1orders/post/responses/409/links" + } + } + } + } + ], "oneWay": false, "idempotency": {}, "auth": null, @@ -184,7 +221,7 @@ { "format": "openapi@3.1", "path": "response-links.yaml", - "hash": "76f06e7ecfec2ebe4bbd3c5ca6e02f781042b76afc35be07e1d4be8a53b0f85c" + "hash": "15ba7e8058c2731f060d6d93c0a9ec22aa2731add81e5a1a19372d16f72b169f" } ] } diff --git a/testdata/conformance/openapi/response-links.yaml b/testdata/conformance/openapi/response-links.yaml index fc6f9a90..eb9425e1 100644 --- a/testdata/conformance/openapi/response-links.yaml +++ b/testdata/conformance/openapi/response-links.yaml @@ -5,6 +5,16 @@ paths: post: operationId: createOrder responses: + # The error case first, so the fixture is declared in the order that used + # to be wrong: links were kept on a success response and dropped on an + # error one, and a fixture whose 4xx trails a 2xx that already works + # cannot show the difference. + "409": + description: conflict + links: + GetConflicting: + operationId: getOrder + parameters: {orderId: '$response.body#/existingId'} "201": description: created links: diff --git a/testdata/conformance/openapi/unwitnessed.golden.txt b/testdata/conformance/openapi/unwitnessed.golden.txt index 0ef3569a..21893c70 100644 --- a/testdata/conformance/openapi/unwitnessed.golden.txt +++ b/testdata/conformance/openapi/unwitnessed.golden.txt @@ -51,7 +51,6 @@ Example.Error Example.Headers Example.Input Example.Output -Example.Unmodeled External.Identity External.MinVersion External.Package @@ -71,7 +70,6 @@ HTTPBinding.PatchImplicitOptionality HTTPBinding.ResponseBodyPath HTTPBinding.SharedRoute HTTPBinding.SuccessStatus -HTTPBinding.Unmodeled HTTPParamBinding.BodyPath HTTPParamBinding.ParamPath HTTPParamBinding.PathPattern @@ -113,7 +111,6 @@ Model.Implements Model.InputOnly Model.Positional Naming.Aliases -OAuthFlow.Unmodeled OTPBinding.Behaviour OTPBinding.Kind OTPBinding.Process @@ -195,8 +192,6 @@ Server.Bindings Server.Protocol Server.ProtocolVersion Server.Tags -Server.Unmodeled -ServerVariable.Unmodeled Service.CommonErrors Service.Extends Service.Namespace @@ -204,7 +199,6 @@ Service.Protocols Service.Provenance Service.Renames Service.Servers -Service.Unmodeled Service.Version StreamDetail.Events StreamDetail.Initial From 3a0bc4e584428333070c6424bfb7b46f20b984c9 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 05:16:14 +0300 Subject: [PATCH 2/3] fix(compilers/openapi): keep keys the source model does not name --- .../openapi/internal/annotation/unknown.go | 207 +++++++++++++++++ .../annotation/unknown_internal_test.go | 212 ++++++++++++++++++ compilers/openapi/internal/auth/auth.go | 8 +- compilers/openapi/internal/diag/diag.go | 34 +++ compilers/openapi/internal/diag/diag_test.go | 1 + .../openapi/internal/operation/content.go | 10 +- .../openapi/internal/operation/operations.go | 19 +- .../openapi/internal/operation/params.go | 6 +- .../openapi/internal/schema/accumulate.go | 14 ++ compilers/openapi/internal/schema/schema.go | 6 +- compilers/openapi/meta.go | 108 +++++++-- compilers/openapi/meta_test.go | 24 +- compilers/openapi/unknownkeys_test.go | 200 +++++++++++++++++ .../allof-oneof-cooccurrence.golden.json | 6 +- .../openapi/allof-oneof-cooccurrence.yaml | 2 +- .../openapi/unwitnessed.golden.txt | 2 - testdata/openapi/unknown_keys.yaml | 59 +++++ 17 files changed, 878 insertions(+), 40 deletions(-) create mode 100644 compilers/openapi/internal/annotation/unknown.go create mode 100644 compilers/openapi/internal/annotation/unknown_internal_test.go create mode 100644 compilers/openapi/unknownkeys_test.go create mode 100644 testdata/openapi/unknown_keys.yaml diff --git a/compilers/openapi/internal/annotation/unknown.go b/compilers/openapi/internal/annotation/unknown.go new file mode 100644 index 00000000..bab5accd --- /dev/null +++ b/compilers/openapi/internal/annotation/unknown.go @@ -0,0 +1,207 @@ +package annotation + +import ( + "reflect" + "slices" + + oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" + "github.com/dexpace/morphic/ir" +) + +// MaxUnknownKeys bounds how many keys one object contributes to the IR. +// +// The key set is the document's to choose the size of, and every collection in +// this compiler is bounded, so this one is too. It sits far above what a +// document writes by accident, so an object reaching it is generated or hostile +// rather than merely sloppy, and what it discards is announced under +// diag.UnknownKeyBudget rather than dropped in silence. +const MaxUnknownKeys = 64 + +// DecidedKeywords are the JSON Schema keywords the library's schema model names +// no field for and this compiler has already decided about, so the census must +// not claim them as unread. Each decision is recorded where it was made, and the +// schema walk's 2020-12 vocabulary test fails if one starts being carried: +// +// - $comment — 2020-12 §8.3 forbids presenting it to end users, so no SDK +// emitter may see it. Dropped on purpose. +// - $dynamicAnchor — read by the anchor index as a reference target, which is +// what lets a $dynamicRef expand; declaring one says nothing about the shape. +// - $dynamicRef — carried by the dynamic-reference lowering, which either +// expands it into the position's type or keeps it under a reason of its own. +// An entry beside an expanded one would tell a consumer the compiler ignored +// a reference it had in fact resolved. +// +// The other 2020-12 keywords with no field of their own — $vocabulary and +// dependentRequired — need no entry here. Their readers write to the same map, +// so the census finds them already recorded and leaves them alone. +var DecidedKeywords = []string{"$comment", "$dynamicAnchor", "$dynamicRef"} + +// UnknownKeywordsIn records on p the keywords s writes that no field of the JSON +// Schema model names, and announces each. +// +// OpenAPI 3.1 schemas are JSON Schema 2020-12, where an unrecognized keyword is +// legal input: the specification requires an implementation to ignore what it +// does not recognize and allows such a keyword to carry meaning for other +// tooling. So this reports a decision rather than a fault, and is graded +// accordingly — see diag.UnknownSchemaKeyword. +// +// It keeps only what no other reader kept, which is why it runs after all of +// them: `$vocabulary` and `dependentRequired` have no field in the model either +// and are read straight off the raw node by readers with more to say about them, +// so the census finds those already recorded and leaves them alone. A keyword no +// reader leaves a trace of needs naming in DecidedKeywords instead. +func UnknownKeywordsIn(p *ir.Unmodeled, s *oas3.Schema, pointer string, srcIndex int) []ir.Diagnostic { + return census(p, s, srcIndex, pointer, "", keyClass{ + code: diag.UnknownSchemaKeyword, + severity: ir.SeverityInfo, + skip: DecidedKeywords, + message: "keyword %q has no field in the schema model this compiler lowers and no IR " + + "position of its own; kept verbatim under Unmodeled", + }) +} + +// UnknownKeysIn records on p the keys an OpenAPI object writes that the +// specification neither defines nor admits as an extension, for an object +// lowering to a node with an Unmodeled map of its own. owner is the object's own +// source pointer. +// +// Unlike its schema neighbour this reports a fault: OpenAPI gives each of its +// objects a closed key set and requires every extension to be prefixed x-, so a +// key that is neither is nothing the document is permitted to write — in +// practice a misspelling of the field beside it. It is kept all the same, +// because invariant 2 does not bend for invalid input, and a misspelt key is the +// one a reader most needs to find. +func UnknownKeysIn(p *ir.Unmodeled, model any, srcIndex int, owner string) []ir.Diagnostic { + return UnknownKeysUnder(p, model, srcIndex, owner, "") +} + +// UnknownKeysUnder is UnknownKeysIn with every entry keyed beneath scope, for +// the objects with no Unmodeled map of their own, whose keys ride on the nearest +// node that has one — an info object's on the document, a tag's on the document. +// +// scope says which object wrote them: the source path from the carrier down to +// the object. Several objects reach one map, where "openapi:status" from two of +// them would be a single key and the entry that survived would depend on which +// lowering ran last. +func UnknownKeysUnder(p *ir.Unmodeled, model any, srcIndex int, owner, scope string) []ir.Diagnostic { + return census(p, model, srcIndex, owner, scope, keyClass{ + code: diag.UnknownObjectKey, + severity: ir.SeverityWarning, + message: "key %q is not defined by the OpenAPI object it is written on and is not an " + + "x- extension; kept verbatim under Unmodeled", + }) +} + +// keyClass is how a key the model does not name is graded: which diagnostic +// announces it, and at what severity. +// +// The reason is not part of it. Both classes carry ReasonOutOfScope, because +// that is a property of the construct rather than of the document: no IR node is +// coming for a key the format does not define, nor for one a schema dialect +// defines and this compiler does not model, so an emitter policy layer is the +// only consumer either has. Which of the two a key is says something about the +// source, and the diagnostic channel is where this compiler says that. +type keyClass struct { + code string + severity ir.Severity + skip []string // keywords already decided about; see DecidedKeywords + message string // one %q, filled with the key +} + +// census records on p every key model's source object wrote that its own model +// names no field for, each under its own key beneath scope. +// +// A key p already holds is left alone and not announced: the census is the +// complement of everything the compiler read, not only of what the model names, +// and a reader with a reason of its own for a keyword has already said it +// better. +func census(p *ir.Unmodeled, model any, srcIndex int, owner, scope string, cl keyClass) []ir.Diagnostic { + keys, root := undeclaredKeys(model) + if len(keys) == 0 { + return nil + } + var diags []ir.Diagnostic + if len(keys) > MaxUnknownKeys { + diags = append(diags, budgetDiag(len(keys), owner, srcIndex)) + keys = keys[:MaxUnknownKeys] + } + for _, key := range keys { + entry := "openapi:" + scoped(scope, key) + if _, recorded := (*p)[entry]; recorded || slices.Contains(cl.skip, key) { + continue + } + at := owner + ids.Ptr(key) + kept, keptDiags := PreserveNodeInto(p, entry, RawChildNode(root, key), + ir.ReasonOutOfScope, at, srcIndex) + diags = append(diags, keptDiags...) + if !kept { + continue + } + diags = append(diags, diag.Newf(cl.severity, cl.code, + ir.Provenance{Source: srcIndex, Pointer: at}, cl.message, key)) + } + return diags +} + +// scoped spells one entry's key on the carrier holding it. +func scoped(scope, key string) string { + if scope == "" { + return key + } + return scope + "/" + key +} + +// budgetDiag reports the keys past MaxUnknownKeys, which reach the IR in no form. +func budgetDiag(total int, owner string, srcIndex int) ir.Diagnostic { + return diag.Newf(ir.SeverityWarning, diag.UnknownKeyBudget, + ir.Provenance{Source: srcIndex, Pointer: owner}, + "object writes %d keys its model names no field for, past the %d this compiler keeps; "+ + "the rest are represented in the IR in no form at all", total, MaxUnknownKeys) +} + +// parsedObject is the part of a parsed model the census reads: its core, which +// holds the census the unmarshaller took, and the mapping node the keys were +// written on. +// +// Declared here rather than taken from the library, so this package depends on +// the shape it uses rather than on the marshaller package, and so a test can +// drive the branches below with a model of its own. +type parsedObject interface { + GetCoreAny() any + GetRootNode() *yaml.Node +} + +// unknownReporter is a core model's own record of the keys it did not name. +type unknownReporter interface{ GetUnknownProperties() []string } + +// undeclaredKeys returns, sorted, the keys model's source object wrote that its +// model names no field for, and the mapping node they were written on. +// +// Sorted, and on a copy: the library fills that list from a parallel walk of the +// mapping under a mutex, so its order is neither source order nor stable, and +// the slice it hands back is the model's own. An unsorted read would order this +// compiler's diagnostics by something the source does not decide, which +// invariant 7 forbids. +// +// A model reporting no census yields nothing rather than panicking. The receiver +// may be a typed nil — an absent object is what the getters return for one the +// document omitted — and a promoted method on one of those dereferences it. +func undeclaredKeys(model any) ([]string, *yaml.Node) { + v := reflect.ValueOf(model) + if v.Kind() == reflect.Pointer && v.IsNil() { + return nil, nil + } + obj, ok := model.(parsedObject) + if !ok { + return nil, nil + } + core, ok := obj.GetCoreAny().(unknownReporter) + if !ok { + return nil, nil + } + return slices.Sorted(slices.Values(core.GetUnknownProperties())), obj.GetRootNode() +} diff --git a/compilers/openapi/internal/annotation/unknown_internal_test.go b/compilers/openapi/internal/annotation/unknown_internal_test.go new file mode 100644 index 00000000..764a402b --- /dev/null +++ b/compilers/openapi/internal/annotation/unknown_internal_test.go @@ -0,0 +1,212 @@ +package annotation + +import ( + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/ir" +) + +// TestUnknownKeywordsIn_KeepsWhatTheModelDoesNotName is the census at its own +// subject: a keyword with no field in the schema model reaches the IR under its +// own key, located at itself, and is announced at info. +func TestUnknownKeywordsIn_KeepsWhatTheModelDoesNotName(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: string\nx-ray: kept\nnotAKeyword: 7\n") + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/components/schemas/A", 3) + + require.Len(t, got, 1, "the x-* is the extension reader's, not the census's; got %v", got) + entry := got["openapi:notAKeyword"] + assert.Equal(t, ir.ReasonOutOfScope, entry.Reason) + assert.Equal(t, ir.RawValue("7"), entry.Value) + assert.Equal(t, ir.Provenance{Source: 3, Pointer: "/components/schemas/A/notAKeyword"}, entry.Provenance) + + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityInfo, diags[0].Severity) + assert.Equal(t, "openapi/unknown-schema-keyword", diags[0].Code) + assert.Contains(t, diags[0].Message, `"notAKeyword"`) +} + +// TestUnknownKeywordsIn_DecidedKeywordsAreLeftAlone holds the census to the +// decisions already recorded elsewhere. Each of these has no field in the schema +// model, so the census would otherwise claim all three and overrule a +// deliberate drop — or, for an expanded $dynamicRef, say the compiler ignored a +// reference it resolved. +func TestUnknownKeywordsIn_DecidedKeywordsAreLeftAlone(t *testing.T) { + t.Parallel() + assert.Equal(t, []string{"$comment", "$dynamicAnchor", "$dynamicRef"}, DecidedKeywords, + "a keyword joining or leaving the exclusion must be decided here too") + + var body strings.Builder + body.WriteString("type: string\n") + for _, keyword := range DecidedKeywords { + body.WriteString(keyword + ": v\n") + } + s := schemaFromYAML(t, body.String()) + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/components/schemas/A", 0) + + assert.Empty(t, got, "each is decided about elsewhere, so the census keeps none of them") + assert.Empty(t, diags) +} + +// TestUnknownKeywordsIn_AlreadyRecordedKeyIsLeftAlone is why the census runs +// last. dependentRequired has no field in the model either and is kept by the +// validation-only reader under a reason that says what it is; a census that +// overwrote it would replace that with a weaker one and announce the keyword +// twice. +func TestUnknownKeywordsIn_AlreadyRecordedKeyIsLeftAlone(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: object\ndependentRequired: {a: [b]}\n") + already := ir.UnmodeledEntry{ + Reason: ir.ReasonValidationOnly, + Value: ir.RawValue(`{"a":["b"]}`), + Provenance: ir.Provenance{Pointer: "/A/dependentRequired"}, + } + got := ir.Unmodeled{"openapi:dependentRequired": already} + + diags := UnknownKeywordsIn(&got, s, "/A", 0) + + assert.Equal(t, ir.Unmodeled{"openapi:dependentRequired": already}, got) + assert.Empty(t, diags, "a keyword another reader already announced is not announced twice") +} + +// TestUnknownKeywordsIn_UnpreservableValueIsReportedNotKept holds the census to +// GitHub #144's rule: a value that cannot be rendered as JSON keeps nothing, and +// says so, rather than announcing a preservation that did not happen. +func TestUnknownKeywordsIn_UnpreservableValueIsReportedNotKept(t *testing.T) { + t.Parallel() + s := schemaFromYAML(t, "type: string\nnotAKeyword: .nan\n") + + var got ir.Unmodeled + diags := UnknownKeywordsIn(&got, s, "/A", 0) + + assert.Empty(t, got) + require.Len(t, diags, 1) + assert.Equal(t, ir.SeverityError, diags[0].Severity) + assert.Equal(t, "openapi/unpreservable-construct", diags[0].Code) +} + +// TestUnknownKeysUnder_KeysBeneathTheScopeAndSorted pins the OpenAPI-object +// class: entries key under the path that says which object wrote them, the +// grading is a warning because the format admits no such key, and both the +// entries and the findings come out in key order. +// +// Sorted matters on its own. The library builds its census from a parallel walk +// of the mapping, so the order it hands back is neither source order nor stable, +// and diagnostics ordered by it would make the document non-deterministic +// (invariant 7). +func TestUnknownKeysUnder_KeysBeneathTheScopeAndSorted(t *testing.T) { + t.Parallel() + reported := []string{"zeta", "alpha"} + obj := fakeObject{core: &fakeCore{keys: reported}, root: parsedMapping(t, "zeta: 1\nalpha: 2\n")} + + var got ir.Unmodeled + diags := UnknownKeysUnder(&got, obj, 1, "/info/contact", "info/contact") + + assert.Equal(t, []string{"zeta", "alpha"}, reported, "the model's own slice is not reordered") + require.Len(t, got, 2) + assert.Equal(t, ir.RawValue("2"), got["openapi:info/contact/alpha"].Value) + assert.Equal(t, ir.Provenance{Source: 1, Pointer: "/info/contact/zeta"}, + got["openapi:info/contact/zeta"].Provenance) + + require.Len(t, diags, 2) + assert.Equal(t, ir.SeverityWarning, diags[0].Severity) + assert.Equal(t, "openapi/unknown-object-key", diags[0].Code) + assert.Contains(t, diags[0].Message, `"alpha"`, "the findings follow the sorted keys") + assert.Contains(t, diags[1].Message, `"zeta"`) +} + +// TestUnknownKeysIn_BudgetBoundsWhatOneObjectContributes exercises the bound. An +// object writing more undeclared keys than MaxUnknownKeys keeps exactly that +// many and reports the remainder, which is what stops the discarded tail from +// being the silent loss this census exists to end. +func TestUnknownKeysIn_BudgetBoundsWhatOneObjectContributes(t *testing.T) { + t.Parallel() + const over = MaxUnknownKeys + 3 + keys := make([]string, 0, over) + var body strings.Builder + for i := range over { + // Zero-padded, so sorting the census is sorting the source order too and the + // key that survives the truncation is a predictable one. + key := "k" + strconv.Itoa(1000+i) + keys = append(keys, key) + body.WriteString(key + ": " + strconv.Itoa(i) + "\n") + } + obj := fakeObject{core: &fakeCore{keys: keys}, root: parsedMapping(t, body.String())} + + var got ir.Unmodeled + diags := UnknownKeysIn(&got, obj, 0, "/x") + + assert.Len(t, got, MaxUnknownKeys) + assert.NotContains(t, got, "openapi:k"+strconv.Itoa(1000+over-1), "the tail past the bound is dropped") + require.NotEmpty(t, diags) + assert.Equal(t, "openapi/unknown-key-budget", diags[0].Code) + assert.Equal(t, ir.SeverityWarning, diags[0].Severity) + assert.Equal(t, ir.Provenance{Pointer: "/x"}, diags[0].Provenance) +} + +// TestUnknownKeysIn_ModelWithNoCensusRecordsNothing covers the shapes the reader +// must survive rather than panic on. The absent object is the one that occurs: +// the getters hand back a typed nil for an object the document omitted, and a +// promoted method on one of those dereferences it. +func TestUnknownKeysIn_ModelWithNoCensusRecordsNothing(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + model any + }{ + {"an object the document omitted", (*fakeObjectPtr)(nil)}, + {"an untyped nil", nil}, + {"a value that is no parsed model", 42}, + {"a model whose core keeps no census", fakeObject{core: "not a core"}}, + {"a model with an empty census", fakeObject{core: &fakeCore{}}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var got ir.Unmodeled + + diags := UnknownKeysIn(&got, tc.model, 0, "/x") + + assert.Nil(t, got) + assert.Empty(t, diags) + }) + } +} + +// fakeObject is a parsed model standing in for the library's, so the census can +// be driven at shapes no real document produces — an unsorted census, one past +// the bound, and a core that keeps none. +type fakeObject struct { + core any + root *yaml.Node +} + +func (f fakeObject) GetCoreAny() any { return f.core } +func (f fakeObject) GetRootNode() *yaml.Node { return f.root } + +// fakeObjectPtr is fakeObject's pointer-receiver twin, for the typed-nil case: +// a promoted method on a nil pointer is what the guard exists for, and a +// value-receiver method on a nil pointer would not reach it. +type fakeObjectPtr struct{ fakeObject } + +// fakeCore is a core model's census, reported exactly as given. +type fakeCore struct{ keys []string } + +func (c *fakeCore) GetUnknownProperties() []string { return c.keys } + +// parsedMapping parses body into the mapping node the census reads values from. +func parsedMapping(t *testing.T, body string) *yaml.Node { + t.Helper() + var doc yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(body), &doc)) + return &doc +} diff --git a/compilers/openapi/internal/auth/auth.go b/compilers/openapi/internal/auth/auth.go index b04dbe96..b7287f59 100644 --- a/compilers/openapi/internal/auth/auth.go +++ b/compilers/openapi/internal/auth/auth.go @@ -134,7 +134,13 @@ 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...) + // Distinct from preserveUnreadFields above it: that keeps the fields OpenAPI + // defines for a securityScheme which this entry's own mechanism gives no + // meaning to, while this keeps the keys OpenAPI defines for no securityScheme + // at all. + return scheme, true, append(diags, + annotation.UnknownKeysIn(&scheme.Unmodeled, ss, c.SrcIndex, decl)...) } // mechanismRefusalDiag reports a securitySchemes entry that declares a scheme diff --git a/compilers/openapi/internal/diag/diag.go b/compilers/openapi/internal/diag/diag.go index 59027084..5a9a12b8 100644 --- a/compilers/openapi/internal/diag/diag.go +++ b/compilers/openapi/internal/diag/diag.go @@ -191,6 +191,40 @@ const ( // DegradedConstruct's constructs survive in a weaker shape, and these survive // in none (GitHub #144). UnpreservableConstruct = "openapi/unpreservable-construct" + // UnknownSchemaKeyword reports a JSON Schema keyword no field of the schema + // model names, kept verbatim under Unmodeled. + // + // Info, because the document did nothing wrong: JSON Schema requires an + // implementation to ignore a keyword it does not recognize, and says such a + // keyword may carry meaning for other tooling, so an unrecognized keyword is + // legal input rather than a defect. What is recorded is this compiler's own + // decision — that it read no meaning from the keyword and kept the text — which + // is the same thing ValidationOnlyKeyword records beside it. + UnknownSchemaKeyword = "openapi/unknown-schema-keyword" + // UnknownObjectKey reports a key on an OpenAPI object that the specification + // neither defines nor admits as an extension, kept verbatim under Unmodeled. + // + // Warning rather than info, because unlike its schema neighbour this one is a + // defect: OpenAPI gives its objects a closed key set and requires every + // extension to be prefixed x-, so a key that is neither is a document error — + // in practice a misspelling of the field beside it, which is precisely the + // class of mistake that survives when the compiler swallows the key in silence. + // + // Warning rather than error for the reason ReservedHeaderName is one: the + // document still lowers, everything the key was written beside is unaffected, + // and harness.Check stops at the first error diagnostic, which would hide every + // later finding in the same spec and make any fixture carrying a stray key + // unable to reach the invariant checks. + UnknownObjectKey = "openapi/unknown-object-key" + // UnknownKeyBudget reports an object declaring more keys the model does not + // name than the compiler keeps, so the ones past the bound reached the IR in no + // form at all. + // + // Every collection here is bounded, and this one is over a key set the document + // chooses the size of. The bound is far above what any document writes by + // accident, so tripping it is either a generated file or a hostile one; the + // diagnostic is what keeps the discarded remainder from being a silent loss. + UnknownKeyBudget = "openapi/unknown-key-budget" ) // Newf builds an ir.Diagnostic with a formatted message. It is the single diff --git a/compilers/openapi/internal/diag/diag_test.go b/compilers/openapi/internal/diag/diag_test.go index 26e351ac..8b172081 100644 --- a/compilers/openapi/internal/diag/diag_test.go +++ b/compilers/openapi/internal/diag/diag_test.go @@ -138,6 +138,7 @@ func codes() []string { diag.AliasAmplification, diag.UnattachableRequired, diag.InternalInvariant, diag.DuplicateOperationID, diag.IncompleteSecurityScheme, diag.ReservedHeaderName, diag.UnpreservableConstruct, + diag.UnknownSchemaKeyword, diag.UnknownObjectKey, diag.UnknownKeyBudget, } } diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index c202731a..94bb76fa 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/compilers/openapi/internal/operation/content.go @@ -88,7 +88,8 @@ func lowerContent(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex if len(ext) > 0 { content.Unmodeled = annotation.MergeUnmodeled(content.Unmodeled, ext) } - return content, diags + return content, append(diags, + annotation.UnknownKeysIn(&content.Unmodeled, media, c.SrcIndex, mediaPtr)...) } // fillSequential lowers 3.2 sequential-media fields: itemSchema becomes the @@ -512,7 +513,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, annotation.UnknownKeysIn(&p.Unmodeled, h, c.SrcIndex, hdecl)...) } // exampleList lowers a single example node and a plural example map into value @@ -604,6 +605,11 @@ func lowerRequestBody(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, bodyPtr, "request body is not required; optionality kept under Unmodeled")) } + // After the payload guard, because ir.Payload is the body's only carrier: a + // request body declaring no content lowers to nothing to hang them on, and + // OpenAPI makes content REQUIRED there, so such a body is a defect in the + // document rather than a shape this compiler has to place. + diags = append(diags, annotation.UnknownKeysIn(&payload.Unmodeled, rb, c.SrcIndex, bodyPtr)...) op.Request = payload hb.RequestContentTypes = contentTypeKeys(rb.GetContent()) return diags diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 351fb057..0c3bf09b 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -293,6 +293,7 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd op.Unmodeled = ext } // After the extensions assignment, which would otherwise overwrite the map. + diags = append(diags, annotation.UnknownKeysIn(&op.Unmodeled, src, c.SrcIndex, decl)...) diags = append(diags, applyOperationServers(c, &op, src, decl)...) return op, extra, append(diags, checkOperationIDUnique(c, operationIDs, op, mount)...) } @@ -465,7 +466,20 @@ func lowerResponse(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde resp.Docs.Description = r.GetDescription() _, linkDiags := schema.PreserveNode(c, &resp.Unmodeled, "openapi:links", annotation.RawChildNode(r.GetRootNode(), "links"), ir.ReasonNoIRHome, rptr+ids.Ptr("links")) - return resp, append(diags, linkDiags...) + diags = append(diags, linkDiags...) + return resp, append(diags, preserveResponseUnknownKeys(c, &resp.Unmodeled, r, rptr)...) +} + +// preserveResponseUnknownKeys keeps the keys a Response Object writes that the +// specification does not define. +// +// One helper for both branches on purpose. ir.Response and ir.ErrorCase are two +// lowerings of the same source object, and a construct kept on only one of them +// makes a declaration survive or vanish on nothing but its status code — which +// is how a response's links came to be kept on a 2xx and dropped on a 4xx +// (GitHub #275). +func preserveResponseUnknownKeys(c lowering.Ctx, p *ir.Unmodeled, r *soa.Response, rptr string) []ir.Diagnostic { + return annotation.UnknownKeysIn(p, r, c.SrcIndex, rptr) } // responseName builds a success response's neutral naming. OpenAPI names no @@ -497,7 +511,8 @@ func lowerErrorCase(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd } ec.Docs.Description = r.GetDescription() diags := fillErrorType(c, ts, anchors, &ec, r, rptr) - return ec, append(diags, preserveErrorHeaders(c, &ec, r, rptr)...) + diags = append(diags, preserveErrorHeaders(c, &ec, r, rptr)...) + return ec, append(diags, preserveResponseUnknownKeys(c, &ec.Unmodeled, r, rptr)...) } // preserveErrorHeaders keeps an error response's headers from being dropped: diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index e0875ee3..c7bcc3a6 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -201,7 +201,10 @@ func fillParamSchemaAnnotations(c lowering.Ctx, ts *compile.Types, param *ir.Par diags = append(diags, preserveParamXML(c, param, s, pointer)...) } param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, a.Unmodeled) - return diags + // Last, per schema.PreserveUnknownKeywords: it keeps only the keywords no + // reader above it kept, and everything annotation.Read recorded is already on + // the parameter by this line. + return append(diags, schema.PreserveUnknownKeywords(c, ¶m.Unmodeled, s, pointer)...) } // preserveParamXML keeps a parameter schema's xml hints instead of dropping @@ -274,6 +277,7 @@ func fillParamDetail(c lowering.Ctx, param *ir.Parameter, p *soa.Parameter, pptr pExt, extDiags := schema.ExtensionsOf(c, p.GetExtensions(), pptr) diags = append(diags, extDiags...) param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, pExt) + diags = append(diags, annotation.UnknownKeysIn(¶m.Unmodeled, p, c.SrcIndex, pptr)...) return append(diags, preserveAllowEmptyValue(c, param, p, pptr)...) } diff --git a/compilers/openapi/internal/schema/accumulate.go b/compilers/openapi/internal/schema/accumulate.go index bf2a5fac..836687c2 100644 --- a/compilers/openapi/internal/schema/accumulate.go +++ b/compilers/openapi/internal/schema/accumulate.go @@ -73,6 +73,20 @@ func PreserveSchemaKeyword(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, keyw return PreserveNode(c, p, "openapi:"+keyword, annotation.RawPropertyNode(s, keyword), reason, pointer) } +// PreserveUnknownKeywords records every keyword s writes that no field of the +// schema model names and no reader above it already kept (GitHub #297). +// +// It is the last thing an attachment does, which is the contract +// annotation.UnknownKeywordsIn states and the reason it is called here rather +// than from inside annotation.Read with the other readers: $dynamicRef has no +// field in the schema model either and is decided by recordUnexpandedDynamicRef, +// which runs after Read and deliberately keeps nothing once the reference has +// expanded. A census running before it could not tell that from an unread +// keyword. +func PreserveUnknownKeywords(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, pointer string) []ir.Diagnostic { + return annotation.UnknownKeywordsIn(p, s, pointer, c.SrcIndex) +} + // preserveKeyword records a validation-only keyword's raw payload under key in // p and returns the one info diagnostic naming it at declPtr, the schema that // wrote it. An absent or unconvertible payload records nothing and returns diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb9..b5cc509f 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -894,7 +894,8 @@ func fillPropertyAnnotations(c lowering.Ctx, ts *compile.Types, anchors *AnchorI // 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)...) - return append(diags, recordUnexpandedDynamicRef(c, anchors, &p.Unmodeled, ref, pointer)...) + diags = append(diags, recordUnexpandedDynamicRef(c, anchors, &p.Unmodeled, ref, pointer)...) + return append(diags, PreserveUnknownKeywords(c, &p.Unmodeled, ref, pointer)...) } // LoweredToOwnNode reports whether the declaration at pointer lowered to a type @@ -978,7 +979,8 @@ func attachDeclaredAnnotations(c lowering.Ctx, ts *compile.Types, anchors *Ancho common.Examples = a.Examples } diags = append(diags, recordUnplacedContent(c, &common.Unmodeled, s, td, pointer)...) - return append(diags, recordUnexpandedDynamicRef(c, anchors, &common.Unmodeled, s, pointer)...) + diags = append(diags, recordUnexpandedDynamicRef(c, anchors, &common.Unmodeled, s, pointer)...) + return append(diags, PreserveUnknownKeywords(c, &common.Unmodeled, s, pointer)...) } // fillAdditional lowers additionalProperties, patternProperties, and diff --git a/compilers/openapi/meta.go b/compilers/openapi/meta.go index 23191d28..b46de471 100644 --- a/compilers/openapi/meta.go +++ b/compilers/openapi/meta.go @@ -1,10 +1,13 @@ package openapi import ( + "strconv" + soa "github.com/speakeasy-api/openapi/openapi" "github.com/dexpace/morphic/compilers/compile" "github.com/dexpace/morphic/compilers/openapi/internal/annotation" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" "github.com/dexpace/morphic/ir" ) @@ -38,11 +41,74 @@ type docMeta struct { // service graph: info, servers, and top-level extensions (ir-design §10, §12). func lowerMeta(c lowering.Ctx) (docMeta, []ir.Diagnostic) { m := lowerInfo(c) - m.Servers = lowerServers(c) + + servers, serverDiags := lowerServers(c) + m.Servers = servers ext, diags := annotation.ExtensionsFrom(c.Doc.GetExtensions(), c.SrcIndex, "") m.Unmodeled = ext - return m, diags + diags = append(diags, serverDiags...) + return m, append(diags, documentUnknownKeys(c, &m.Unmodeled)...) +} + +// documentUnknownKeys collects the keys the OpenAPI model names no field for +// from every object around the document metadata that lowers to no node of its +// own: the document root, the info block and the contact and license inside it, +// the root externalDocs, and each declared tag. +// +// ir.Document is the nearest node with an Unmodeled map for all of them, so each +// object's keys are scoped by the source path they were written at. One unscoped +// "openapi:status" would be a single key for six objects, and the entry that +// survived would be whichever site ran last. +func documentUnknownKeys(c lowering.Ctx, p *ir.Unmodeled) []ir.Diagnostic { + sites := append(rootUnknownSites(c), tagUnknownSites(c)...) + diags := make([]ir.Diagnostic, 0, len(sites)) + for _, site := range sites { + diags = append(diags, + annotation.UnknownKeysUnder(p, site.model, c.SrcIndex, site.owner, site.scope)...) + } + return diags +} + +// unknownSite is one object's census: what it keys under on the carrier holding +// it, the object's own source pointer, and the parsed object itself. +type unknownSite struct { + scope string + owner string + model any +} + +// rootUnknownSites returns the census sites a document has exactly one of. The +// root's keys take no scope, since ir.Document stands for the OpenAPI Object +// itself; the rest are keyed by the path from it down to the object that wrote +// them. +func rootUnknownSites(c lowering.Ctx) []unknownSite { + info := c.Doc.GetInfo() + infoPtr := ids.Ptr("info") + return []unknownSite{ + {"", "", c.Doc}, + {"info", infoPtr, info}, + {"info/contact", infoPtr + ids.Ptr("contact"), info.GetContact()}, + {"info/license", infoPtr + ids.Ptr("license"), info.GetLicense()}, + {"externalDocs", ids.Ptr("externalDocs"), c.Doc.GetExternalDocs()}, + } +} + +// tagUnknownSites returns one census site per declared tag, since ir.TagDef +// holds no Unmodeled map for a tag's own keys to land on. +// +// Scoped by index rather than name, which is the pointer a tag is written at. A +// name would read better, but OpenAPI's requirement that tag names be unique is +// the document's to keep and not this compiler's to rely on: two tags spelled +// alike would silently leave one entry. +func tagUnknownSites(c lowering.Ctx) []unknownSite { + tags := c.Doc.GetTags() + out := make([]unknownSite, 0, len(tags)) + for i, t := range tags { + index := strconv.Itoa(i) + out = append(out, unknownSite{"tags/" + index, ids.Ptr("tags", index), t}) + } + return out } // lowerInfo maps info onto the document identity, docs, contact, and license. @@ -79,31 +145,37 @@ func infoDocs(c lowering.Ctx, info *soa.Info) ir.Docs { // template, description, and templated variables (ir-design §10). It returns nil // rather than an empty slice when every entry was skipped, so a document // declaring no usable server leaves the field unset. -func lowerServers(c lowering.Ctx) []ir.Server { +func lowerServers(c lowering.Ctx) ([]ir.Server, []ir.Diagnostic) { // GetServers never returns an empty slice — it injects a default "/" server // when none are declared — so the loop always runs at least once. servers := c.Doc.GetServers() out := make([]ir.Server, 0, len(servers)) - for _, s := range servers { + var diags []ir.Diagnostic + for i, s := range servers { if s == nil { continue } - out = append(out, lowerServer(s)) + one, serverDiags := lowerServer(c, s, ids.Ptr("servers", strconv.Itoa(i))) + diags = append(diags, serverDiags...) + out = append(out, one) } if len(out) == 0 { - return nil + return nil, diags } - return out + return out, diags } -// lowerServer lowers one server, named by serverName. -func lowerServer(s *soa.Server) ir.Server { - return ir.Server{ +// lowerServer lowers one server, named by serverName; sptr is the server's own +// pointer in the servers list. +func lowerServer(c lowering.Ctx, s *soa.Server, sptr string) (ir.Server, []ir.Diagnostic) { + vars, diags := serverVariables(c, s, sptr) + out := ir.Server{ Name: serverName(s), URLTemplate: s.GetURL(), Description: ir.Docs{Description: s.GetDescription()}, - Variables: serverVariables(s), + Variables: vars, } + return out, append(diags, annotation.UnknownKeysIn(&out.Unmodeled, s, c.SrcIndex, sptr)...) } // serverName builds a server's neutral naming: the declared name when the source @@ -134,22 +206,26 @@ func serverName(s *soa.Server) ir.Naming { // serverVariables lowers a server's URL template variables in source order, or // nil when it declares none. -func serverVariables(s *soa.Server) []ir.ServerVariable { +func serverVariables(c lowering.Ctx, s *soa.Server, sptr string) ([]ir.ServerVariable, []ir.Diagnostic) { vars := s.GetVariables() if vars == nil || vars.Len() == 0 { - return nil + return nil, nil } out := make([]ir.ServerVariable, 0, vars.Len()) + var diags []ir.Diagnostic for name, v := range vars.All() { if v == nil { continue } - out = append(out, ir.ServerVariable{ + one := ir.ServerVariable{ Name: name, Default: v.GetDefault(), Enum: v.GetEnum(), Docs: ir.Docs{Description: v.GetDescription()}, - }) + } + diags = append(diags, annotation.UnknownKeysIn(&one.Unmodeled, v, c.SrcIndex, + sptr+ids.Ptr("variables", name))...) + out = append(out, one) } - return out + return out, diags } diff --git a/compilers/openapi/meta_test.go b/compilers/openapi/meta_test.go index 528f6243..212936e6 100644 --- a/compilers/openapi/meta_test.go +++ b/compilers/openapi/meta_test.go @@ -103,7 +103,7 @@ func TestServerName_DerivedFromURLWhenUnnamed(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tc.want, lowerServer(&soa.Server{URL: tc.url}).Name) + assert.Equal(t, tc.want, serverName(&soa.Server{URL: tc.url})) }) } } @@ -114,9 +114,9 @@ func TestServerName_DerivedFromURLWhenUnnamed(t *testing.T) { // both servers the same. func TestServerName_DistinguishesServersDifferingOnlyInPath(t *testing.T) { t.Parallel() - v1 := lowerServer(&soa.Server{URL: "https://api.example.com/v1"}) - v2 := lowerServer(&soa.Server{URL: "https://api.example.com/v2"}) - assert.NotEqual(t, v1.Name.Hint, v2.Name.Hint, "two distinct servers get two distinct hints") + v1 := serverName(&soa.Server{URL: "https://api.example.com/v1"}) + v2 := serverName(&soa.Server{URL: "https://api.example.com/v2"}) + assert.NotEqual(t, v1.Hint, v2.Hint, "two distinct servers get two distinct hints") } // TestServerName_CollidesOnPunctuationAlone bounds that claim, which must not be @@ -126,17 +126,18 @@ func TestServerName_DistinguishesServersDifferingOnlyInPath(t *testing.T) { // known bound rather than a later discovery. func TestServerName_CollidesOnPunctuationAlone(t *testing.T) { t.Parallel() - dotted := lowerServer(&soa.Server{URL: "https://api.example.com/v1"}) - dashed := lowerServer(&soa.Server{URL: "https://api.example.com/v-1"}) - assert.Equal(t, dotted.Name.Hint, dashed.Name.Hint, + dotted := serverName(&soa.Server{URL: "https://api.example.com/v1"}) + dashed := serverName(&soa.Server{URL: "https://api.example.com/v-1"}) + assert.Equal(t, dotted.Hint, dashed.Hint, "neutral words carry no punctuation, so these two collide") } func TestLowerServers_NilEntrySkipped(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Servers: []*soa.Server{nil, {URL: "https://x.example.com"}}} - got := lowerServers(lowering.Ctx{Doc: doc}) + got, diags := lowerServers(lowering.Ctx{Doc: doc}) + assert.Empty(t, diags) require.Len(t, got, 1, "nil server entry skipped, valid one lowered") assert.Equal(t, "https://x.example.com", got[0].URLTemplate) } @@ -147,7 +148,8 @@ func TestServerVariables_NilEntrySkipped(t *testing.T) { sequencedmap.NewElem("skip", (*soa.ServerVariable)(nil)), sequencedmap.NewElem("keep", &soa.ServerVariable{}), ) - srv := lowerServer(&soa.Server{URL: "https://x", Variables: vars}) + srv, diags := lowerServer(lowering.Ctx{}, &soa.Server{URL: "https://x", Variables: vars}, "/servers/0") + assert.Empty(t, diags) require.Len(t, srv.Variables, 1, "nil variable entry skipped") assert.Equal(t, "keep", srv.Variables[0].Name) } @@ -159,5 +161,7 @@ func TestLowerServers_EveryEntrySkippedIsNil(t *testing.T) { t.Parallel() doc := &soa.OpenAPI{Servers: []*soa.Server{nil, nil}} - assert.Nil(t, lowerServers(lowering.Ctx{Doc: doc})) + got, diags := lowerServers(lowering.Ctx{Doc: doc}) + assert.Nil(t, got) + assert.Empty(t, diags) } diff --git a/compilers/openapi/unknownkeys_test.go b/compilers/openapi/unknownkeys_test.go new file mode 100644 index 00000000..ba0d55a0 --- /dev/null +++ b/compilers/openapi/unknownkeys_test.go @@ -0,0 +1,200 @@ +// This file covers one property: a key the OpenAPI model names no field for +// reaches the IR rather than vanishing between the parser and the lowering. It +// is separate from annotations_test.go because the subject is the complement of +// what the model names, not any one annotation. +package openapi_test // external test package — exercises only the public API + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/ir" +) + +// unknownKeysSpec reads the census fixture the corpus sweeps also drive, so the +// assertions below and the six oracles run over the same bytes: putting it under +// testdata is what gets it compiled in both declaration orders, round-tripped and +// verified, none of which a spec written inline reaches. +func unknownKeysSpec(t *testing.T) string { + t.Helper() + data, err := os.ReadFile(filepath.Join("..", "..", "testdata", "openapi", "unknown_keys.yaml")) + require.NoError(t, err) + return string(data) +} + +// TestUnknownKeys_KeptAtEveryObject holds the whole rule rather than the +// positions that happened to be noticed. A key the model does not name reached +// no IR field, no Unmodeled entry and no diagnostic at every one of these +// objects, so two documents differing only in it compiled to the same IR +// (GitHub #297). +// +// Carriers are derived from the value graph rather than named: each row says +// which Unmodeled map the entry must land on by the path the walk reaches it at, +// so an entry written to the wrong carrier fails here rather than passing +// because the assertion looked only where it expected. +func TestUnknownKeys_KeptAtEveryObject(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "unknown-keys", unknownKeysSpec(t)) + requireNoErrorDiagnostics(t, diags) + sites := unmodeledSitesOf(doc) + + for _, tc := range []struct { + object string + key string + want string + carrier string + }{ + {"openapi root", "openapi:basePath", `"ROOT"`, "doc.Unmodeled"}, + {"info", "openapi:info/contactEmail", `"INFO"`, "doc.Unmodeled"}, + {"contact", "openapi:info/contact/slack", `"CONTACT"`, "doc.Unmodeled"}, + {"license", "openapi:info/license/spdx", `"LICENSE"`, "doc.Unmodeled"}, + {"externalDocs", "openapi:externalDocs/title", `"EXTERNALDOCS"`, "doc.Unmodeled"}, + {"server", "openapi:host", `"SERVER"`, "doc.Servers[0].Unmodeled"}, + {"server variable", "openapi:example", `"SERVERVARIABLE"`, "doc.Servers[0].Variables[0].Unmodeled"}, + {"tag", "openapi:tags/0/color", `"TAG"`, "doc.Unmodeled"}, + {"operation", "openapi:operationid", `"OPERATION"`, ".Unmodeled"}, + {"parameter", "openapi:collectionFormat", `"PARAMETER"`, ".Params[0].Unmodeled"}, + {"request body", "openapi:schema", `"REQUESTBODY"`, ".Request.Unmodeled"}, + {"media type", "openapi:format", `"MEDIATYPE"`, ".Contents[0].Unmodeled"}, + {"response", "openapi:status", `"RESPONSE"`, ".Responses[0].Unmodeled"}, + {"error response", "openapi:status", `"ERRORRESPONSE"`, ".Errors[0].Unmodeled"}, + {"header", "openapi:in", `"HEADER"`, ".Headers[0].Unmodeled"}, + {"security scheme", "openapi:tokenUrl", `"SECURITYSCHEME"`, + "doc.Auth[auth/openapi/components/securitySchemes/k].Unmodeled"}, + {"schema", "openapi:additionalItems", `"SCHEMA"`, + "doc.Types[t/openapi/components/schemas/S].Unmodeled"}, + // The property's schema reduced to a shared primitive, so it owns no node + // and its keywords stay on the declaring property — the carrier rule + // fillPropertyAnnotations already applies to every other annotation. + {"property schema", "openapi:divisibleBy", `"PROPERTYSCHEMA"`, + "doc.Types[t/openapi/components/schemas/S].Properties[0].Unmodeled"}, + } { + site, found := findUnmodeledSite(sites, tc.key, tc.want) + if !assert.True(t, found, "%s drops %s = %s: it is nowhere in the document", + tc.object, tc.key, tc.want) { + continue + } + assert.Contains(t, site.path, tc.carrier, "%s key lands on the wrong carrier", tc.object) + } +} + +// TestUnknownKeys_SchemaAndObjectAreGradedApart pins the one distinction the +// census turns on, at the two keys the fixture picks for it: a draft-07 +// additionalItems in a schema, and an operationId with the case wrong on an +// operation. +// +// JSON Schema states that an implementation must ignore a keyword it does not +// recognize, so an unrecognized keyword in a schema is legal input and may carry +// meaning for other tooling; OpenAPI states that an extension key must be +// prefixed x-, so an undefined key on one of its objects is not an extension but +// a defect in the document. +// +// Both are kept — invariant 2 does not bend for invalid input — and both carry +// the same reason, because "no IR node is coming" is true of each. What differs +// is what the document did, which is the diagnostic channel's subject: an +// unrecognized keyword records a decision at info, and an undefined key reports +// a fault at warning. +func TestUnknownKeys_SchemaAndObjectAreGradedApart(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "unknown-keys", unknownKeysSpec(t)) + requireNoErrorDiagnostics(t, diags) + + schema, ok := doc.Types[ir.TypeID("t/openapi/components/schemas/S")] + require.True(t, ok) + keyword := unmodeledEntry(t, schema.Common().Unmodeled, "openapi:additionalItems") + assert.Equal(t, ir.ReasonOutOfScope, keyword.Reason, + "no IR node is coming for a keyword this compiler does not model") + assert.Equal(t, "/components/schemas/S/additionalItems", keyword.Provenance.Pointer) + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, + diagsAt(diags, "openapi/unknown-schema-keyword", "/components/schemas/S/additionalItems")) + + op, ok := opByName(doc, "listWidgets") + require.True(t, ok) + key := unmodeledEntry(t, op.Unmodeled, "openapi:operationid") + assert.Equal(t, ir.ReasonOutOfScope, key.Reason, + "OpenAPI defines no such key, so no IR node is coming for it either") + assert.Equal(t, "/paths/~1widgets/get/operationid", key.Provenance.Pointer) + assert.Equal(t, []ir.Severity{ir.SeverityWarning}, + diagsAt(diags, "openapi/unknown-object-key", "/paths/~1widgets/get/operationid")) +} + +// TestUnknownKeys_WellFormedDocumentRecordsNothing is the control the census +// needs: a document writing only what the model names keeps no entry and reports +// no finding, so the rows above are evidence of the keys they name rather than +// of a sweep that fires on everything. +func TestUnknownKeys_WellFormedDocumentRecordsNothing(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "clean", `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /widgets: + get: + operationId: listWidgets + responses: {"200": {description: ok}} +components: + schemas: + S: {type: object, properties: {a: {type: string}}} +`) + requireNoErrorDiagnostics(t, diags) + + assert.Empty(t, unmodeledSitesOf(doc), "nothing undeclared, so nothing kept") + for _, d := range diags { + assert.NotContains(t, d.Code, "unknown-", "a well-formed document reports no unknown key: %+v", d) + } +} + +// unmodeledSite is one Unmodeled entry paired with the walk path of the map +// holding it. +type unmodeledSite struct { + key string + path string + entry ir.UnmodeledEntry +} + +// unmodeledSitesOf returns every Unmodeled entry the document holds, found by +// walking the value graph rather than by naming the carriers a test expects. +func unmodeledSitesOf(doc *ir.Document) []unmodeledSite { + unmodeledType := reflect.TypeOf(ir.Unmodeled(nil)) + var out []unmodeledSite + ir.WalkValues(doc, ir.DocumentPath, func(v reflect.Value, path string) bool { + if v.Type() != unmodeledType || !v.CanInterface() { + return true + } + u, ok := v.Interface().(ir.Unmodeled) + if !ok { + return true + } + for key, entry := range u { + out = append(out, unmodeledSite{key: key, path: path, entry: entry}) + } + return true + }) + return out +} + +// findUnmodeledSite returns the site holding key with the given JSON value. The +// value is part of the match because one key spelling occurs at several +// carriers — "openapi:status" is written on two responses in this fixture — so +// matching on the key alone would find another object's entry and call it a +// pass. +func findUnmodeledSite(sites []unmodeledSite, key, wantJSON string) (unmodeledSite, bool) { + for _, site := range sites { + if site.key == key && string(site.entry.Value) == wantJSON { + return site, true + } + } + return unmodeledSite{}, false +} + +// requireNoErrorDiagnostics fails the test on the first error-severity +// diagnostic, naming it. +func requireNoErrorDiagnostics(t *testing.T, diags []ir.Diagnostic) { + t.Helper() + d, ok := ir.FirstError(diags) + require.False(t, ok, "unexpected error diagnostic: %+v", d) +} diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json index c43e9937..3218cb4f 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json @@ -44,7 +44,7 @@ }, "anonymous": true, "docs": { - "description": "named by position" + "description": "named by position, not by target" }, "sensitive": false, "provenance": { @@ -366,7 +366,7 @@ "eventPayload": false, "secret": false, "docs": { - "description": "named by position" + "description": "named by position, not by target" }, "provenance": { "source": 0, @@ -548,7 +548,7 @@ { "format": "openapi@3.1", "path": "allof-oneof-cooccurrence.yaml", - "hash": "cb7fb6bc613222b0211fc363b9028e7739b44a1d14b04f74cf709b8d603655f2" + "hash": "580c2ffd45b8b24e175b4c5b7f1af6c237a920de7d1942d84b10a00ad2dc1e33" } ] } diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml index f085da90..042727df 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml @@ -56,5 +56,5 @@ components: branch: {$ref: '#/components/schemas/InlineHost/oneOf/0'} InlineHost: oneOf: - - {type: integer, description: named by position, not by target} + - {type: integer, description: 'named by position, not by target'} - {type: string} diff --git a/testdata/conformance/openapi/unwitnessed.golden.txt b/testdata/conformance/openapi/unwitnessed.golden.txt index 0ef3569a..ca1e200c 100644 --- a/testdata/conformance/openapi/unwitnessed.golden.txt +++ b/testdata/conformance/openapi/unwitnessed.golden.txt @@ -195,8 +195,6 @@ Server.Bindings Server.Protocol Server.ProtocolVersion Server.Tags -Server.Unmodeled -ServerVariable.Unmodeled Service.CommonErrors Service.Extends Service.Namespace diff --git a/testdata/openapi/unknown_keys.yaml b/testdata/openapi/unknown_keys.yaml new file mode 100644 index 00000000..7a49ac00 --- /dev/null +++ b/testdata/openapi/unknown_keys.yaml @@ -0,0 +1,59 @@ +# One key the OpenAPI model names no field for at every object the compiler takes +# a census from, each valued with the object it was written on so an entry found +# at the wrong carrier cannot pass for the right one. +# +# Every key here is one real documents carry rather than invented nonsense: a +# Swagger 2.0 field with no OpenAPI 3 equivalent (basePath, host, a body +# parameter's schema), a keyword from an older JSON Schema draft (additionalItems, +# divisibleBy), a field belonging to a neighbouring object (a flow's tokenUrl on +# the scheme, a parameter's `in` on a header), or operationId with the case wrong. +# None of them reached an IR field, an Unmodeled entry or a diagnostic. +openapi: 3.1.0 +basePath: ROOT +info: + title: T + version: "1" + contactEmail: INFO + contact: {name: n, slack: CONTACT} + license: {name: MIT, spdx: LICENSE} +externalDocs: {url: 'https://d.example', title: EXTERNALDOCS} +servers: + - url: 'https://a.example/{region}' + host: SERVER + variables: + region: {default: us, example: SERVERVARIABLE} +tags: + - {name: t1, color: TAG} +paths: + /widgets: + get: + operationId: listWidgets + tags: [t1] + operationid: OPERATION + parameters: + - {name: shape, in: query, schema: {type: string}, collectionFormat: PARAMETER} + requestBody: + required: true + schema: REQUESTBODY + content: + application/json: + schema: {$ref: '#/components/schemas/S'} + format: MEDIATYPE + responses: + "200": + description: ok + status: RESPONSE + headers: + X-Trace: {schema: {type: string}, in: HEADER} + "404": + description: gone + status: ERRORRESPONSE +components: + securitySchemes: + k: {type: apiKey, in: header, name: X-Key, tokenUrl: SECURITYSCHEME} + schemas: + S: + type: object + additionalItems: SCHEMA + properties: + a: {type: string, divisibleBy: PROPERTYSCHEMA} From 053feb278bcac24fe7ffc8a9b3b9a883a6e9d9aa Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 07:32:41 +0300 Subject: [PATCH 3/3] fix(compilers/openapi): keep undeclared keys on nested objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A key the OpenAPI model names no field for was kept at the objects that lower to a node with an Unmodeled map of their own, and dropped in silence at the objects nested inside one: an example, an encoding, the oauth flows and each flow, a schema's xml, discriminator and externalDocs, an operation's and a tag's externalDocs, and the components object. Those had no Unmodeled map to land on until the carriers went in; this reads the census at each of them and keys the entries under the same scheme the extensions there already use, so an entry says which object wrote it and two objects reaching one map cannot collide. The components object was classified as a map with nothing to census. It is not: only the map under each of its keys is the document's to name, while its own key set is the fixed list of component kinds, which the library models as named fields and takes a census over. `paths`, `responses` and a callback are the real maps of that kind, and each is confirmed to fold an unrecognized key into itself rather than report it. Two objects are deliberately left out, both recorded at the code: - A path item. The library folds an unrecognized key into the item's embedded operations map, so there is no census to read; recovering the value needs a method vocabulary wider than the one this compiler owns, which is what #293 is about. Unlike every object above, the key is not lost in silence — folding it reports a type mismatch at error severity naming the key at its own pointer. Filed as #377. - A Link Object, following the same decision made for its extensions. This compiler lowers no Link Object anywhere: a response's links survive only as a verbatim node, and an unreferenced components link is dropped whole. A schema's xml, discriminator and externalDocs are censused but stay out of the corpus fixture. The OpenAPI dialect meta-schema closes all three to anything but an x- key, so an undeclared key there draws a library validation error, and an error diagnostic stops harness.Check before the oracles that fixture exists to reach. --- .../openapi/internal/annotation/annotation.go | 34 +++++---- compilers/openapi/internal/auth/auth.go | 32 +++++---- .../openapi/internal/operation/content.go | 14 ++-- .../openapi/internal/operation/operations.go | 52 +++++++++++--- compilers/openapi/meta.go | 21 ++++-- compilers/openapi/unknownkeys_test.go | 71 +++++++++++++++++++ testdata/openapi/unknown_keys.yaml | 48 +++++++++++-- 7 files changed, 221 insertions(+), 51 deletions(-) diff --git a/compilers/openapi/internal/annotation/annotation.go b/compilers/openapi/internal/annotation/annotation.go index 518973bc..a2125148 100644 --- a/compilers/openapi/internal/annotation/annotation.go +++ b/compilers/openapi/internal/annotation/annotation.go @@ -544,7 +544,7 @@ func Read(st Site, pointer string, srcIndex int) (Set, []ir.Diagnostic) { out.Examples = examples ext, extDiags := ExtensionsFrom(st.Node.GetExtensions(), srcIndex, pointer) - sub, subDiags := subObjectExtensions(st.Node, pointer, srcIndex) + sub, subDiags := subObjectKeys(st.Node, pointer, srcIndex) kept, keptDiags := unmodeledAt(st.Node, pointer, srcIndex) diags := make([]ir.Diagnostic, 0, len(exDiags)+len(extDiags)+len(subDiags)+len(keptDiags)) @@ -557,27 +557,37 @@ func Read(st Site, pointer string, srcIndex int) (Set, []ir.Diagnostic) { return out, diags } -// subObjectExtensions collects the x-* the sub-objects of a schema declare — -// its xml, its discriminator and its externalDocs. Each is an OpenAPI object -// that admits extensions, and none of ir.XMLHints, ir.Discriminator or ir.Link -// holds an Unmodeled map, so the entries ride on the node the schema itself -// lowers to; the keyword each was written under is what keeps three objects' -// extensions apart on that one map. -func subObjectExtensions(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, []ir.Diagnostic) { +// subObjectKeys collects what the sub-objects of a schema declare that reaches +// no IR field — the x-* they carry and the keys the specification defines for +// none of them — over its xml, its discriminator and its externalDocs. +// +// Each is an OpenAPI object with its own closed key set, and none of +// ir.XMLHints, ir.Discriminator or ir.Link holds an Unmodeled map, so the +// entries ride on the node the schema itself lowers to; the keyword each was +// written under is what keeps three objects' entries apart on that one map. +// +// The census is graded as an OpenAPI object's rather than as a schema keyword's, +// even though these hang off a schema: the JSON Schema rule that an unrecognized +// keyword is legal governs the schema itself, and these three are OpenAPI +// objects that the schema vocabulary says nothing about. +func subObjectKeys(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, []ir.Diagnostic) { subs := []struct { keyword string + obj any ext *extensions.Extensions }{ - {"xml", s.GetXML().GetExtensions()}, - {"discriminator", s.GetDiscriminator().GetExtensions()}, - {"externalDocs", s.GetExternalDocs().GetExtensions()}, + {"xml", s.GetXML(), s.GetXML().GetExtensions()}, + {"discriminator", s.GetDiscriminator(), s.GetDiscriminator().GetExtensions()}, + {"externalDocs", s.GetExternalDocs(), s.GetExternalDocs().GetExtensions()}, } var out ir.Unmodeled var diags []ir.Diagnostic for _, sub := range subs { - ext, extDiags := ExtensionsUnder(sub.ext, srcIndex, pointer+ids.Ptr(sub.keyword), sub.keyword) + owner := pointer + ids.Ptr(sub.keyword) + ext, extDiags := ExtensionsUnder(sub.ext, srcIndex, owner, sub.keyword) out = MergeUnmodeled(out, ext) diags = append(diags, extDiags...) + diags = append(diags, UnknownKeysUnder(&out, sub.obj, srcIndex, owner, sub.keyword)...) } return out, diags } diff --git a/compilers/openapi/internal/auth/auth.go b/compilers/openapi/internal/auth/auth.go index b24a3a5c..fdfc82af 100644 --- a/compilers/openapi/internal/auth/auth.go +++ b/compilers/openapi/internal/auth/auth.go @@ -132,7 +132,7 @@ 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) - diags = append(diags, applySchemeExtensions(c, &scheme, ss, decl)...) + diags = append(diags, applySchemeAnnotations(c, &scheme, ss, decl)...) // Distinct from preserveUnreadFields above it: that keeps the fields OpenAPI // defines for a securityScheme which this entry's own mechanism gives no // meaning to, while this keeps the keys OpenAPI defines for no securityScheme @@ -141,37 +141,43 @@ func lowerSecurityScheme(c lowering.Ctx, name string, ss *soa.SecurityScheme, annotation.UnknownKeysIn(&scheme.Unmodeled, ss, c.SrcIndex, decl)...) } -// applySchemeExtensions keeps the x-* of the securitySchemes entry and, for an -// oauth2 scheme, of the flows object and of each flow inside it. ir.OAuthFlow +// applySchemeAnnotations keeps what the securitySchemes entry and, for an oauth2 +// scheme, the flows object and each flow inside it declare that reaches no IR +// field: their x-*, and the keys OpenAPI defines for none of them. ir.OAuthFlow // carries an Unmodeled map of its own; the flows object does not lower to a node -// at all, so its own extensions are kept on the scheme under the keyword. +// at all, so its entries are kept on the scheme under the keyword. // // Only oauth2 reads inside `flows`: on any other type the whole node is kept // verbatim by preserveUnreadFields, extensions and all, and no ir.OAuthFlow was // lowered for a flow's own to land on. -func applySchemeExtensions(c lowering.Ctx, scheme *ir.AuthScheme, ss *soa.SecurityScheme, decl string) []ir.Diagnostic { +func applySchemeAnnotations(c lowering.Ctx, scheme *ir.AuthScheme, ss *soa.SecurityScheme, decl string) []ir.Diagnostic { ext, diags := annotation.ExtensionsFrom(ss.GetExtensions(), c.SrcIndex, decl) scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, ext) if scheme.Kind != ir.AuthKindOAuth2 { return diags } + flows := ss.GetFlows() flowsPtr := decl + ids.Ptr("flows") - flowsExt, flowsDiags := annotation.ExtensionsUnder(ss.GetFlows().GetExtensions(), c.SrcIndex, flowsPtr, "flows") + flowsExt, flowsDiags := annotation.ExtensionsUnder(flows.GetExtensions(), c.SrcIndex, flowsPtr, "flows") scheme.Unmodeled = annotation.MergeUnmodeled(scheme.Unmodeled, flowsExt) diags = append(diags, flowsDiags...) - return append(diags, applyFlowExtensions(c, scheme.Flows, ss.GetFlows(), flowsPtr)...) + diags = append(diags, annotation.UnknownKeysUnder(&scheme.Unmodeled, flows, c.SrcIndex, flowsPtr, "flows")...) + return append(diags, applyFlowAnnotations(c, scheme.Flows, flows, flowsPtr)...) } -// applyFlowExtensions writes each declared flow's own x-* onto the ir.OAuthFlow -// it lowered to. Both lists are one ordered reading of the same object — -// scheme.Flows came from oauthFlows, which walks presentFlows — so the i-th -// lowered flow is the i-th declared one and the two cannot differ in length. -func applyFlowExtensions(c lowering.Ctx, lowered []ir.OAuthFlow, flows *soa.OAuthFlows, flowsPtr string) []ir.Diagnostic { +// applyFlowAnnotations writes each declared flow's own x-* and undeclared keys +// onto the ir.OAuthFlow it lowered to. Both lists are one ordered reading of the +// same object — scheme.Flows came from oauthFlows, which walks presentFlows — so +// the i-th lowered flow is the i-th declared one and the two cannot differ in +// length. +func applyFlowAnnotations(c lowering.Ctx, lowered []ir.OAuthFlow, flows *soa.OAuthFlows, flowsPtr string) []ir.Diagnostic { var diags []ir.Diagnostic for i, f := range presentFlows(flows) { - ext, extDiags := annotation.ExtensionsFrom(f.src.GetExtensions(), c.SrcIndex, flowsPtr+ids.Ptr(f.keyword)) + fptr := flowsPtr + ids.Ptr(f.keyword) + ext, extDiags := annotation.ExtensionsFrom(f.src.GetExtensions(), c.SrcIndex, fptr) lowered[i].Unmodeled = annotation.MergeUnmodeled(lowered[i].Unmodeled, ext) diags = append(diags, extDiags...) + diags = append(diags, annotation.UnknownKeysIn(&lowered[i].Unmodeled, f.src, c.SrcIndex, fptr)...) } return diags } diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index b2dbdd40..677f1109 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/compilers/openapi/internal/operation/content.go @@ -355,9 +355,10 @@ func encodingConfig(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd // encodingUnmodeled keeps what an Encoding Object declares that nothing in the // IR holds: `allowReserved`, which ir.PartEncoding has no field for even though -// its neighbours style and explode do, and the object's own x-*. Neither had -// reached an IR field, an Unmodeled entry or a diagnostic, so two documents -// differing only in them compiled to one IR (GitHub #291). +// its neighbours style and explode do, the object's own x-*, and the keys the +// specification defines for no encoding at all. Neither allowReserved nor the +// extensions had reached an IR field, an Unmodeled entry or a diagnostic, so two +// documents differing only in them compiled to one IR (GitHub #291). // // Both ride on the owning ir.Content, since PartEncoding carries no Unmodeled // map, keyed under scope — "encoding/" or "itemEncoding". One content can @@ -378,7 +379,8 @@ func encodingUnmodeled(c lowering.Ctx, enc *soa.Encoding, encPtr, scope string) } ext, extDiags := schema.ExtensionsIn(c, enc.GetExtensions(), encPtr, scope) out = annotation.MergeUnmodeled(out, ext) - return out, append(diags, extDiags...) + diags = append(diags, extDiags...) + return out, append(diags, annotation.UnknownKeysUnder(&out, enc, c.SrcIndex, encPtr, scope)...) } // lowerHeaders lowers a header map into Properties in source order. Each @@ -593,12 +595,14 @@ func exampleList(c lowering.Ctx, single *yaml.Node, plural *sequencedmap.Map[str func appendPluralExample(c lowering.Ctx, out []ir.Example, re *soa.ReferencedExample, pointer, name string) ([]ir.Example, []ir.Diagnostic) { // The declaration pointer, not the entry's, is where an Example Object's own // keywords are written: a $ref entry holds none of them. ir.Example carries an - // Unmodeled map, so the object's x-* need no scope. + // Unmodeled map, so neither the object's x-* nor its undeclared keys need a + // scope. ex, decl := resolve.ObjectAt[soa.Example](c.RefScope(), re, pointer+ids.Ptr("examples", name)) if ex == nil { return out, nil } ext, diags := schema.ExtensionsOf(c, ex.GetExtensions(), decl) + diags = append(diags, annotation.UnknownKeysIn(&ext, ex, c.SrcIndex, decl)...) proto := ir.Example{ Name: name, Summary: ex.GetSummary(), diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 2e91a71a..583b964b 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -296,31 +296,38 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd diags = append(diags, cbDiags...) } op.Bindings = ir.OpBindings{HTTP: []ir.HTTPBinding{hb}} - diags = append(diags, applyOperationExtensions(c, &op, src, decl)...) - diags = append(diags, annotation.UnknownKeysIn(&op.Unmodeled, src, c.SrcIndex, decl)...) + diags = append(diags, applyOperationAnnotations(c, &op, src, decl)...) diags = append(diags, applyOperationServers(c, &op, src, decl)...) return op, extra, append(diags, checkOperationIDUnique(c, operationIDs, op, mount)...) } -// applyOperationExtensions keeps the operation's own x-* and those of the two -// objects beneath it that lower to no node of their own: its externalDocs, -// since ir.Link holds no Unmodeled map, and its Responses Object, whose -// extensions are the map's own rather than any one response's. +// applyOperationAnnotations keeps the operation's own x-* and undeclared keys, +// and those of the two objects beneath it that lower to no node of their own: +// its externalDocs, since ir.Link holds no Unmodeled map, and its Responses +// Object, whose extensions are the map's own rather than any one response's. +// +// The Responses Object contributes extensions but no census. Its key set is the +// status codes the document chooses, which the library models as a map, so an +// undeclared key there is read as one more response rather than reported as +// unknown — there is nothing for a census to say about it. // // It merges rather than assigns. The operation's map already carries whatever // its parameters or callbacks wrote by the time this runs, and an assignment // here would drop them — which is why the servers preservation used to have to // run after it. -func applyOperationExtensions(c lowering.Ctx, op *ir.Operation, src *soa.Operation, decl string) []ir.Diagnostic { +func applyOperationAnnotations(c lowering.Ctx, op *ir.Operation, src *soa.Operation, decl string) []ir.Diagnostic { + docsPtr := decl + ids.Ptr("externalDocs") ext, diags := annotation.ExtensionsAt(c.SrcIndex, annotation.ExtensionSite{Owner: decl, Ext: src.GetExtensions()}, - annotation.ExtensionSite{Scope: "externalDocs", Owner: decl + ids.Ptr("externalDocs"), + annotation.ExtensionSite{Scope: "externalDocs", Owner: docsPtr, Ext: src.GetExternalDocs().GetExtensions()}, annotation.ExtensionSite{Scope: "responses", Owner: decl + ids.Ptr("responses"), Ext: src.GetResponses().GetExtensions()}, ) op.Unmodeled = annotation.MergeUnmodeled(op.Unmodeled, ext) - return diags + diags = append(diags, annotation.UnknownKeysIn(&op.Unmodeled, src, c.SrcIndex, decl)...) + return append(diags, annotation.UnknownKeysUnder(&op.Unmodeled, + src.GetExternalDocs(), c.SrcIndex, docsPtr, "externalDocs")...) } // applyOperationServers preserves an operation's own `servers` verbatim under @@ -412,6 +419,25 @@ func fillOperationDocs(d *ir.Docs, src *soa.Operation) { // must reach both, and a second call beside the first is a second chance to // forget one on a route added later. That is exactly how the servers half came // to be missing on two of its three routes (GitHub #39). +// +// A path item takes no census, unlike every other object the compiler reads +// extensions from, and deliberately so. The library folds a key it does not +// recognize into the item's embedded operations map rather than recording it as +// undeclared, so GetUnknownProperties reports nothing and there is no census to +// read (speakeasy-api/openapi v1.24.0). Two consequences decide it: +// +// - The key is not lost in silence. Folding it produces a +// validation-type-mismatch at error severity naming the key at its own +// pointer, which is the losslessness property the census exists for; what is +// lost is the key's value, not the fact that it was written. +// - Recovering the value means reading the raw node against a path item's key +// vocabulary, and the only vocabulary this compiler owns is httpMethods, +// which is narrower than the library's — it has no `query`, the method +// OpenAPI 3.2 adds. A census over what httpMethods does not name would +// therefore report a valid 3.2 `query` operation as an undeclared key. +// +// That vocabulary is what GitHub #293 is about, so widening it here would settle +// that issue as a side effect of this one. Tracked separately in GitHub #377. func applyPathItem(c lowering.Ctx, op *ir.Operation, pi *soa.PathItem, declPtr string) []ir.Diagnostic { diags := applyPathServers(c, op, pi, declPtr) ext, extDiags := schema.ExtensionsIn(c, pi.GetExtensions(), declPtr, "pathItem") @@ -522,6 +548,14 @@ func lowerResponse(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde // The links entry carries ReasonNoIRHome and no diagnostic, as it always has: // nothing is degraded, the map is in the document, and the gap is one the IR can // close by growing a links field. +// +// A Link Object inside that map gets no entry and no census of its own, which is +// the decision already recorded for its extensions. This compiler lowers no Link +// Object anywhere: a response's links survive only as the verbatim node above, +// and a components/links entry nothing references is dropped whole, so a keyed +// entry at one of the two positions would be the only trace of a construct the +// IR does not model — while duplicating, for the response position alone, a +// value the node above already carries. func preserveResponseExtras(c lowering.Ctx, p *ir.Unmodeled, r *soa.Response, rptr string) []ir.Diagnostic { _, diags := schema.PreserveNode(c, p, "openapi:links", annotation.RawChildNode(r.GetRootNode(), "links"), ir.ReasonNoIRHome, rptr+ids.Ptr("links")) diff --git a/compilers/openapi/meta.go b/compilers/openapi/meta.go index f8874e6d..ae6aafdb 100644 --- a/compilers/openapi/meta.go +++ b/compilers/openapi/meta.go @@ -109,7 +109,8 @@ func tagExtensions(c lowering.Ctx) []annotation.ExtensionSite { // documentUnknownKeys collects the keys the OpenAPI model names no field for // from every object around the document metadata that lowers to no node of its // own: the document root, the info block and the contact and license inside it, -// the root externalDocs, and each declared tag. +// the root externalDocs, the components object, and each declared tag with its +// own externalDocs. // // ir.Document is the nearest node with an Unmodeled map for all of them, so each // object's keys are scoped by the source path they were written at. One unscoped @@ -137,6 +138,11 @@ type unknownSite struct { // root's keys take no scope, since ir.Document stands for the OpenAPI Object // itself; the rest are keyed by the path from it down to the object that wrote // them. +// +// The components object is one of them rather than a map with nothing to say: +// its own key set is the fixed list of component kinds, which the library models +// as named fields and takes a census over, and only the map *under* each of +// those keys is the document's to name. func rootUnknownSites(c lowering.Ctx) []unknownSite { info := c.Doc.GetInfo() infoPtr := ids.Ptr("info") @@ -146,11 +152,13 @@ func rootUnknownSites(c lowering.Ctx) []unknownSite { {"info/contact", infoPtr + ids.Ptr("contact"), info.GetContact()}, {"info/license", infoPtr + ids.Ptr("license"), info.GetLicense()}, {"externalDocs", ids.Ptr("externalDocs"), c.Doc.GetExternalDocs()}, + {"components", ids.Ptr("components"), c.Doc.GetComponents()}, } } -// tagUnknownSites returns one census site per declared tag, since ir.TagDef -// holds no Unmodeled map for a tag's own keys to land on. +// tagUnknownSites returns the census sites each declared tag contributes: its +// own, and its externalDocs object's. Neither ir.TagDef nor ir.Link holds an +// Unmodeled map, so both ride on the document. // // Scoped by index rather than name, which is the pointer a tag is written at. A // name would read better, but OpenAPI's requirement that tag names be unique is @@ -158,10 +166,13 @@ func rootUnknownSites(c lowering.Ctx) []unknownSite { // alike would silently leave one entry. func tagUnknownSites(c lowering.Ctx) []unknownSite { tags := c.Doc.GetTags() - out := make([]unknownSite, 0, len(tags)) + out := make([]unknownSite, 0, 2*len(tags)) for i, t := range tags { index := strconv.Itoa(i) - out = append(out, unknownSite{"tags/" + index, ids.Ptr("tags", index), t}) + ptr := ids.Ptr("tags", index) + out = append(out, + unknownSite{"tags/" + index, ptr, t}, + unknownSite{"tags/" + index + "/externalDocs", ptr + ids.Ptr("externalDocs"), t.GetExternalDocs()}) } return out } diff --git a/compilers/openapi/unknownkeys_test.go b/compilers/openapi/unknownkeys_test.go index 784316ba..34f5269c 100644 --- a/compilers/openapi/unknownkeys_test.go +++ b/compilers/openapi/unknownkeys_test.go @@ -56,15 +56,31 @@ func TestUnknownKeys_KeptAtEveryObject(t *testing.T) { {"server", "openapi:host", `"SERVER"`, "doc.Servers[0].Unmodeled"}, {"server variable", "openapi:example", `"SERVERVARIABLE"`, "doc.Servers[0].Variables[0].Unmodeled"}, {"tag", "openapi:tags/0/color", `"TAG"`, "doc.Unmodeled"}, + {"components", "openapi:components/definitions", `"COMPONENTS"`, "doc.Unmodeled"}, + {"tag externalDocs", "openapi:tags/0/externalDocs/title", `"TAGEXTERNALDOCS"`, "doc.Unmodeled"}, {"operation", "openapi:operationid", `"OPERATION"`, ".Unmodeled"}, + // The carrier is named down to the operation rather than left at + // ".Unmodeled": the document writes this exact key too, and the row is only + // evidence of the operation's if it cannot match the document's. + {"operation externalDocs", "openapi:externalDocs/title", `"OPERATIONEXTERNALDOCS"`, + "Operations[0].Unmodeled"}, {"parameter", "openapi:collectionFormat", `"PARAMETER"`, ".Params[0].Unmodeled"}, + {"example", "openapi:name", `"EXAMPLE"`, ".Params[0].Examples[0].Unmodeled"}, {"request body", "openapi:schema", `"REQUESTBODY"`, ".Request.Unmodeled"}, {"media type", "openapi:format", `"MEDIATYPE"`, ".Contents[0].Unmodeled"}, + // On the content rather than on the part: ir.PartEncoding holds no + // Unmodeled map, and one content can carry an entry per part, so the part + // name is what tells two encodings' keys apart on that one map. + {"encoding", "openapi:encoding/part/contentEncoding", `"ENCODING"`, ".Contents[1].Unmodeled"}, {"response", "openapi:status", `"RESPONSE"`, ".Responses[0].Unmodeled"}, {"error response", "openapi:status", `"ERRORRESPONSE"`, ".Errors[0].Unmodeled"}, {"header", "openapi:in", `"HEADER"`, ".Headers[0].Unmodeled"}, {"security scheme", "openapi:tokenUrl", `"SECURITYSCHEME"`, "doc.Auth[auth/openapi/components/securitySchemes/k].Unmodeled"}, + {"oauth flows", "openapi:flows/application", `"OAUTHFLOWS"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Unmodeled"}, + {"oauth flow", "openapi:scope", `"OAUTHFLOW"`, + "doc.Auth[auth/openapi/components/securitySchemes/o].Flows[0].Unmodeled"}, {"schema", "openapi:additionalItems", `"SCHEMA"`, "doc.Types[t/openapi/components/schemas/S].Unmodeled"}, // The property's schema reduced to a shared primitive, so it owns no node @@ -147,6 +163,61 @@ components: } } +// TestUnknownKeys_SchemaSubObjects covers the three objects that hang off a +// schema — its xml, its discriminator and its externalDocs — which the fixture +// above deliberately leaves out. +// +// It leaves them out because the OpenAPI dialect meta-schema closes all three to +// anything but an x- key, so the library reports a validation error on each and +// harness.Check returns at the first one, before the oracles that fixture exists +// to reach. The keys are kept and announced all the same, which is what this +// asserts: an invalid document is still not a document whose keys may vanish. +// +// Graded as an OpenAPI object's keys rather than as schema keywords, at warning +// rather than info: JSON Schema's rule that an unrecognized keyword is legal +// governs the schema, and these three are OpenAPI objects the schema vocabulary +// says nothing about. All three ride on the schema's own map, since none of +// ir.XMLHints, ir.Discriminator or ir.Link holds one, and the keyword each was +// written under is what keeps them apart there. +func TestUnknownKeys_SchemaSubObjects(t *testing.T) { + t.Parallel() + doc, diags := compileAnnotationSpec(t, "schema-sub-objects", `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: {} +components: + schemas: + S: + type: object + xml: {name: s, attribute2: XML} + discriminator: {propertyName: k, mapping2: DISCRIMINATOR} + externalDocs: {url: 'https://d.example', title: SCHEMAEXTERNALDOCS} + properties: {k: {type: string}} +`) + schema, ok := doc.Types[ir.TypeID("t/openapi/components/schemas/S")] + require.True(t, ok) + + // assert rather than require on the lookup, so a keyword missing from the map + // reports itself and leaves the other two still checked. The three share one + // reader, and a row that never runs is no evidence about the keyword it names. + for _, tc := range []struct{ keyword, key, want string }{ + {"xml", "openapi:xml/attribute2", `"XML"`}, + {"discriminator", "openapi:discriminator/mapping2", `"DISCRIMINATOR"`}, + {"externalDocs", "openapi:externalDocs/title", `"SCHEMAEXTERNALDOCS"`}, + } { + entry, found := schema.Common().Unmodeled[tc.key] + if !assert.True(t, found, "schema %s drops %s: it is nowhere on the schema", tc.keyword, tc.key) { + continue + } + assert.Equal(t, tc.want, string(entry.Value), "%s key keeps what the source wrote", tc.keyword) + assert.Equal(t, ir.ReasonOutOfScope, entry.Reason, "%s key reason", tc.keyword) + at := "/components/schemas/S/" + tc.keyword + "/" + tc.key[len("openapi:"+tc.keyword+"/"):] + assert.Equal(t, at, entry.Provenance.Pointer, "%s key provenance", tc.keyword) + assert.Equal(t, []ir.Severity{ir.SeverityWarning}, + diagsAt(diags, "openapi/unknown-object-key", at), + "%s key is announced as an object's, not as a schema keyword's", tc.keyword) + } +} + // requireNoErrorDiagnostics fails the test on the first error-severity // diagnostic, naming it. func requireNoErrorDiagnostics(t *testing.T, diags []ir.Diagnostic) { diff --git a/testdata/openapi/unknown_keys.yaml b/testdata/openapi/unknown_keys.yaml index 7a49ac00..ba1668b1 100644 --- a/testdata/openapi/unknown_keys.yaml +++ b/testdata/openapi/unknown_keys.yaml @@ -3,11 +3,24 @@ # at the wrong carrier cannot pass for the right one. # # Every key here is one real documents carry rather than invented nonsense: a -# Swagger 2.0 field with no OpenAPI 3 equivalent (basePath, host, a body -# parameter's schema), a keyword from an older JSON Schema draft (additionalItems, -# divisibleBy), a field belonging to a neighbouring object (a flow's tokenUrl on -# the scheme, a parameter's `in` on a header), or operationId with the case wrong. -# None of them reached an IR field, an Unmodeled entry or a diagnostic. +# Swagger 2.0 field with no OpenAPI 3 equivalent (basePath, host, definitions, a +# body parameter's schema, a flow named `application`), a keyword from an older +# JSON Schema draft (additionalItems, divisibleBy), a field belonging to a +# neighbouring object (a flow's tokenUrl on the scheme, a parameter's `in` on a +# header, a documentation page's `title` on an externalDocs), or a field with the +# case or the number wrong (operationid, scope). None of them reached an IR field, +# an Unmodeled entry or a diagnostic. +# +# `title` is written on all three externalDocs objects and `scope` on the one +# flow, so the entries a document can write more than once are here in more than +# one copy: an unscoped key would leave a single entry and the survivor would +# depend on which lowering ran last. +# +# A schema's own xml, discriminator and externalDocs are censused too but are not +# here. The OpenAPI dialect meta-schema closes those three to undeclared keys, so +# one draws a library validation error, and an error diagnostic stops +# harness.Check before the oracles this fixture exists to reach — see +# TestUnknownKeys_SchemaSubObjects for where they are asserted instead. openapi: 3.1.0 basePath: ROOT info: @@ -23,15 +36,23 @@ servers: variables: region: {default: us, example: SERVERVARIABLE} tags: - - {name: t1, color: TAG} + - name: t1 + color: TAG + externalDocs: {url: 'https://t.example', title: TAGEXTERNALDOCS} paths: /widgets: get: operationId: listWidgets tags: [t1] operationid: OPERATION + externalDocs: {url: 'https://o.example', title: OPERATIONEXTERNALDOCS} parameters: - - {name: shape, in: query, schema: {type: string}, collectionFormat: PARAMETER} + - name: shape + in: query + schema: {type: string} + collectionFormat: PARAMETER + examples: + round: {value: circle, name: EXAMPLE} requestBody: required: true schema: REQUESTBODY @@ -39,6 +60,10 @@ paths: application/json: schema: {$ref: '#/components/schemas/S'} format: MEDIATYPE + multipart/form-data: + schema: {type: object, properties: {part: {type: string}}} + encoding: + part: {contentType: text/plain, contentEncoding: ENCODING} responses: "200": description: ok @@ -49,8 +74,17 @@ paths: description: gone status: ERRORRESPONSE components: + definitions: COMPONENTS securitySchemes: k: {type: apiKey, in: header, name: X-Key, tokenUrl: SECURITYSCHEME} + o: + type: oauth2 + flows: + application: OAUTHFLOWS + implicit: + authorizationUrl: 'https://auth.example' + scopes: {} + scope: OAUTHFLOW schemas: S: type: object