fix(compilers/openapi): place or keep every union's discriminator

compilers/openapi/conformance_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/dexpace/morphic/compilers/openapi/internal/openapitest"
 	"github.com/dexpace/morphic/ir"
 	"github.com/dexpace/morphic/ir/irtest"
+	"github.com/dexpace/morphic/pass"
 )
 
 // conformanceDir is the corpus of one minimal spec per capability row of
@@ -163,6 +164,8 @@ func conformanceCases() []conformanceCase {
 		{"discriminator-default-mapping", assertDiscriminatorDefaultMapping, []string{"tagged-unions"}},
 		{"discriminator-transitive", assertDiscriminatorTransitive, []string{"tagged-unions", "inheritance"}},
 		{"discriminator-alias-mapping", assertDiscriminatorAliasMapping, []string{"tagged-unions"}},
+		{"discriminated-union-beside-model", assertDiscriminatedUnionBesideModel, []string{"tagged-unions", "inheritance"}},
+		{"nullable-union-discriminator", assertNullableUnionDiscriminator, []string{"tagged-unions", "optionality-vs-nullability"}},
 		{"unhomed-keywords", assertUnhomedKeywords, nil},
 		{"union-beside-ref", assertUnionBesideRef, []string{"untagged-unions"}},
 		{"codeclared-keywords", assertCoDeclaredKeywords, []string{"intersection", "literal-types", "enums-string"}},
@@ -1016,6 +1019,47 @@ func assertDiscriminatorDefaultMapping(t *testing.T, doc *ir.Document, diags []i
 		"and it is read separately from the mapping")
 }
 
