fix(compilers/openapi): keep the declarations that were dropped

compilers/openapi/internal/annotation/annotation.go

@@ -610,16 +610,15 @@ type Set struct {
 	Unmodeled  ir.Unmodeled
 }
 
-// Read reads every site-local annotation at st.
+// Read reads every site-local annotation at st. reads32 reports that the
+// document speaks OpenAPI 3.2, whose XML object adds a `nodeType` the parser's
+// model has no field for, read here off the raw node (GitHub #615). It is a
+// parameter because this package may not import the lowering or the loader.
 //
-// This is the single call site the decomposition exists for. Not because it
-// merges duplicate readers — the docs readers are genuinely distinct and stay
-// distinct — but because the site-versus-referent choice is made here once
-// instead of at each position that attaches annotations. Three attachment points
-// previously made it separately and disagreed: one passed a referent, one passed
-// nil because a declaration has none, and one passed nil because it never
-// resolved the referent it had.
-func Read(st Site, pointer jsontext.Pointer, locate Locator) (Set, []ir.Diagnostic) {
+// This is the one place the site-versus-referent choice is made, rather than
+// at each position that attaches annotations: three attachment points used to
+// make it separately and disagreed.
+func Read(st Site, pointer jsontext.Pointer, locate Locator, reads32 bool) (Set, []ir.Diagnostic) {
 	var out Set
 
 	referent := st.Referent
@@ -630,12 +629,15 @@ func Read(st Site, pointer jsontext.Pointer, locate Locator) (Set, []ir.Diagnost
 	FillCarrierDocs(&out.Docs, st.Node, referent)
 	out.Deprecated = EffectiveDeprecated(st.Node, referent)
 	out.XML = XMLHints(st.Node.GetXML())
+	if reads32 {
+		applyNodeType(out.XML, st.Node)
+	}
 
 	examples, exDiags := schemaExamplesAt(st.Node, pointer, locate)
 	out.Examples = examples
 
 	ext, extDiags := ExtensionsFrom(st.Node.GetExtensions(), locate, pointer)
-	sub, subDiags := subObjectKeys(st.Node, pointer, locate)
+	sub, subDiags := subObjectKeys(st.Node, pointer, locate, reads32)
 	kept, keptDiags := unmodeledAt(st.Node, pointer, locate)
 
 	diags := make([]ir.Diagnostic, 0, len(exDiags)+len(extDiags)+len(subDiags)+len(keptDiags))
@@ -648,27 +650,43 @@ func Read(st Site, pointer jsontext.Pointer, locate Locator) (Set, []ir.Diagnost
 	return out, diags
 }
 
+// applyNodeType fills the 3.2 nodeType an XML object declares, read off the raw
+// node because the parser's XML model has no field for it (GitHub #615).
+// ir.XMLHints.NodeType already exists and its GoDoc already names the version.
+//
+// A schema declaring no xml object has no XMLHints to fill — nodeType is written
+// inside the xml object, so there is no position it could have been declared at.
+// A declared nodeType wins over the attribute flag: 3.2 replaces `attribute:
+// true` with `nodeType: attribute`, so a document writing both has stated the
+// newer field, and the reader that took it last makes that the value.
+func applyNodeType(h *ir.XMLHints, s *oas3.Schema) {
+	if h == nil {
+		return
+	}
+	if node := RawChildNode(RawPropertyNode(s, "xml"), "nodeType"); node != nil {
+		h.NodeType = node.Value
+	}
+}
+
 // subObjectKeys collects what a schema's xml, discriminator and externalDocs
 // declare that reaches no IR field: the x-* they carry and the keys the
-// specification defines for none of them.
-//
-// None of ir.XMLHints, ir.Discriminator or ir.Link holds an Unmodeled map, so
-// the entries ride on the node the schema lowers to, and the keyword each was
-// written under keeps the three objects' entries apart.
+// specification defines for none of them. None of those IR types holds an
+// Unmodeled map, so the entries ride on the schema's node, keyed apart by the
+// keyword each was written under.
 //
-// The census is graded as an OpenAPI object's, not as a schema keyword's: JSON
-// Schema's rule that an unrecognized keyword is legal governs the schema
-// itself, and these three are OpenAPI objects its vocabulary says nothing
-// about.
-func subObjectKeys(s *oas3.Schema, pointer jsontext.Pointer, locate Locator) (ir.Unmodeled, []ir.Diagnostic) {
+// The census is graded as an OpenAPI object's, not a schema keyword's. reads32
+// names the key a 3.2 document defines that Read already takes raw, the XML
+// object's `nodeType`, so the census skips it there and still warns below 3.2.
+func subObjectKeys(s *oas3.Schema, pointer jsontext.Pointer, locate Locator, reads32 bool) (ir.Unmodeled, []ir.Diagnostic) {
 	subs := []struct {
 		keyword string
 		obj     any
 		ext     *extensions.Extensions
+		decided []string
 	}{
-		{"xml", s.GetXML(), s.GetXML().GetExtensions()},
-		{"discriminator", s.GetDiscriminator(), s.GetDiscriminator().GetExtensions()},
-		{"externalDocs", s.GetExternalDocs(), s.GetExternalDocs().GetExtensions()},
+		{"xml", s.GetXML(), s.GetXML().GetExtensions(), nodeTypeKey(reads32)},
+		{"discriminator", s.GetDiscriminator(), s.GetDiscriminator().GetExtensions(), nil},
+		{"externalDocs", s.GetExternalDocs(), s.GetExternalDocs().GetExtensions(), nil},
 	}
 	var out ir.Unmodeled
 	var diags []ir.Diagnostic
@@ -677,11 +695,21 @@ func subObjectKeys(s *oas3.Schema, pointer jsontext.Pointer, locate Locator) (ir
 		ext, extDiags := ExtensionsUnder(sub.ext, locate, owner, sub.keyword)
 		out = MergeUnmodeled(out, ext)
 		diags = append(diags, extDiags...)
-		diags = append(diags, UnknownKeysUnder(&out, sub.obj, locate, owner, sub.keyword)...)
+		diags = append(diags, UnknownKeysDecided(&out, sub.obj, locate, owner, sub.keyword, sub.decided)...)
 	}
 	return out, diags
 }
 
+// nodeTypeKey names the XML object's 3.2 `nodeType` for the census, and nothing
+// below 3.2: the key is a misspelling there rather than a field the dialect
+// added, and the warning is what says so.
+func nodeTypeKey(reads32 bool) []string {
+	if !reads32 {
+		return nil
+	}
+	return []string{"nodeType"}
+}
+
 // unmodeledAt collects every keyword a site declares that the IR keeps verbatim
 // instead of modelling, each under the reason that says which of those it is
 // (§12): validation logic the IR draws a boundary against (§4.7), and JSON

compilers/openapi/internal/annotation/annotation_internal_test.go

@@ -24,7 +24,7 @@ func TestAnnotations_SiteOverridesReferent(t *testing.T) {
 	ref := &oas3.Schema{Description: new("SiteDesc")}
 	tgt := &oas3.Schema{Description: new("TargetDesc"), Deprecated: new(true)}
 
-	got, diags := Read(Site{Kind: Reference, Node: ref, Referent: tgt}, "/p", sourced(0))
+	got, diags := Read(Site{Kind: Reference, Node: ref, Referent: tgt}, "/p", sourced(0), false)
 
 	assert.Empty(t, diags)
 	assert.Equal(t, "SiteDesc", got.Docs.Description, "the site's own description wins")
@@ -40,7 +40,7 @@ func TestAnnotations_DeclarationIgnoresAnyReferent(t *testing.T) {
 	node := &oas3.Schema{Description: new("OwnDesc")}
 	stray := &oas3.Schema{Title: new("StraySummary"), Deprecated: new(true)}
 
-	got, _ := Read(Site{Kind: Declaration, Node: node, Referent: stray}, "/p", sourced(0))
+	got, _ := Read(Site{Kind: Declaration, Node: node, Referent: stray}, "/p", sourced(0), false)
 
 	assert.Equal(t, "OwnDesc", got.Docs.Description)
 	assert.Empty(t, got.Docs.Summary, "a declaration inherits nothing, whatever it is handed")
@@ -54,7 +54,7 @@ func TestAnnotations_ReadsEverySiteLocalAspect(t *testing.T) {
 		XML:         &oas3.XML{Name: new("Q")},
 		Example:     openapitest.YAMLNode(t, "hello"),
 	}
-	got, diags := Read(Site{Kind: Declaration, Node: node}, "/components/schemas/S", sourced(0))
+	got, diags := Read(Site{Kind: Declaration, Node: node}, "/components/schemas/S", sourced(0), false)
 
 	assert.Equal(t, "D", got.Docs.Description)
 	require.NotNil(t, got.XML)
@@ -208,3 +208,51 @@ func TestJSONObject_PropagatesAQuoteFailure(t *testing.T) {
 	assert.Nil(t, got)
 	assert.Contains(t, err.Error(), "invalid UTF-8")
 }
+
+// TestRead_NodeTypeIsReadFromTheRawXMLObject pins the one key of a 3.2
+// document this reader takes off the raw node: the library's XML model has no
+// field for `nodeType`, so a reader that consulted the model alone would drop
+// a declaration the document makes (GitHub #615). The census is told the key
+// was read, which is why the same fixture read as 3.1 warns below instead.
+func TestRead_NodeTypeIsReadFromTheRawXMLObject(t *testing.T) {
+	t.Parallel()
+	s := schemaFromYAML(t, "type: string\nxml:\n  name: q\n  nodeType: attribute\n")
+
+	got, diags := Read(Site{Kind: Declaration, Node: s}, "/components/schemas/S", sourced(0), true)
+
+	require.NotNil(t, got.XML)
+	assert.Equal(t, "q", got.XML.Name, "the modelled fields still come from the model")
+	assert.Equal(t, "attribute", got.XML.NodeType)
+	assert.Empty(t, diags, "a key the dialect defines is not announced as undefined")
+}
+
+// TestRead_NodeTypeBelow32IsCensused is the other side of the gate: below 3.2
+// the key is a misspelling rather than a field the dialect added, so nothing is
+// read into the IR and the census owes the warning.
+func TestRead_NodeTypeBelow32IsCensused(t *testing.T) {
+	t.Parallel()
+	s := schemaFromYAML(t, "type: string\nxml:\n  name: q\n  nodeType: attribute\n")
+
+	got, diags := Read(Site{Kind: Declaration, Node: s}, "/components/schemas/S", sourced(0), false)
+
+	require.NotNil(t, got.XML)
+	assert.Empty(t, got.XML.NodeType, "3.1 defines no nodeType, so no field is filled")
+	require.Len(t, diags, 1)
+	assert.Equal(t, diag.UnknownObjectKey, diags[0].Code)
+	assert.Equal(t, ir.SeverityWarning, diags[0].Severity)
+	assert.Equal(t, jsontext.Pointer("/components/schemas/S/xml/nodeType"), diags[0].Provenance.Pointer,
+		"the warning names the key's own position")
+}
+
+// TestRead_NodeTypeWithNoXMLObjectHasNothingToFill covers applyNodeType's first
+// branch. `nodeType` is written inside the xml object, so a schema declaring no
+// xml object has no position the key could sit at — and no hints to carry it.
+func TestRead_NodeTypeWithNoXMLObjectHasNothingToFill(t *testing.T) {
+	t.Parallel()
+	s := schemaFromYAML(t, "type: string\n")
+
+	got, diags := Read(Site{Kind: Declaration, Node: s}, "/components/schemas/S", sourced(0), true)
+
+	assert.Nil(t, got.XML)
+	assert.Empty(t, diags)
+}

compilers/openapi/internal/annotation/unknown.go

@@ -114,8 +114,32 @@ func UnknownKeysIn(p *ir.Unmodeled, model any, locate Locator, owner jsontext.Po
 // 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, locate Locator, owner jsontext.Pointer, scope string) []ir.Diagnostic {
+	return UnknownKeysDecided(p, model, locate, owner, scope, nil)
+}
+
+// UnknownKeysDecided is UnknownKeysUnder for an object one of whose keys a
+// reader has already read raw: `decided` names those keys, and the census leaves
+// them alone rather than reporting a key the document does define as undefined.
+//
+// The parser's model has no field for the keys OpenAPI 3.2 added, such as a
+// Response Object's `summary`, so each is read off the raw node (GitHub #615).
+// `decided` must be empty below 3.2, where the same key is a misspelling and
+// the warning is owed.
+func UnknownKeysDecided(p *ir.Unmodeled, model any, locate Locator, owner jsontext.Pointer, scope string, decided []string) []ir.Diagnostic {
 	keys, root := undeclaredKeys(model)
-	return UnknownKeysNamed(p, keys, root, locate, owner, scope)
+	return census(p, keys, root, locate, owner, scope, objectKeyClass(decided))
+}
+
+// objectKeyClass grades a key the OpenAPI object it is written on does not
+// define, with the keys a reader has already taken raw left out.
+func objectKeyClass(decided []string) keyClass {
+	return keyClass{
+		code:     diag.UnknownObjectKey,
+		severity: ir.SeverityWarning,
+		skip:     decided,
+		message: "key %q is not defined by the OpenAPI object it is written on and is not an " +
+			"x- extension; kept verbatim under Unmodeled",
+	}
 }
 
 // UnknownKeysNamed is UnknownKeysUnder for an object whose model keeps no
@@ -131,12 +155,7 @@ func UnknownKeysUnder(p *ir.Unmodeled, model any, locate Locator, owner jsontext
 func UnknownKeysNamed(p *ir.Unmodeled, keys []string, root *yaml.Node,
 	locate Locator, owner jsontext.Pointer, scope string,
 ) []ir.Diagnostic {
-	return census(p, keys, root, locate, 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",
-	})
+	return census(p, keys, root, locate, owner, scope, objectKeyClass(nil))
 }
 
 // keyClass is how a key the model does not name is graded: which diagnostic

compilers/openapi/internal/annotation/unknown_internal_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 	yaml "gopkg.in/yaml.v3"
 
+	"github.com/dexpace/morphic/compilers/openapi/internal/diag"
 	"github.com/dexpace/morphic/ir"
 )
 
@@ -286,6 +287,27 @@ func TestUnknownKeysIn_ModelWithNoCensusRecordsNothing(t *testing.T) {
 	}
 }
 
+// TestUnknownKeysNamed_DelegatesTheSameGrading covers the object census's other
+// entry point: an object whose model keeps no key list of its own — a Path
+// Item's leftovers, in practice — so the caller names the keys the model does
+// not define and this reader grades them exactly as the modelled path does.
+func TestUnknownKeysNamed_DelegatesTheSameGrading(t *testing.T) {
+	t.Parallel()
+	root := parsedMapping(t, "get: {}\nnotAKey: 3\n")
+
+	var got ir.Unmodeled
+	diags := UnknownKeysNamed(&got, []string{"notAKey"}, root, sourced(0), "/paths/~1p", "")
+
+	require.Len(t, got, 1, "got %v", got)
+	entry := got["openapi:notAKey"]
+	assert.Equal(t, ir.ReasonOutOfScope, entry.Reason)
+	assert.Equal(t, ir.RawValue("3"), entry.Value)
+	assert.Equal(t, ir.Provenance{Pointer: "/paths/~1p/notAKey"}, entry.Provenance)
+	require.Len(t, diags, 1)
+	assert.Equal(t, diag.UnknownObjectKey, diags[0].Code)
+	assert.Equal(t, ir.SeverityWarning, diags[0].Severity)
+}
+
 // 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.

compilers/openapi/internal/auth/auth.go

@@ -59,6 +59,10 @@ func LowerSecuritySchemes(c lowering.Ctx) (map[ir.AuthID]ir.AuthScheme, []ir.Dia
 		if !ok {
 			continue
 		}
+		// A securitySchemes entry written as a Reference Object keeps the summary
+		// and description written beside its $ref, which describe this entry rather
+		// than the declaration it names (GitHub #610).
+		scheme.Docs = resolve.RefDocs(rs, scheme.Docs)
 		out[ids.Auth(name)] = scheme
 	}
 	if len(out) == 0 {

compilers/openapi/internal/auth/auth_test.go

@@ -1192,3 +1192,34 @@ func loaderErrors(diags []ir.Diagnostic) []string {
 	}
 	return out
 }
+
+// TestLowerSecuritySchemes_RefSiteDocsOverrideTheDeclaration pins the auth half
+// of GitHub #610: a securitySchemes entry written as a Reference Object keeps
+// the description written beside its $ref, and the declaration it names keeps
+// its own — the fold is per entry, never a mutation of the target.
+func TestLowerSecuritySchemes_RefSiteDocsOverrideTheDeclaration(t *testing.T) {
+	t.Parallel()
+	doc, _, diags := serviceSpec(t, `openapi: 3.1.0
+info: {title: T, version: "1"}
+paths: {}
+components:
+  securitySchemes:
+    BaseKey:
+      description: declared
+      type: apiKey
+      in: header
+      name: X-Key
+    ApiKey:
+      $ref: '#/components/securitySchemes/BaseKey'
+      description: the use site
+`)
+	require.Len(t, doc.Auth, 2)
+	byName := map[string]string{}
+	for _, scheme := range doc.Auth {
+		byName[scheme.Name.Source] = scheme.Docs.Description
+	}
+	assert.Equal(t, "the use site", byName["ApiKey"])
+	assert.Equal(t, "declared", byName["BaseKey"],
+		"the aliasing entry's siblings do not reach the declaration")
+	require.Empty(t, messagesAt(diags, diag.UnresolvedRef))
+}