diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 12d593a..0d821c5 100644 --- a/compilers/openapi/conformance_test.go +++ b/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) { diff --git a/compilers/openapi/internal/schema/compose.go b/compilers/openapi/internal/schema/compose.go index e4d93ed..faf3eb3 100644 --- a/compilers/openapi/internal/schema/compose.go +++ b/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 { diff --git a/compilers/openapi/internal/schema/compose_test.go b/compilers/openapi/internal/schema/compose_test.go index 447db21..7b2dc6b 100644 --- a/compilers/openapi/internal/schema/compose_test.go +++ b/compilers/openapi/internal/schema/compose_test.go @@ -803,6 +803,134 @@ func TestAllOf_ExtraRefsBecomeMixins(t *testing.T) { assert.Equal(t, "c", c.Properties[0].Name.Source) } +// TestAllOf_NullabilityFollowsTheConjuncts pins that a composition over a +// null-admitting conjunct reads as nullable at every usage naming it, and that +// the bit is derived from the conjuncts rather than asserted. +// +// A composition declares no nullability of its own, so asking the composing +// schema alone answered "no" — and Model.Base carries no Nullable bit either, +// which left `allOf: [{$ref: T}]` over a nullable T with no record of the null +// anywhere and no diagnostic saying so. Against a model target there is not even +// a second hop to recover it from (GitHub #279). +// +// WrapPlain and WrapBoth are the halves that keep the rule from being "a +// conjunction is nullable": a conjunct declaring `type: object` forbids null, +// and one forbidding conjunct decides the conjunction however many of its +// siblings admit it. +func TestAllOf_NullabilityFollowsTheConjuncts(t *testing.T) { + t.Parallel() + spec := openapitest.ComponentSpec(` NullableName: {type: [string, "null"]} + NullableModel: + type: [object, "null"] + properties: {a: {type: string}} + PlainModel: + type: object + properties: {b: {type: string}} + WrapScalar: {allOf: [{$ref: '#/components/schemas/NullableName'}]} + WrapModel: {allOf: [{$ref: '#/components/schemas/NullableModel'}]} + WrapPlain: {allOf: [{$ref: '#/components/schemas/PlainModel'}]} + WrapBoth: + allOf: + - {$ref: '#/components/schemas/NullableModel'} + - {$ref: '#/components/schemas/PlainModel'} + Holder: + type: object + properties: + direct: {$ref: '#/components/schemas/NullableName'} + viaAllOf: {$ref: '#/components/schemas/WrapScalar'} + directModel: {$ref: '#/components/schemas/NullableModel'} + viaAllOfModel: {$ref: '#/components/schemas/WrapModel'} + viaAllOfPlain: {$ref: '#/components/schemas/WrapPlain'} + viaAllOfBoth: {$ref: '#/components/schemas/WrapBoth'} +`) + doc, diags := lowerSpec(t, spec) + openapitest.RequireNoErrorDiags(t, diags) + holder, ok := typeByName(doc, "Holder").(*ir.Model) + require.True(t, ok, "Holder is a model") + props := openapitest.PropsByWire(holder.Properties) + require.Len(t, props, 6) + + assert.True(t, props["viaAllOf"].Type.Nullable, "a conjunction over a nullable scalar admits null") + assert.Equal(t, props["direct"].Type.Nullable, props["viaAllOf"].Type.Nullable, + "wrapping a nullable scalar in an allOf does not change what the position admits") + assert.True(t, props["viaAllOfModel"].Type.Nullable, "a conjunction over a nullable model admits null") + assert.Equal(t, props["directModel"].Type.Nullable, props["viaAllOfModel"].Type.Nullable, + "a model target has no second hop to recover the null from, so the conjunction must carry it") + assert.False(t, props["viaAllOfPlain"].Type.Nullable, "a conjunction over an object admits no null") + assert.False(t, props["viaAllOfBoth"].Type.Nullable, + "one conjunct forbidding null decides the conjunction") + + // The bit lives on the usage, not on the conjunct: Base and Mixins name one + // side of a conjunction, which is the rule conjoinBranch and fillAllOf share. + for _, name := range []string{"WrapScalar", "WrapModel", "WrapPlain"} { + m, isModel := typeByName(doc, name).(*ir.Model) + require.True(t, isModel, "%s is a model", name) + require.NotNil(t, m.Base, "%s composes a sole $ref as its Base", name) + assert.False(t, m.Base.Nullable, "%s.Base names a conjunct and carries no Nullable bit", name) + } +} + +// TestDistributedUnion_VariantCarriesTheBranchNullability pins that a union +// distributed across its branches answers what the plain union over the same +// branches does. The distributed variant is a synthesized model, and the TypeRef +// naming it was built with no Nullable bit at all — so the branch's null was +// dropped whatever the branch said, with one info diagnostic that says nothing +// about it. +// +// The two distributed rows differ only in the enclosing `type: object`, which is +// the whole point: the variant is the body conjoined with the branch, so the +// body's own type keyword decides as much as the branch does. A fix that copied +// the branch's bit unconditionally passes the untyped row and fails the typed +// one. +func TestDistributedUnion_VariantCarriesTheBranchNullability(t *testing.T) { + t.Parallel() + spec := openapitest.ComponentSpec(` NullableModel: + type: [object, "null"] + properties: {a: {type: string}} + PlainModel: + type: object + properties: {b: {type: string}} + PlainUnion: + oneOf: + - {$ref: '#/components/schemas/NullableModel'} + - {$ref: '#/components/schemas/PlainModel'} + DistributedUntyped: + properties: {z: {type: string}} + oneOf: + - {$ref: '#/components/schemas/NullableModel'} + - {$ref: '#/components/schemas/PlainModel'} + DistributedTyped: + type: object + properties: {z: {type: string}} + oneOf: + - {$ref: '#/components/schemas/NullableModel'} + - {$ref: '#/components/schemas/PlainModel'} +`) + doc, diags := lowerSpec(t, spec) + openapitest.RequireNoErrorDiags(t, diags) + + nullableOf := func(name string) []bool { + t.Helper() + u, isUnion := typeByName(doc, name).(*ir.Union) + require.True(t, isUnion, "%s is a union", name) + require.Len(t, u.Variants, 2, "%s keeps one variant per branch", name) + return []bool{u.Variants[0].Type.Nullable, u.Variants[1].Type.Nullable} + } + + assert.Equal(t, []bool{true, false}, nullableOf("PlainUnion"), + "a plain union's variant carries its branch's nullability") + assert.Equal(t, nullableOf("PlainUnion"), nullableOf("DistributedUntyped"), + "distributing a body that declares no type does not change what a branch admits") + assert.Equal(t, []bool{false, false}, nullableOf("DistributedTyped"), + "a body declaring type: object conjoins with every branch and forbids null") + + // The synthesized variant model still names its branch as a plain conjunct. + base, isModel := doc.Types[ir.TypeID("t/composed/components/schemas/DistributedUntyped/oneOf/0")].(*ir.Model) + require.True(t, isModel, "the untyped distribution synthesizes a variant model") + require.NotNil(t, base.Base) + assert.False(t, base.Base.Nullable, "the variant model's Base names a conjunct and carries no bit") +} + func TestAllOf_DiscriminatorSubtypeValue(t *testing.T) { t.Parallel() spec := openapitest.ComponentSpec(` Pet: @@ -1031,6 +1159,24 @@ func TestEnum_NullMemberNormalizesToNullable(t *testing.T) { wantValueType: ir.PrimNumber, wantMembers: numMembers, }, + { + // A bare enum: nothing beside the members says whether null is in the + // value space, so the members say it themselves and the set normalizes + // exactly as its type-array spelling does. It used to keep the whole + // enum on the union-of-literals fallback (GitHub #265). + name: "bare enum, trailing null", + version: "3.1.0", + schema: `{enum: [red, green, null]}`, + wantValueType: ir.PrimString, + wantMembers: strMembers, + }, + { + name: "bare enum, leading null", + version: "3.1.0", + schema: `{enum: [null, 1, 2]}`, + wantValueType: ir.PrimNumber, + wantMembers: numMembers, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -1068,18 +1214,24 @@ func TestEnum_NullMemberNormalizesToNullable(t *testing.T) { // the normalization must not touch still lowers to a union of literals, with its // info diagnostic and every member preserved. // -// The last three rows are the ones that decide how far the rule reaches. A -// schema whose type keyword excludes null conjoins the two, so its `null` member -// admits nothing and normalizing would widen the type; a bare enum declares no -// nullability that schemaAdmitsNull — which a reference re-derives the bit from -// — would recognize, so stripping there would drop the null entirely; and an -// all-null set has no member left to build an Enum from. +// The last two rows decide how far the rule reaches. A schema whose type keyword +// excludes null conjoins the two, so its `null` member admits nothing and +// normalizing would widen the declared type — that row must stay non-nullable +// however the predicate is widened elsewhere; and an all-null set has no member +// left to build an Enum from. +// +// wantNullable is asserted beside the variants because the two answers have to +// agree: a set that keeps its null as a Literal variant *and* reads as nullable +// states one fact twice, which is tolerable, but a set whose null the fallback +// keeps while the reference denies it would be a position an emitter cannot +// generate. func TestEnum_NullMemberKeepsUnionFallback(t *testing.T) { t.Parallel() null := ir.Value{Kind: ir.ValueNull} cases := []struct { name, version, schema string wantVariants []ir.Value + wantNullable bool }{ { name: "heterogeneous members beside a null member", @@ -1090,32 +1242,38 @@ func TestEnum_NullMemberKeepsUnionFallback(t *testing.T) { {Kind: ir.ValueString, Str: "a"}, null, }, + wantNullable: true, }, { - name: "type keyword excludes null", + // The bare spelling of the row above: no type keyword, so the members + // decide, and they say the same thing the type array did. + name: "heterogeneous members, no type keyword", version: "3.1.0", - schema: `{type: string, enum: [red, green, null]}`, + schema: `{enum: [1, a, null]}`, wantVariants: []ir.Value{ - {Kind: ir.ValueString, Str: "red"}, - {Kind: ir.ValueString, Str: "green"}, + {Kind: ir.ValueNumber, Num: ir.BigVal("1")}, + {Kind: ir.ValueString, Str: "a"}, null, }, + wantNullable: true, }, { - name: "no type keyword to admit null", + name: "type keyword excludes null", version: "3.1.0", - schema: `{enum: [red, green, null]}`, + schema: `{type: string, enum: [red, green, null]}`, wantVariants: []ir.Value{ {Kind: ir.ValueString, Str: "red"}, {Kind: ir.ValueString, Str: "green"}, null, }, + wantNullable: false, }, { name: "every member is null", version: "3.1.0", schema: `{type: ["null"], enum: [null]}`, wantVariants: []ir.Value{null}, + wantNullable: true, }, } for _, tc := range cases { @@ -1126,6 +1284,12 @@ func TestEnum_NullMemberKeepsUnionFallback(t *testing.T) { assert.True(t, openapitest.HasDiagAt(diags, diag.DegradedConstruct, ir.SeverityInfo), "the degraded-enum diagnostic still fires; got %+v", diags) + owner, ok := doc.Types[componentID("S")].(*ir.Model) + require.True(t, ok, "S is a model") + require.Len(t, owner.Properties, 1) + assert.Equal(t, tc.wantNullable, owner.Properties[0].Type.Nullable, + "the reference agrees with the members the fallback kept") + u, ok := doc.Types[ir.TypeID("t/anon/components/schemas/S/properties/p")].(*ir.Union) require.True(t, ok, "the property lowers to a union of literals") require.Len(t, u.Variants, len(tc.wantVariants)) @@ -1155,6 +1319,37 @@ func TestConst_BecomesLiteral(t *testing.T) { assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "fixed"}, k.Value) } +// TestConst_FixesWhetherTheValueSpaceHoldsNull pins that `const` decides null +// admission the way a non-empty `enum` does — it is the one-member spelling of +// the same keyword. +// +// `{type: [string, "null"], const: "a"}` is the const form of the enum case that +// used to overstate: the type array puts null in the type space and the const +// takes it back out of the value space, so the position does not admit it. +func TestConst_FixesWhetherTheValueSpaceHoldsNull(t *testing.T) { + t.Parallel() + spec := openapitest.ComponentSpec(` S: + type: object + properties: + justNull: {const: null} + narrowed: {type: [string, "null"], const: "a"} +`) + doc, diags := lowerSpec(t, spec) + openapitest.RequireNoErrorDiags(t, diags) + m, ok := typeByName(doc, "S").(*ir.Model) + require.True(t, ok, "S is a model") + props := openapitest.PropsByWire(m.Properties) + require.Len(t, props, 2) + + assert.True(t, props["justNull"].Type.Nullable, "a const of null admits null") + lit, isLit := doc.Types[props["justNull"].Type.Target].(*ir.Literal) + require.True(t, isLit, "the const still lowers to its Literal") + assert.Equal(t, ir.Value{Kind: ir.ValueNull}, lit.Value) + + assert.False(t, props["narrowed"].Type.Nullable, + "a non-null const conjoins with the type array and takes the null back out") +} + func TestHoistLiteral_UnconvertibleConstBecomesAny(t *testing.T) { t.Parallel() // A custom tag is structurally unconvertible (no scalarValue case resolves diff --git a/compilers/openapi/internal/schema/resolve.go b/compilers/openapi/internal/schema/resolve.go index a340714..cd42b3f 100644 --- a/compilers/openapi/internal/schema/resolve.go +++ b/compilers/openapi/internal/schema/resolve.go @@ -187,17 +187,10 @@ func subSchemaHint(decl *oas3.JSONSchema[oas3.Referenceable], pointer string) st } // refNullable reports whether a $ref usage admits null: the reference site or -// its resolved target admits null in any spelling. The ref site must recompute -// this because a target interned at its own ID (a model, a union) discards the -// TypeRef its definition produced, so the bit survives nowhere else. +// its resolved target admits null in any spelling. It is schemaAdmitsNull read +// across the reference (refNullVerdict), so the two cannot answer one schema +// differently. func refNullable(js *oas3.JSONSchema[oas3.Referenceable]) bool { - if s := js.GetSchema(); s != nil && schemaAdmitsNull(s) { - return true - } - resolved := js.GetResolvedSchema() - if resolved == nil { - return false - } - target := resolved.GetSchema() - return target != nil && schemaAdmitsNull(target) + budget := maxNullConjuncts + return refNullVerdict(js, &budget) == nullAdmitted } diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index 3510b37..434a37e 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -8,6 +8,7 @@ import ( "strings" oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" + "github.com/speakeasy-api/openapi/values" yaml "gopkg.in/yaml.v3" "github.com/dexpace/morphic/compilers/compile" @@ -1714,33 +1715,202 @@ func effectiveTypes(s *oas3.Schema) []oas3.SchemaType { return out } -// schemaHasNull reports whether a schema admits null via either dialect: 3.0 -// nullable: true or a 3.1 type array containing "null". -func schemaHasNull(s *oas3.Schema) bool { +// nullVerdict is what one keyword family says about the null value. JSON Schema +// conjoins keywords, so the families are read together rather than first-match: +// a schema admits null when one family puts it in the value space and no other +// takes it out. +type nullVerdict int + +const ( + // nullSilent is a family the schema does not declare, or one that constrains + // only non-null instances. It forbids nothing. + nullSilent nullVerdict = iota + // nullAdmitted is a family that puts null in the value space. + nullAdmitted + // nullForbidden is a family that takes null out of it. + nullForbidden +) + +// maxNullConjuncts bounds the conjunct walk below (styleguide bounded-recursion +// rule). One budget unit is spent per schema visited, so it caps the walk's +// depth as well as its breadth — an `allOf` naming its own schema, or a diamond +// of conjunctions, terminates on it rather than on the shape of the source. A +// conjunction deeper or wider than this reads as silent, which is the answer +// that claims the least. +const maxNullConjuncts = 256 + +// schemaAdmitsNull reports whether a schema admits the null value. Lowering +// lifts every spelling of that onto the enclosing TypeRef rather than into the +// type node, so this is the one predicate every site computing a Nullable bit +// goes through — a definition site, a union, an allOf conjunct and a $ref use +// site must never disagree about the same schema. +func schemaAdmitsNull(s *oas3.Schema) bool { + budget := maxNullConjuncts + return schemaNullVerdict(s, &budget) == nullAdmitted +} + +// schemaNullVerdict reads what a whole schema says about null, by conjoining +// what each of its keyword families says (foldNullVerdicts). +// +// 3.0 `nullable: true` is the one keyword that does not conjoin: it widens the +// schema it is written on, which is what it exists to do, so it decides alone. +// The 3.1 spelling is an ordinary `type` member and conjoins like any other — +// which is why `{type: [string, "null"], enum: [red, green]}` does not admit +// null while `{type: string, nullable: true, enum: [red, green]}` does. +func schemaNullVerdict(s *oas3.Schema, budget *int) nullVerdict { + if s == nil || *budget <= 0 { + return nullSilent + } + *budget-- if s.Nullable != nil && *s.Nullable { - return true + return nullAdmitted + } + return foldNullVerdicts(typeNullVerdict(s), constNullVerdict(s), enumNullVerdict(s), + unionNullVerdict(s), allOfNullVerdict(s, budget)) +} + +// foldNullVerdicts conjoins keyword verdicts: one forbidding family decides the +// schema, otherwise one admitting family does, and a schema no family speaks for +// stays silent — which is not the same answer as forbidding, since a silent +// conjunct must not veto a sibling that admits. +func foldNullVerdicts(verdicts ...nullVerdict) nullVerdict { + out := nullSilent + for _, v := range verdicts { + if v == nullForbidden { + return nullForbidden + } + if v == nullAdmitted { + out = nullAdmitted + } + } + return out +} + +// typeNullVerdict reads the `type` keyword. A schema writing none constrains no +// instance kind at all, so it is silent rather than forbidding. +func typeNullVerdict(s *oas3.Schema) nullVerdict { + types := s.GetType() + if len(types) == 0 { + return nullSilent + } + if slices.Contains(types, oas3.SchemaTypeNull) { + return nullAdmitted + } + return nullForbidden +} + +// constNullVerdict reads `const`, which fixes the value space to one member. +func constNullVerdict(s *oas3.Schema) nullVerdict { + node := s.GetConst() + if node == nil { + return nullSilent + } + if isNullValue(node) { + return nullAdmitted } - return slices.Contains(s.GetType(), oas3.SchemaTypeNull) + return nullForbidden } -// schemaAdmitsNull reports whether a schema admits null in any spelling: the two -// keyword dialects (schemaHasNull) or a oneOf/anyOf null branch. Lowering lifts -// all of them onto the enclosing TypeRef rather than into the type node, so this -// is the one predicate every site computing a Nullable bit goes through — a -// definition site, a union, and a $ref use site must never disagree about the -// same schema. +// enumNullVerdict reads `enum`, which fixes the value space to its members: the +// position admits null exactly when a member is null. // -// A null branch counts only when the union is the type itself. Structural -// siblings intersect with the union (JSON Schema conjoins keywords), so +// An empty enum is silent rather than forbidding. It lists no member at all, so +// reading it as "no null member" would let a degenerate keyword strip a +// co-declared type array's null; what an empty enum lowers to is GitHub #278's +// question, and this rule leaves it open. +func enumNullVerdict(s *oas3.Schema) nullVerdict { + nodes := s.GetEnum() + if len(nodes) == 0 { + return nullSilent + } + if slices.ContainsFunc(nodes, isNullValue) { + return nullAdmitted + } + return nullForbidden +} + +// isNullValue reports whether an enum member or const node is the null literal, +// read through the same converter enumMembers drops a member by. Both must +// recognize one spelling: the null this predicate lifts onto a reference is +// exactly the member that lowering strips. +func isNullValue(node values.Value) bool { + val, err := value.FromNode(node) + return err == nil && val.Kind == ir.ValueNull +} + +// unionNullVerdict reads a oneOf/anyOf null branch, which counts only when the +// union is the type itself. Structural siblings intersect with the union, so // `{type: object, oneOf: [{type: string}, {type: null}]}` admits neither string // nor null; that union is kept verbatim under Unmodeled instead. A `type: null` // branch is written inline, so it also blocks distribution — no distributed // union can strip a null branch out from under this rule. -func schemaAdmitsNull(s *oas3.Schema) bool { - if schemaHasNull(s) { - return true +// +// It never forbids. A union with no null branch says nothing about a null a +// sibling keyword admits — `{nullable: true, oneOf: [...]}` is the 3.0 spelling +// of a nullable union, and a 3.1 `{type: [X, "null"]}` beside a union is the +// same statement. +func unionNullVerdict(s *oas3.Schema) nullVerdict { + if oneOfAnyOfHasNull(s) && !hasUnionSiblings(s) { + return nullAdmitted + } + return nullSilent +} + +// allOfNullVerdict conjoins what the allOf branches say. A conjunction admits +// null when a branch does and none forbids it, which is what makes +// `{allOf: [{$ref: T}]}` answer the same as `{$ref: T}` — the composition +// declares no nullability of its own, and the usage naming it has nowhere else +// to derive the bit from (GitHub #279). Model.Base and Mixins still carry no +// Nullable bit: they name a conjunct, and nullability is a property of the +// usage that names the conjunction. +func allOfNullVerdict(s *oas3.Schema, budget *int) nullVerdict { + out := nullSilent + for _, b := range s.GetAllOf() { + out = foldNullVerdicts(out, conjunctNullVerdict(b, budget)) } - return oneOfAnyOfHasNull(s) && !hasUnionSiblings(s) + return out +} + +// conjunctNullVerdict reads one allOf branch. A `false` branch admits no +// instance whatever, null included; a `true` branch constrains nothing. +func conjunctNullVerdict(b *oas3.JSONSchema[oas3.Referenceable], budget *int) nullVerdict { + if b == nil { + return nullSilent + } + if b.IsBool() { + if v := b.GetBool(); v != nil && !*v { + return nullForbidden + } + return nullSilent + } + s := b.GetSchema() + if resolve.IsRefSite(b, s) { + return refNullVerdict(b, budget) + } + return schemaNullVerdict(s, budget) +} + +// refNullVerdict reads a $ref usage: the reference site or its resolved target +// admitting null is enough, since a site's keywords widen the referent as often +// as they narrow it (3.0 writes `{$ref: T, nullable: true}` for exactly that). +// Only when neither admits does a forbidding side decide. +// +// The ref site must be read at all because a target interned at its own ID — a +// model, a union — discards the TypeRef its definition produced, so the bit +// survives nowhere else. +func refNullVerdict(js *oas3.JSONSchema[oas3.Referenceable], budget *int) nullVerdict { + site := schemaNullVerdict(js.GetSchema(), budget) + if site == nullAdmitted { + return nullAdmitted + } + var target nullVerdict + if resolved := js.GetResolvedSchema(); resolved != nil { + target = schemaNullVerdict(resolved.GetSchema(), budget) + } + if target == nullAdmitted { + return nullAdmitted + } + return foldNullVerdicts(site, target) } // nullUnionCollapse detects a oneOf/anyOf that has exactly one non-null branch diff --git a/compilers/openapi/internal/schema/schema_internal_test.go b/compilers/openapi/internal/schema/schema_internal_test.go index 1d1dbb9..0a4617d 100644 --- a/compilers/openapi/internal/schema/schema_internal_test.go +++ b/compilers/openapi/internal/schema/schema_internal_test.go @@ -447,6 +447,43 @@ func TestRefNullable_AnUnresolvedRefIsNotNullable(t *testing.T) { assert.False(t, refNullable(js)) } +// TestSchemaNullVerdict_TheConjunctWalkIsBounded pins the budget the conjunct +// walk runs on. Whether a schema admits null is decided partly by its allOf +// conjuncts, each reached through a $ref whose target is asked the same +// question, so a schema conjoining itself would otherwise not terminate. +// +// A budget spent per schema visited caps depth as well as breadth, and an +// exhausted walk answers "silent" — the verdict that claims the least, so a +// spec too deep to read is never reported as admitting a null it does not. +func TestSchemaNullVerdict_TheConjunctWalkIsBounded(t *testing.T) { + t.Parallel() + nullable := &oas3.Schema{ + Type: oas3.NewTypeFromArray([]oas3.SchemaType{oas3.SchemaTypeString, oas3.SchemaTypeNull}), + } + + budget := 1 + require.Equal(t, nullAdmitted, schemaNullVerdict(nullable, &budget), + "the fixture admits null while there is budget to read it") + + spent := 0 + assert.Equal(t, nullSilent, schemaNullVerdict(nullable, &spent), + "an exhausted budget stops the walk without claiming anything") + assert.Positive(t, maxNullConjuncts, "the cap is a real bound, not zero") +} + +// TestConjunctNullVerdict_ABranchWithNoSchemaSaysNothing pins the guard on an +// absent allOf entry. The parser never produces a nil branch, so nothing in the +// corpus reaches it; a conjunct that is not there constrains nothing, which is +// silence rather than a refusal — reading it as forbidding would let one +// missing entry strip a sibling's null. +func TestConjunctNullVerdict_ABranchWithNoSchemaSaysNothing(t *testing.T) { + t.Parallel() + budget := maxNullConjuncts + assert.Equal(t, nullSilent, conjunctNullVerdict(nil, &budget)) + assert.Equal(t, nullSilent, conjunctNullVerdict(openapitest.EmptyEitherSchema(), &budget), + "a branch whose either-value holds neither schema nor bool says nothing either") +} + // TestComponentSchemaAt_OnlyATopLevelComponentPointerHasABody pins the split // the function exists for: a component pointer has a body, and a pointer into // that same component does not. diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index bc15552..ba4933b 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -1175,6 +1175,57 @@ func TestSchema_RefNullableAcrossSpellings(t *testing.T) { } } +// TestSchema_NullabilityAgreesAcrossEnumSpellings pins that a value set and a +// type keyword conjoin, and that both ways of writing the same conjunction get +// the same answer. +// +// `{type: [string, "null"], enum: [red, green]}` and +// `{enum: [red, green], oneOf: [{type: string}, {type: "null"}]}` say one thing: +// null is in the type space and out of the value space, so the position does not +// admit it. The type-array spelling used to read the type keyword alone and call +// the position nullable while the oneOf spelling read the enum and called it +// not — two answers for one constraint in a single document (GitHub #288). +// +// Both members of a pair are written into one document on purpose: "the same +// schema, two spellings" is then a property of one compile rather than of two +// runs that could differ for unrelated reasons. The admitting pair is here for +// the same reason the excluding one is — a predicate hardcoded either way fails +// exactly one of them. +func TestSchema_NullabilityAgreesAcrossEnumSpellings(t *testing.T) { + t.Parallel() + spec := openapitest.ComponentSpec(` S: + type: object + properties: + excludedByType: {type: [string, "null"], enum: [red, green]} + excludedByUnion: {enum: [red, green], oneOf: [{type: string}, {type: "null"}]} + admittedByType: {type: [string, "null"], enum: [red, green, null]} + admittedByUnion: {enum: [red, green, null], oneOf: [{type: string}, {type: "null"}]} +`) + doc, diags := lowerSpec(t, spec) + openapitest.RequireNoErrorDiags(t, diags) + m, ok := typeByName(doc, "S").(*ir.Model) + require.True(t, ok, "S is a model") + props := openapitest.PropsByWire(m.Properties) + require.Len(t, props, 4) + + pairs := []struct { + name, typeSpelling, unionSpelling string + want bool + }{ + {"an enum listing no null member", "excludedByType", "excludedByUnion", false}, + {"an enum listing one", "admittedByType", "admittedByUnion", true}, + } + for _, p := range pairs { + t.Run(p.name, func(t *testing.T) { + t.Parallel() + got := props[p.typeSpelling].Type.Nullable + assert.Equal(t, p.want, got, "the enum decides whether the position admits null") + assert.Equal(t, got, props[p.unionSpelling].Type.Nullable, + "the type-array and oneOf spellings of one constraint must agree") + }) + } +} + // TestSchema_RefNullableMatchesInlineForUnionSiblings pins that one schema body // lowers to the same Nullable bit whether it is written inline or reached // through a $ref. The $ref site recomputes nullability, so it is the one place diff --git a/docs/ir-design.md b/docs/ir-design.md index 6194620..49da0f9 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -244,6 +244,12 @@ Compilers normalize every source spelling to this one bit: OAS 3.0 `nullable: tr A oneOf/anyOf/union whose only distinction is a null variant becomes a plain nullable `TypeRef`, never a union node. +The bit describes the whole schema, not the keyword that spells it. Where a source format conjoins +keywords (JSON Schema), a compiler reads them together: a value set that omits `null` beside a type +that names it — `{type: [string, "null"], enum: [red, green]}` — does not admit null, one that lists +`null` with no type beside it does, and a composition admits what its conjuncts jointly admit. So +every spelling of one constraint reaches the same bit, which is the only thing an emitter reads. + Protobuf field *presence* is **not** nullability — protobuf has no null. Presence disciplines lower to `Property.Presence` (§5.1), keeping `Nullable` strictly about wire-null. diff --git a/internal/archtest/recursion_test.go b/internal/archtest/recursion_test.go index f4decb6..e30ffb0 100644 --- a/internal/archtest/recursion_test.go +++ b/internal/archtest/recursion_test.go @@ -37,6 +37,15 @@ var loweringRecursions = [][]string{ // records this as the reason schema, compose and resolve cannot be separated // into packages. schemaRecursion, + // The nullability predicate. JSON Schema conjoins keywords, so whether a + // schema admits null is decided by its allOf conjuncts as much as by its own + // type set, and a conjunct is reached through a $ref whose target is a schema + // asked the same question. These four are not lowerings — they build no node + // and report no diagnostic — but they live in the same package and the call + // graph reads it whole, so the cycle is pinned here with the rest. It is + // bounded by an explicit budget (maxNullConjuncts), which is what stops a + // self-referential allOf rather than anything in this shape. + {"allOfNullVerdict", "conjunctNullVerdict", "refNullVerdict", "schemaNullVerdict"}, // Callbacks. An operation may declare callbacks, each of which is a path item // holding operations of its own (ir-design §8.1), so the operation lowering // reaches itself through them. diff --git a/testdata/conformance/openapi/nullability-conjunction.golden.json b/testdata/conformance/openapi/nullability-conjunction.golden.json new file mode 100644 index 0000000..6788849 --- /dev/null +++ b/testdata/conformance/openapi/nullability-conjunction.golden.json @@ -0,0 +1,580 @@ +{ + "irVersion": "0.3.0", + "name": "NullabilityConjunction", + "version": "1.0.0", + "docs": {}, + "services": [ + { + "id": "s/openapi/0", + "name": { + "source": "NullabilityConjunction", + "canonical": "nullability_conjunction" + }, + "docs": {}, + "auth": null, + "provenance": { + "source": 0 + } + } + ], + "types": { + "t/composed/components/schemas/DistributedUnion/oneOf/0": { + "kind": "model", + "id": "t/composed/components/schemas/DistributedUnion/oneOf/0", + "name": { + "hint": "DistributedUnion_NullableModel" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/DistributedUnion/oneOf/0" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/DistributedUnion/properties/z", + "name": { + "source": "z", + "canonical": "z" + }, + "wireName": "z", + "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/DistributedUnion/properties/z" + } + } + ], + "base": { + "target": "t/openapi/components/schemas/NullableModel", + "nullable": false + }, + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/composed/components/schemas/DistributedUnion/oneOf/1": { + "kind": "model", + "id": "t/composed/components/schemas/DistributedUnion/oneOf/1", + "name": { + "hint": "DistributedUnion_PlainModel" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/DistributedUnion/oneOf/1" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/DistributedUnion/properties/z", + "name": { + "source": "z", + "canonical": "z" + }, + "wireName": "z", + "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/DistributedUnion/properties/z" + } + } + ], + "base": { + "target": "t/openapi/components/schemas/PlainModel", + "nullable": false + }, + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/DistributedUnion": { + "kind": "union", + "id": "t/openapi/components/schemas/DistributedUnion", + "name": { + "source": "DistributedUnion", + "canonical": "distributed_union" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/DistributedUnion" + }, + "variants": [ + { + "name": { + "hint": "NullableModel" + }, + "type": { + "target": "t/composed/components/schemas/DistributedUnion/oneOf/0", + "nullable": true + }, + "docs": {} + }, + { + "name": { + "hint": "PlainModel" + }, + "type": { + "target": "t/composed/components/schemas/DistributedUnion/oneOf/1", + "nullable": false + }, + "docs": {} + } + ], + "exclusive": true, + "wireTagged": false + }, + "t/openapi/components/schemas/Holder": { + "kind": "model", + "id": "t/openapi/components/schemas/Holder", + "name": { + "source": "Holder", + "canonical": "holder" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Holder" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Holder/properties/direct", + "name": { + "source": "direct", + "canonical": "direct" + }, + "wireName": "direct", + "type": { + "target": "t/openapi/components/schemas/NullableScalar", + "nullable": true + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Holder/properties/direct" + } + }, + { + "id": "p/openapi/components/schemas/Holder/properties/viaAllOf", + "name": { + "source": "viaAllOf", + "canonical": "via_all_of" + }, + "wireName": "viaAllOf", + "type": { + "target": "t/openapi/components/schemas/WrapNullableScalar", + "nullable": true + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Holder/properties/viaAllOf" + } + }, + { + "id": "p/openapi/components/schemas/Holder/properties/directModel", + "name": { + "source": "directModel", + "canonical": "direct_model" + }, + "wireName": "directModel", + "type": { + "target": "t/openapi/components/schemas/NullableModel", + "nullable": true + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Holder/properties/directModel" + } + }, + { + "id": "p/openapi/components/schemas/Holder/properties/viaAllOfModel", + "name": { + "source": "viaAllOfModel", + "canonical": "via_all_of_model" + }, + "wireName": "viaAllOfModel", + "type": { + "target": "t/openapi/components/schemas/WrapNullableModel", + "nullable": true + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Holder/properties/viaAllOfModel" + } + }, + { + "id": "p/openapi/components/schemas/Holder/properties/viaAllOfBoth", + "name": { + "source": "viaAllOfBoth", + "canonical": "via_all_of_both" + }, + "wireName": "viaAllOfBoth", + "type": { + "target": "t/openapi/components/schemas/WrapBoth", + "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/Holder/properties/viaAllOfBoth" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/NullableModel": { + "kind": "model", + "id": "t/openapi/components/schemas/NullableModel", + "name": { + "source": "NullableModel", + "canonical": "nullable_model" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/NullableModel" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/NullableModel/properties/a", + "name": { + "source": "a", + "canonical": "a" + }, + "wireName": "a", + "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/NullableModel/properties/a" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/NullableScalar": { + "kind": "scalar", + "id": "t/openapi/components/schemas/NullableScalar", + "name": { + "source": "NullableScalar", + "canonical": "nullable_scalar" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/NullableScalar" + }, + "base": { + "target": "t/prim/string", + "nullable": true + } + }, + "t/openapi/components/schemas/PlainModel": { + "kind": "model", + "id": "t/openapi/components/schemas/PlainModel", + "name": { + "source": "PlainModel", + "canonical": "plain_model" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/PlainModel" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/PlainModel/properties/b", + "name": { + "source": "b", + "canonical": "b" + }, + "wireName": "b", + "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/PlainModel/properties/b" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/PlainUnion": { + "kind": "union", + "id": "t/openapi/components/schemas/PlainUnion", + "name": { + "source": "PlainUnion", + "canonical": "plain_union" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/PlainUnion" + }, + "variants": [ + { + "name": { + "hint": "NullableModel" + }, + "type": { + "target": "t/openapi/components/schemas/NullableModel", + "nullable": true + }, + "docs": {} + }, + { + "name": { + "hint": "PlainModel" + }, + "type": { + "target": "t/openapi/components/schemas/PlainModel", + "nullable": false + }, + "docs": {} + } + ], + "exclusive": true, + "wireTagged": false + }, + "t/openapi/components/schemas/WrapBoth": { + "kind": "model", + "id": "t/openapi/components/schemas/WrapBoth", + "name": { + "source": "WrapBoth", + "canonical": "wrap_both" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/WrapBoth" + }, + "mixins": [ + { + "target": "t/openapi/components/schemas/NullableModel", + "nullable": false + }, + { + "target": "t/openapi/components/schemas/PlainModel", + "nullable": false + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/WrapNullableModel": { + "kind": "model", + "id": "t/openapi/components/schemas/WrapNullableModel", + "name": { + "source": "WrapNullableModel", + "canonical": "wrap_nullable_model" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/WrapNullableModel" + }, + "base": { + "target": "t/openapi/components/schemas/NullableModel", + "nullable": false + }, + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/WrapNullableScalar": { + "kind": "model", + "id": "t/openapi/components/schemas/WrapNullableScalar", + "name": { + "source": "WrapNullableScalar", + "canonical": "wrap_nullable_scalar" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/WrapNullableScalar" + }, + "base": { + "target": "t/openapi/components/schemas/NullableScalar", + "nullable": false + }, + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/prim/string": { + "kind": "primitive", + "id": "t/prim/string", + "name": {}, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0 + }, + "prim": "string" + } + }, + "servers": [ + { + "name": { + "hint": "server" + }, + "urlTemplate": "/", + "description": {}, + "auth": null + } + ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/composition-lowering", + "message": "oneOf/anyOf co-declared with structural keywords; the composition is distributed across the union variants", + "provenance": { + "source": 0, + "pointer": "/components/schemas/DistributedUnion" + } + } + ], + "sources": [ + { + "format": "openapi@3.1", + "path": "nullability-conjunction.yaml", + "hash": "c93c4e662d34eed8099ad24c4c96ab5c57916fdf0d43d1c1c7bb1b6a2ffd2570" + } + ] +} diff --git a/testdata/conformance/openapi/nullability-conjunction.yaml b/testdata/conformance/openapi/nullability-conjunction.yaml new file mode 100644 index 0000000..7003364 --- /dev/null +++ b/testdata/conformance/openapi/nullability-conjunction.yaml @@ -0,0 +1,48 @@ +openapi: 3.1.0 +info: {title: NullabilityConjunction, version: "1.0.0"} +paths: {} +components: + schemas: + # JSON Schema conjoins keywords, so whether a position admits null is a + # question about the whole schema rather than about its type keyword. Every + # pair below writes one constraint two ways; the two spellings have to reach + # the same Nullable bit, because an emitter generates from the bit alone. + NullableScalar: {type: [string, "null"]} + NullableModel: + type: [object, "null"] + properties: {a: {type: string}} + PlainModel: + type: object + properties: {b: {type: string}} + # A composition declares no nullability of its own, and Base names a conjunct + # rather than a usage, so the null a sole conjunct admits has nowhere to sit + # but on the reference that names the conjunction. + WrapNullableScalar: + allOf: [{$ref: '#/components/schemas/NullableScalar'}] + WrapNullableModel: + allOf: [{$ref: '#/components/schemas/NullableModel'}] + # One conjunct forbidding null decides the conjunction, however many of its + # siblings admit it. + WrapBoth: + allOf: + - {$ref: '#/components/schemas/NullableModel'} + - {$ref: '#/components/schemas/PlainModel'} + # A union and the same union distributed across a sibling body: the variants + # name different nodes, but a branch admits what it admits either way. + PlainUnion: + oneOf: + - {$ref: '#/components/schemas/NullableModel'} + - {$ref: '#/components/schemas/PlainModel'} + DistributedUnion: + properties: {z: {type: string}} + oneOf: + - {$ref: '#/components/schemas/NullableModel'} + - {$ref: '#/components/schemas/PlainModel'} + Holder: + type: object + properties: + direct: {$ref: '#/components/schemas/NullableScalar'} + viaAllOf: {$ref: '#/components/schemas/WrapNullableScalar'} + directModel: {$ref: '#/components/schemas/NullableModel'} + viaAllOfModel: {$ref: '#/components/schemas/WrapNullableModel'} + viaAllOfBoth: {$ref: '#/components/schemas/WrapBoth'} diff --git a/testdata/conformance/openapi/nullable-enum-31.golden.json b/testdata/conformance/openapi/nullable-enum-31.golden.json index 45e6be9..c87377d 100644 --- a/testdata/conformance/openapi/nullable-enum-31.golden.json +++ b/testdata/conformance/openapi/nullable-enum-31.golden.json @@ -114,6 +114,54 @@ "nullable": true } }, + "t/openapi/components/schemas/BareColor": { + "kind": "enum", + "id": "t/openapi/components/schemas/BareColor", + "name": { + "source": "BareColor", + "canonical": "bare_color" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/BareColor" + }, + "valueType": "string", + "members": [ + { + "name": { + "source": "red", + "canonical": "red" + }, + "value": { + "kind": "string", + "str": "red", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + }, + { + "name": { + "source": "green", + "canonical": "green" + }, + "value": { + "kind": "string", + "str": "green", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + } + ], + "closed": true, + "flags": false + }, "t/openapi/components/schemas/Color": { "kind": "enum", "id": "t/openapi/components/schemas/Color", @@ -230,11 +278,113 @@ "source": 0, "pointer": "/components/schemas/Holder/properties/many" } + }, + { + "id": "p/openapi/components/schemas/Holder/properties/bare", + "name": { + "source": "bare", + "canonical": "bare" + }, + "wireName": "bare", + "type": { + "target": "t/openapi/components/schemas/BareColor", + "nullable": true + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Holder/properties/bare" + } + }, + { + "id": "p/openapi/components/schemas/Holder/properties/narrowed", + "name": { + "source": "narrowed", + "canonical": "narrowed" + }, + "wireName": "narrowed", + "type": { + "target": "t/openapi/components/schemas/NarrowedColor", + "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/Holder/properties/narrowed" + } } ], "abstract": false, "positional": false, "inputOnly": false + }, + "t/openapi/components/schemas/NarrowedColor": { + "kind": "enum", + "id": "t/openapi/components/schemas/NarrowedColor", + "name": { + "source": "NarrowedColor", + "canonical": "narrowed_color" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/NarrowedColor" + }, + "valueType": "string", + "members": [ + { + "name": { + "source": "red", + "canonical": "red" + }, + "value": { + "kind": "string", + "str": "red", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + }, + { + "name": { + "source": "green", + "canonical": "green" + }, + "value": { + "kind": "string", + "str": "green", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + } + ], + "closed": true, + "flags": false } }, "servers": [ @@ -251,7 +401,7 @@ { "format": "openapi@3.1", "path": "nullable-enum-31.yaml", - "hash": "b499a6a9913ed3f5173a77a4d492f04961325c66a10974a7fc4da2bfec5a36b1" + "hash": "8de80fe33a8b5b1a3f3edd5a56544fafa6a1cd89ecfe9506acdd811551b821f3" } ] } diff --git a/testdata/conformance/openapi/nullable-enum-31.yaml b/testdata/conformance/openapi/nullable-enum-31.yaml index 5691837..5e94449 100644 --- a/testdata/conformance/openapi/nullable-enum-31.yaml +++ b/testdata/conformance/openapi/nullable-enum-31.yaml @@ -14,8 +14,21 @@ components: Color: type: [string, "null"] enum: [red, green, null] + # The same set with nothing beside it. A non-empty enum fixes the value + # space, so the members alone say null is in it and the declaration + # normalizes exactly as Color does. + BareColor: + enum: [red, green, null] + # The other direction: the type array puts null in the type space and the + # members take it back out, so this position does not admit null even though + # its type keyword names it. + NarrowedColor: + type: [string, "null"] + enum: [red, green] Holder: type: object properties: one: {$ref: '#/components/schemas/Color'} many: {type: array, items: {$ref: '#/components/schemas/Color'}} + bare: {$ref: '#/components/schemas/BareColor'} + narrowed: {$ref: '#/components/schemas/NarrowedColor'}