+// assertDiscriminatedUnionBesideModel pins where a discriminator goes when its
+// union is kept verbatim beside a model. Pet's targets are not subtypes of it,
+// so pass.Validate would reject the discriminator on the model; it moves whole
+// to Unmodeled beside the union. Shape's targets are true allOf subtypes, so
+// the model keeps it, and the document carries no validation error.
+func assertDiscriminatedUnionBesideModel(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
+	assert.Empty(t, pass.Validate(doc))
+
+	pet, ok := doc.Types[namedID("Pet")].(*ir.Model)
+	require.True(t, ok)
+	assert.Nil(t, pet.Discriminator, "its targets are not subtypes, so the model does not carry it")
+	entry, ok := pet.Unmodeled["openapi:discriminator"]
+	require.True(t, ok, "it is kept verbatim instead")
+	assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason)
+	assert.Contains(t, pet.Unmodeled, "openapi:oneOf", "beside the union it routes")
+	assert.True(t, openapitest.HasDiagCodeAt(diags, diag.DegradedConstruct, "/components/schemas/Pet"))
+
+	shape, ok := doc.Types[namedID("Shape")].(*ir.Model)
+	require.True(t, ok)
+	require.NotNil(t, shape.Discriminator, "every target is a subtype, so it stays")
+	assert.Equal(t, namedID("Circle"), shape.Discriminator.Mapping["circle"])
+	assert.NotContains(t, shape.Unmodeled, "openapi:discriminator")
+}
+
+// assertNullableUnionDiscriminator pins that a discriminator beside a oneOf or
+// anyOf that collapses to nullable X is kept verbatim, with a diagnostic,
+// where the collapse leaves no node with a field for it.
+func assertNullableUnionDiscriminator(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
+	maybe, ok := doc.Types[namedID("MaybeCat")].(*ir.Scalar)
+	require.True(t, ok, "the component is an alias over the nullable reference")
+	assert.Equal(t, ir.TypeRef{Target: namedID("Cat"), Nullable: true}, *maybe.Base)
+	assert.Contains(t, maybe.Unmodeled, "openapi:discriminator")
+	assert.True(t, openapitest.HasDiagCodeAt(diags, diag.DegradedConstruct, "/components/schemas/MaybeCat"))
+
+	owner, ok := doc.Types[namedID("Owner")].(*ir.Model)
+	require.True(t, ok)
+	require.Len(t, owner.Properties, 1)
+	assert.Equal(t, ir.TypeRef{Target: namedID("Cat"), Nullable: true}, owner.Properties[0].Type)
+	assert.Contains(t, owner.Properties[0].Unmodeled, "openapi:discriminator", "the property carries it")
+}
+
 func assertAnyOfUntagged(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	u, ok := doc.Types[namedID("StringOrNumber")].(*ir.Union)
 	require.True(t, ok)

compilers/openapi/internal/schema/collapse_discriminator_test.go

@@ -0,0 +1,228 @@
+package schema_test
+
+import (
+	"encoding/json/v2"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/dexpace/morphic/compilers/openapi/internal/diag"
+	"github.com/dexpace/morphic/compilers/openapi/internal/openapitest"
+	"github.com/dexpace/morphic/ir"
+	"github.com/dexpace/morphic/pass"
+)
+
+const nullableCat = "oneOf: [{$ref: '#/components/schemas/Cat'}, {type: 'null'}]"
+
+// collapseSpec declares a nullable Cat at every position a schema can sit at
+// with a discriminator beside it, plus a parameter and a response body.
+const collapseSpec = `openapi: 3.2.0
+info: {title: T, version: "1"}
+paths:
+  /p:
+    get:
+      parameters:
+        - name: q
+          in: query
+          schema:
+            ` + nullableCat + `
+            discriminator: {propertyName: kind}
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema:
+                ` + nullableCat + `
+                discriminator: {propertyName: kind}
+components:
+  schemas:
+    Cat: {type: object, properties: {kind: {type: string}}}
+    W:
+      ` + nullableCat + `
+      discriminator: {propertyName: kind, defaultMapping: '#/components/schemas/Cat', mapping: {cat: '#/components/schemas/Cat'}}
+    Holder:
+      type: object
+      properties:
+        w:
+          anyOf: [{$ref: '#/components/schemas/Cat'}, {type: 'null'}]
+          discriminator: {propertyName: kind}
+    L:
+      type: array
+      items:
+        ` + nullableCat + `
+        discriminator: {propertyName: kind}
+    M:
+      type: object
+      additionalProperties:
+        ` + nullableCat + `
+        discriminator: {propertyName: kind}
+`
+
+// TestCollapsedUnion_KeepsItsDiscriminatorAtEveryPosition pins that a
+// discriminator beside a oneOf/anyOf that collapses to a nullable reference is
+// kept verbatim, whole, on the node that position owns, with an info
+// diagnostic, instead of vanishing. The reference itself is unchanged. The
+// component, the model property, an array's items, additionalProperties, a
+// parameter and a response body each reach lowerOneOfAnyOf by their own path.
+func TestCollapsedUnion_KeepsItsDiscriminatorAtEveryPosition(t *testing.T) {
+	t.Parallel()
+	doc, diags := parseFull(t, collapseSpec)
+	openapitest.RequireNoErrorDiags(t, diags)
+	assert.Empty(t, pass.Validate(doc))
+	cat := componentID("Cat")
+
+	w, ok := typeByName(doc, "W").(*ir.Scalar)
+	require.True(t, ok, "the component is an alias over the nullable reference")
+	assert.Equal(t, ir.TypeRef{Target: cat, Nullable: true}, *w.Base, "the reference is unchanged")
+	entry, ok := w.Unmodeled["openapi:discriminator"]
+	require.True(t, ok, "the component keeps its discriminator")
+	assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason)
+	assert.Contains(t, string(entry.Value), "defaultMapping", "the whole object is kept, not just the property name")
+	assert.Contains(t, string(entry.Value), `"mapping"`)
+
+	holder, ok := typeByName(doc, "Holder").(*ir.Model)
+	require.True(t, ok)
+	require.Len(t, holder.Properties, 1)
+	assert.Equal(t, ir.TypeRef{Target: cat, Nullable: true}, holder.Properties[0].Type, "the property's reference is unchanged")
+	assert.Contains(t, holder.Properties[0].Unmodeled, "openapi:discriminator", "the property carrier keeps it")
+
+	for _, id := range []ir.TypeID{"t/anon/components/schemas/L/items", "t/anon/components/schemas/M/additionalProperties"} {
+		node, ok := doc.Types[id]
+		require.True(t, ok, id)
+		assert.Contains(t, node.Common().Unmodeled, "openapi:discriminator", "%s keeps it on the node it hoists", id)
+	}
+
+	for _, pointer := range []string{
+		"/components/schemas/W", "/components/schemas/Holder/properties/w",
+		"/components/schemas/L/items", "/components/schemas/M/additionalProperties",
+		"/paths/~1p/get/parameters/0/schema", "/paths/~1p/get/responses/200/content/application~1json/schema",
+	} {
+		assert.Contains(t,
+			openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, pointer),
+			"collapses to a nullable reference", pointer)
+	}
+	assert.Equal(t, 6, strings.Count(marshalForTest(t, doc), `"openapi:discriminator"`),
+		"one entry per position, including the parameter and the response, none duplicated")
+}
+
+// TestCollapsedUnion_WithoutADiscriminatorHoistsNothing pins that the same
+// positions without a discriminator stay as they were: no entry, no
+// diagnostic, and no node hoisted for the position that has none today.
+func TestCollapsedUnion_WithoutADiscriminatorHoistsNothing(t *testing.T) {
+	t.Parallel()
+	spec := openapitest.ComponentSpec(`    Cat: {type: object, properties: {kind: {type: string}}}
+    L: {type: array, items: {` + nullableCat + `}}
+`)
+	doc, diags := parseFull(t, spec)
+	openapitest.RequireNoErrorDiags(t, diags)
+	assert.Zero(t, openapitest.CountDiagsAt(diags, diag.DegradedConstruct, ir.SeverityInfo), "%+v", diags)
+	assert.NotContains(t, marshalForTest(t, doc), "openapi:discriminator")
+	assert.NotContains(t, doc.Types, ir.TypeID("t/anon/components/schemas/L/items"), "a bare nullable reference needs no node")
+}
+
+// marshalForTest returns doc's canonical JSON, to count entries wherever they
+// sit in it.
+func marshalForTest(t *testing.T, doc *ir.Document) string {
+	t.Helper()
+	out, err := json.Marshal(doc)
+	require.NoError(t, err)
+	return string(out)
+}
+
+// unplacedSpec declares a discriminator beside a oneOf/anyOf at each position
+// whose lowering has no model to hold it: a null-only union as a component, a
+// property and beside a second combinator, a union beside a const, an enum and
+// a scalar, and a union beside a $ref as a component and as a property. Each
+// reaches its keeper by its own path.
+const unplacedSpec = `openapi: 3.2.0
+info: {title: T, version: "1"}
+paths: {}
+components:
+  schemas:
+    Cat: {type: object, properties: {kind: {type: string}}}
+    Dog: {type: object, properties: {kind: {type: string}}}
+    NullOnly:
+      oneOf: [{type: 'null'}]
+      discriminator: {propertyName: kind}
+    NullBoth:
+      oneOf: [{type: 'null'}, {type: 'null'}]
+      anyOf: [{type: 'null'}]
+      discriminator: {propertyName: kind}
+    EnumU:
+      enum: [a, b]
+      oneOf: [{$ref: '#/components/schemas/Cat'}, {type: string}]
+      discriminator: {propertyName: kind}
+    ConstU:
+      const: a
+      oneOf: [{$ref: '#/components/schemas/Cat'}, {type: string}]
+      discriminator: {propertyName: kind}
+    ScalarU:
+      type: string
+      oneOf: [{$ref: '#/components/schemas/Cat'}, {type: string}]
+      discriminator: {propertyName: kind}
+    RefSite:
+      $ref: '#/components/schemas/Cat'
+      oneOf: [{$ref: '#/components/schemas/Cat'}, {$ref: '#/components/schemas/Dog'}]
+      discriminator: {propertyName: kind}
+    Holder:
+      type: object
+      properties:
+        n:
+          anyOf: [{type: 'null'}, {type: 'null'}]
+          discriminator: {propertyName: kind}
+        r:
+          $ref: '#/components/schemas/Cat'
+          oneOf: [{$ref: '#/components/schemas/Cat'}, {$ref: '#/components/schemas/Dog'}]
+          discriminator: {propertyName: kind}
+`
+
+// TestUnplacedDiscriminator_IsKeptAtEverySite pins that a discriminator beside
+// a oneOf/anyOf whose lowering has no model to hold it is kept verbatim on the
+// node or carrier that owns the position, with an info diagnostic, instead of
+// vanishing. A model-bodied union still carries it in its own field.
+func TestUnplacedDiscriminator_IsKeptAtEverySite(t *testing.T) {
+	t.Parallel()
+	doc, diags := parseFull(t, unplacedSpec)
+	openapitest.RequireNoErrorDiags(t, diags)
+	assert.Empty(t, pass.Validate(doc))
+
+	for _, name := range []string{"NullOnly", "NullBoth", "EnumU", "ConstU", "ScalarU", "RefSite"} {
+		node := typeByName(doc, name)
+		require.NotNil(t, node, name)
+		entry, ok := node.Common().Unmodeled["openapi:discriminator"]
+		require.True(t, ok, "%s keeps its discriminator", name)
+		assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason, name)
+		assert.Contains(t, string(entry.Value), "propertyName", name)
+		assert.True(t, namesDiscriminatorAt(diags, "/components/schemas/"+name), name)
+	}
+	for _, pointer := range []string{"/components/schemas/Holder/properties/n", "/components/schemas/Holder/properties/r"} {
+		assert.True(t, namesDiscriminatorAt(diags, pointer), pointer)
+	}
+
+	holder, ok := typeByName(doc, "Holder").(*ir.Model)
+	require.True(t, ok)
+	require.Len(t, holder.Properties, 2)
+	owned := map[string]ir.Unmodeled{"n": nil, "r": nil}
+	for _, prop := range holder.Properties {
+		owned[prop.Name.Source] = prop.Unmodeled
+	}
+	// The null-only property hoists a node of its own; the $ref one is on the carrier.
+	assert.Contains(t, doc.Types["t/anon/components/schemas/Holder/properties/n"].Common().Unmodeled, "openapi:discriminator")
+	assert.Contains(t, owned["r"], "openapi:discriminator")
+	assert.Equal(t, 8, strings.Count(marshalForTest(t, doc), `"openapi:discriminator"`),
+		"one entry per position, none duplicated")
+}
+
+// namesDiscriminatorAt reports whether an info degraded-construct diagnostic at
+// pointer mentions a discriminator. A site also reports its union, so the count
+// at a pointer is not one.
+func namesDiscriminatorAt(diags []ir.Diagnostic, pointer string) bool {
+	return slices.ContainsFunc(diags, func(d ir.Diagnostic) bool {
+		return d.Code == diag.DegradedConstruct && d.Severity == ir.SeverityInfo &&
+			string(d.Provenance.Pointer) == pointer && strings.Contains(d.Message, "discriminator")
+	})
+}

