fix(compilers/openapi): decide nullability from the whole schema

compilers/openapi/conformance_test.go

@@ -171,6 +171,7 @@ func conformanceCases() []conformanceCase {
 		{"nullable-30", assertNullable30},
 		{"nullable-31-ref", assertNullable31Ref},
 		{"nullable-enum-31", assertNullableEnum31},
+		{"nullability-conjunction", assertNullabilityConjunction},
 		{"defaults", assertDefaults},
 		{"yaml-timestamp-scalars", assertYAMLTimestampScalars},
 		{"constraints", assertConstraints},
@@ -1002,6 +1003,79 @@ func assertNullableEnum31(t *testing.T, doc *ir.Document, diags []ir.Diagnostic)
 	require.True(t, ok)
 	assert.Equal(t, ir.TypeRef{Target: namedID("Color"), Nullable: true}, p.Type,
 		"a parameter reaches the same declaration through the operation walk")
+
+	// The two spellings beside Color. A non-empty enum fixes the value space, so
+	// the members decide null admission whether or not a type keyword is written
+	// beside them — and when one is, the two conjoin rather than the type winning.
+	bare, ok := doc.Types[namedID("BareColor")].(*ir.Enum)
+	require.True(t, ok, "a bare enum listing null is still an enum of its scalar members")
+	require.Len(t, bare.Members, 2, "the null member is normalized away here too")
+	assert.Equal(t, ir.PrimString, bare.ValueType, "the kept members supply the value type")
+
+	narrowed, ok := doc.Types[namedID("NarrowedColor")].(*ir.Enum)
+	require.True(t, ok)
+	require.Len(t, narrowed.Members, 2, "nothing to strip: the members never listed null")
+
+	bareRef, ok := propByWire(m, "bare")
+	require.True(t, ok)
+	assert.Equal(t, ir.TypeRef{Target: namedID("BareColor"), Nullable: true}, bareRef.Type,
+		"the bare spelling admits null at its uses, like the type-array spelling of the same set")
+
+	narrowedRef, ok := propByWire(m, "narrowed")
+	require.True(t, ok)
+	assert.Equal(t, ir.TypeRef{Target: namedID("NarrowedColor"), Nullable: false}, narrowedRef.Type,
+		"an enum excluding null must not admit it, whatever the type keyword names")
+}
+
+// assertNullabilityConjunction covers the rule that decides null admission for a
+// schema whose keywords disagree: JSON Schema conjoins them, so a position
+// admits null when something declares it and nothing takes it away. The
+// capability claimed is agreement — the same constraint written two ways reaches
+// one Nullable bit — which is what a target language can act on, since an
+// emitter reads the bit and never the spelling.
+//
+// Base and Mixins are asserted to stay bare on purpose. They name one side of a
+// conjunction, so the bit belongs to the usage that names the whole of it; a
+// composition carrying its own would put the same fact in two places that can
+// then disagree.
+func assertNullabilityConjunction(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
+	m, ok := doc.Types[namedID("Holder")].(*ir.Model)
+	require.True(t, ok)
+	props := openapitest.PropsByWire(m.Properties)
+	require.Len(t, props, 5)
+
+	assert.Equal(t, props["direct"].Type.Nullable, props["viaAllOf"].Type.Nullable,
+		"a sole conjunct's null survives the composition that names it")
+	assert.True(t, props["viaAllOf"].Type.Nullable)
+	assert.Equal(t, props["directModel"].Type.Nullable, props["viaAllOfModel"].Type.Nullable,
+		"a model target has no second hop to recover the null from")
+	assert.True(t, props["viaAllOfModel"].Type.Nullable)
+	assert.False(t, props["viaAllOfBoth"].Type.Nullable,
+		"one conjunct forbidding null decides the conjunction")
+
+	for _, name := range []string{"WrapNullableScalar", "WrapNullableModel", "WrapBoth"} {
+		wrap, isModel := doc.Types[namedID(name)].(*ir.Model)
+		require.True(t, isModel, "%s composes as a model", name)
+		if wrap.Base != nil {
+			assert.False(t, wrap.Base.Nullable, "%s.Base names a conjunct, not a usage", name)
+		}
+		for i, mix := range wrap.Mixins {
+			assert.False(t, mix.Nullable, "%s.Mixins[%d] names a conjunct, not a usage", name, i)
+		}
+	}
+
+	plain, ok := doc.Types[namedID("PlainUnion")].(*ir.Union)
+	require.True(t, ok)
+	distributed, ok := doc.Types[namedID("DistributedUnion")].(*ir.Union)
+	require.True(t, ok)
+	require.Len(t, plain.Variants, 2)
+	require.Len(t, distributed.Variants, 2)
+	for i := range plain.Variants {
+		assert.Equal(t, plain.Variants[i].Type.Nullable, distributed.Variants[i].Type.Nullable,
+			"variant %d: distributing a body that declares no type does not change what its branch admits", i)
+	}
+	assert.True(t, plain.Variants[0].Type.Nullable, "the nullable branch is the one that admits null")
+	assert.False(t, plain.Variants[1].Type.Nullable, "and the plain one is not")
 }
 
 func assertDefaults(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {

compilers/openapi/internal/schema/compose.go

@@ -791,7 +791,26 @@ func composedVariant(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, de
 	common := commonFor(c, id, vptr, compile.SubHint(body.hint, vhint))
 	def, variantDiags := buildComposedVariant(c, ts, anchors, depth, body, branch.Target, common)
 	ts.Register(id, def)
-	return ir.TypeRef{Target: id}, append(diags, variantDiags...)
+	return ir.TypeRef{Target: id, Nullable: composedVariantNullable(body.schema, branch)},
+		append(diags, variantDiags...)
+}
+
+// composedVariantNullable reports whether the union variant naming a
+// synthesized variant admits null: the variant is the enclosing body conjoined
+// with the branch, so it admits null exactly when the branch does and the body
+// does not forbid it. That is schemaNullVerdict's allOf rule applied to the one
+// conjunction the source does not spell as an allOf, which is what keeps a
+// distributed union answering what the plain union over the same branch does.
+//
+// The bit belongs on this TypeRef rather than on the variant model's Base or
+// Mixins for the reason conjoinBranch records: those name a conjunct, and
+// nullability is a property of the usage that names the conjunction.
+func composedVariantNullable(body *oas3.Schema, branch ir.TypeRef) bool {
+	if !branch.Nullable {
+		return false
+	}
+	budget := maxNullConjuncts
+	return schemaNullVerdict(body, &budget) != nullForbidden
 }
 
 // buildComposedVariant assembles the variant Model itself. Every fill reads the
@@ -818,7 +837,8 @@ func buildComposedVariant(c lowering.Ctx, ts *compile.Types, anchors *AnchorInde
 // nothing, a Mixin otherwise. Only the target is carried, never the branch
 // ref's Nullable bit — fillAllOf drops it for an allOf entry on the same
 // reasoning: Base and Mixins name a conjunct, and nullability is a property of
-// the usage that names the conjunction, not of one side of it.
+// the usage that names the conjunction, not of one side of it. The usage here is
+// the union variant, and composedVariantNullable is what puts the bit on it.
 func conjoinBranch(m *ir.Model, branch ir.TypeID) {
 	target := ir.TypeRef{Target: branch}
 	if m.Base == nil && len(m.Mixins) == 0 {
@@ -1014,25 +1034,25 @@ func propIDByName(m *ir.Model, name string) (ir.PropID, bool) {
 // non-scalar member set has no Enum home, so it falls back to a Union of
 // Literals with an info diagnostic — nothing is dropped.
 //
-// A schema that admits null in its own right — `type: [T, "null"]`, 3.0
-// `nullable: true` — spells its nullable enum by listing `null` among the
+// A schema that admits null spells its nullable enum by listing `null` among the
 // members, so that member is stripped and normalized onto the enclosing
 // reference's Nullable bit (ir-design §3.3) rather than degrading the whole
 // enum. schemaAdmitsNull is what decides that here *and* what a reference
 // re-derives the bit from (refNullable at a $ref site, lowerSchemaBody inline),
 // so the null this drops is exactly the null those put back; a spelling only one
-// of them recognized would lose it. A conjunct position is the standing
-// exception: Model.Base and Mixins name one side of a conjunction and carry no
-// Nullable bit at all (conjoinBranch), so an `allOf: [{$ref: T}]` over a nullable
-// T reaches no null — in every spelling of T's nullability, this one included
-// rather than this one only. GitHub #279 holds that.
+// of them recognized would lose it.
+//
+// A non-empty enum decides null admission by itself, so a bare
+// `{enum: [red, green, null]}` normalizes like the type-array spelling of the
+// same set. `{type: string, enum: [red, green, null]}` still does not: the type
+// keyword conjoins with the members and forbids the null they list, so
+// stripping there would widen the declared type rather than normalize it.
 //
-// That is also why the enum's own `null` member does not itself count as
-// admitting null: `{enum: [red, green, null]}` with no type keyword is left to
-// the union-of-literals fallback, since schemaAdmitsNull does not read enum
-// members and stripping on a wider rule here would set the bit nowhere. Widening
-// it is a change to every site that computes nullability, not to this one, and
-// it has its own decisions to make — GitHub #265 holds them.
+// A member set with no Enum home is unaffected by the stripping either way. It
+// falls back to a Union over the members as written, null included, beside a
+// reference that also says the position admits null — one fact stated twice,
+// which is what the type-array spelling of such a set already produced and what
+// the bare spelling now matches.
 func lowerEnum(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, pointer, hint string) (ir.TypeID, []ir.Diagnostic) {
 	var diags []ir.Diagnostic
 	id := internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef {