fix(compilers/openapi): keep undeclared keys on nested objects

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

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")
 }

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,51 @@ func Read(st Site, pointer string, srcIndex int) (Set, []ir.Diagnostic) {
 	out.Examples = examples
 
 	ext, extDiags := ExtensionsFrom(st.Node.GetExtensions(), srcIndex, pointer)
+	sub, subDiags := subObjectKeys(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
+}
+
+// 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(), 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 {
+		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
 }
 

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.