compilers/openapi/internal/schema/compose.go

@@ -590,6 +590,9 @@ func preserveNullOnlyUnion(c lowering.Ctx, ts *compile.Types, id ir.TypeID, s *o
 	common := td.Common()
 	kept, keepDiags := preserveBranchSets(c, &common.Unmodeled, s, ir.ReasonDegradedLowering, pointer)
 	diags = append(diags, keepDiags...)
+	diags = append(diags, keepUnplacedDiscriminator(c, &common.Unmodeled, s, pointer,
+		"every branch admits only the null value, so a discriminator beside them routes "+
+			"through no union; kept verbatim under Unmodeled")...)
 	if len(kept) == 0 {
 		return diags
 	}
@@ -696,11 +699,26 @@ const coDeclaredUnionWhyPrefix = "oneOf/anyOf co-declared with structural keywor
 // lowerCoDeclaredUnion lowers a schema whose oneOf/anyOf sits beside structural
 // keywords, per classifyUnionSiblings.
 func lowerCoDeclaredUnion(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, pointer jsontext.Pointer, hint string) (ir.TypeID, []ir.Diagnostic) {
+	// beside is the one place a body lowered to a Model beside a kept union gets
+	// its discriminator judged: every degraded outcome goes through it.
 	beside := func(reason ir.UnmodeledReason, why string) (ir.TypeID, []ir.Diagnostic) {
+		moves := s.GetDiscriminator() != nil && composesAsModel(s) && !discriminatorRoutesToSubtypes(c, s)
 		if why != "" {
 			why = coDeclaredUnionWhyPrefix + why
+			if moves {
+				why += discriminatorMovedWhy
+			}
+		}
+		id, diags := lowerBesideUnmodeledUnion(c, ts, anchors, depth, s, pointer, hint, reason, why)
+		if !moves {
+			return id, diags
 		}
-		return lowerBesideUnmodeledUnion(c, ts, anchors, depth, s, pointer, hint, reason, why)
+		if !namesMovedDiscriminator(diags) {
+			diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct,
+				pointer+ids.Ptr("discriminator"), "the discriminator routes to a target that is not a subtype "+
+					"of this model, so it is kept verbatim"))
+		}
+		return id, append(diags, moveDiscriminatorToUnmodeled(c, ts, id, s, pointer)...)
 	}
 	switch classifyUnionSiblings(c, s) {
 	case unionDistributed:
@@ -726,6 +744,18 @@ func lowerCoDeclaredUnion(c lowering.Ctx, ts *compile.Types, anchors *AnchorInde
 	}
 }
 
+// namesMovedDiscriminator reports whether diags already carry an info
+// degraded-construct diagnostic that says the discriminator moved off the model.
+func namesMovedDiscriminator(diags []ir.Diagnostic) bool {
+	for _, d := range diags {
+		if d.Code == diag.DegradedConstruct && d.Severity == ir.SeverityInfo &&
+			strings.Contains(d.Message, discriminatorMovedWhy) {
+			return true
+		}
+	}
+	return false
+}
+
 // diagUnresolvedBranches returns one diagnostic per union branch whose $ref
 // names nothing this compilation resolves, or none when they all resolve. The branch text survives verbatim under Unmodeled, so
 // nothing is dropped — but a reference that resolves nowhere is a defect of the

compilers/openapi/internal/schema/compose_internal_test.go

@@ -801,3 +801,15 @@ func TestResolveMappingTarget_DefsValueNothingInternedIsHoistedAtItsOwnPosition(
 		})
 	}
 }
+
+// TestMoveDiscriminatorToUnmodeled_ReportsAMissingNode pins that a node absent
+// from the registry is announced, not skipped: the discriminator would vanish
+// from the model with nowhere to be kept.
+func TestMoveDiscriminatorToUnmodeled_ReportsAMissingNode(t *testing.T) {
+	t.Parallel()
+
+	diags := moveDiscriminatorToUnmodeled(lowering.Ctx{}, compile.NewTypes(), "t/absent", nil, "/components/schemas/Ghost")
+
+	require.Len(t, diags, 1)
+	assert.Equal(t, diag.InternalInvariant, diags[0].Code)
+}