fix(compilers/openapi): honor every null spelling at $ref sites - #101
Merged
Conversation
A component that spells nullability the 3.1 way (`type: [object, "null"]`) lost that bit at every `$ref` use site: the referencing `TypeRef.Nullable` came out false, while the equivalent 3.0 spelling (`nullable: true`) came out true. Generated SDKs would treat such fields as non-nullable. refNullable inspected only the 3.0 `nullable` keyword on the reference site and on the resolved target. Under 3.1 the null member is stripped into the type node by effectiveTypes, so the bit is computed only on the TypeRef returned at the definition site — which lowerComponentSchema discards for interned components. Use schemaHasNull on both sites instead, the same dialect-agnostic helper the rest of the lowering already uses. The gap was wider than a direct ref: chained refs, refs to non-component sub-schemas, and nullable array/scalar components all dropped the bit under 3.1 while working under 3.0. All of them are now covered by tests, along with a conformance corpus case pinning the ref-site behaviour. Closes #28
A $ref use site has to recompute nullability, because a target interned at its
own TypeID has the TypeRef its definition produced discarded by
lowerComponentSchema. That recomputation understood only the two keyword
spellings, so a target spelling nullability as a union null branch lost the bit
at every reference. For a multi-branch union the loss is total: the null branch
is stripped out of the variants and the enclosing bit is dropped, so nothing in
the IR records that the type admits null.
Add schemaAdmitsNull and route both the reference site and the union lowering
through it, so the two places that compute a Nullable bit cannot drift apart. A
null branch counts only where the union is the type itself; structural siblings
intersect with it, so {type: object, oneOf: [string, null]} admits neither
string nor null — the same view schemaBody already takes inline, which keeps a
$ref and an inline copy of one body from disagreeing.
Harden the ref-nullability tests alongside: assert the resolved target on every
case, add a negative control per shape, pin ref/inline agreement, and cover a
non-property (list element) position.
schemaAdmitsNull ran hasUnionSiblings before oneOfAnyOfHasNull. The former ends in len(effectiveTypes(s)) > 0, and effectiveTypes allocates a slice only to take its length, so every $ref site paid that allocation twice even though almost no schema carries a union at all. Swapping the operands is behaviour-identical — both predicates are pure — and reaches hasUnionSiblings only when a null branch actually exists, which is also the only case its own doc comment describes.
…redicate schemaBody read nullability with schemaHasNull while the $ref site and the union lowering used schemaAdmitsNull. The two agree — on the union-siblings path the null-branch term is switched off by the sibling guard, and on the plain path there are no branches to weigh — so this changes no behaviour, and every golden in the corpus is byte-identical. It is the disagreement itself that is worth removing: nullability is spelled three ways and lifted onto the TypeRef by four different call sites, so a site computing it with its own predicate is how a spelling comes to be honoured in one position and dropped in another. One predicate leaves nowhere for that to happen, and a fourth spelling would land in a single function.
The whitebox tests hand-wrote the "t/openapi/components/schemas/" prefix at 49 sites, so a change to component ID derivation would have to be chased across three files by hand. Route them through componentID, and fold the two hand-rolled property-by-wire-name loops in the conformance suite into the propsByWire helper the whitebox package already had.
This was referenced Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OpenAPI lowering normalizes "admits null" onto the enclosing
TypeRef.Nullablebit rather than intothe type node (
ir-design.md§3.3, invariant 8). Three source spellings mean null:refNullable— the function that computes that bit at a$refuse site — recognized only the first.The other two were silently dropped at every reference, with no diagnostic, so generated SDKs would
treat such fields as non-nullable.
The bit is lost rather than merely relocated. A
$reftarget interned at its own TypeID (a model, aunion) has the
TypeRefits definition site produced discarded bylowerComponentSchema(
if owned { return }), so nothing downstream can recover it. For a multi-branch union the loss istotal:
oneOf: [string, integer, "null"]strips the null branch out of the variants and drops theenclosing bit, so the IR ends up with no record that the type admits null at all — an invariant-2
(lossless by default) violation.
The fix introduces
schemaAdmitsNulland routes both the reference site and its resolved targetthrough it, alongside the union lowering site in
compose.gothat already computed the same thinginline — so the two places that produce a Nullable bit can no longer drift apart.
A null branch counts only where the union is the type. Structural siblings intersect with it (JSON
Schema conjoins keywords), so
{type: object, oneOf: [{type: string}, {type: null}]}admits neitherstring nor null, and
{enum: [open, closed], oneOf: [{type: string}, {type: null}]}cannot be null atall.
schemaBodyalready takes that view inline — it reads nullability withschemaHasNullon theunion-siblings path and preserves the union verbatim under
Extensions— so the$refsite nowmatches it rather than over-reporting.
Measured before/after, with the 3.0 spelling for contrast:
$refto a nullable object component$ref(Owner.p → Mid → Base)$refto a nullable non-component sub-schema$ref$refto a collapsedoneOf: [X, null]component$refto a multi-branch union with a null branch$refto ananyOf: [X, null]component$refto a null branch intersected by structural siblingsChain-following is the resolver library's (
GetResolvedSchema) and was already exercised by the 3.0path, so this adds no new recursion and needs no new bound.
effectiveTypes,lowerComponentSchemaand the composition lowering are untouched.
Also in this PR
Two cleanups the fix argued for, neither changing behaviour:
schemaBodycomputed its Nullable bit withschemaHasNullwhile the$refsite and the unionlowering used
schemaAdmitsNull. The two agree — the sibling guard switches the null-branch term offon the union-siblings path, and the plain path has no branches to weigh — so every golden in the
corpus stays byte-identical. Routing all of them through one predicate is what removes the failure
mode this PR is about: a site computing nullability its own way is how a spelling gets honoured in
one position and dropped in another.
t/openapi/components/schemas/prefix at 49 sites; they now buildIDs with a
componentIDhelper, and the conformance suite's two hand-rolled property-by-wire-nameloops use the
propsByWirehelper the whitebox package already had.Test plan
TestSchema_RefNullableAcrossSpellings: 18 table cases covering all three spellings across direct,chained, sub-schema, array/scalar/union and ref-site-sibling shapes, each with a negative control
(plain target, non-null ref-site type array, union with no null branch, union intersected by a
structural body, enum with a null-branch sibling). Every case asserts the resolved target as well as
the bit. Verified load-bearing by reverting each production change in turn: the widening's cases fail
without it, the intersection guard's negative controls fail without it, and every 3.0 case passes
throughout.
TestSchema_RefNullableMatchesInlineForUnionSiblingsbuilds one body string and places it at both a$reftarget and an inline property, pinning that the two spellings agree — the specific way aref-site recomputation can drift from the inline rule.
TestSchema_RefNullableAtNonPropertyPositioncovers a$refused as a list element, since the bithas to reach every schema position, not just model properties.
nullable-31-refcovers both the type-array and union spellings, and pinsthat the union keeps exactly its two non-null variants — the null branch lifts to the ref rather
than becoming a variant.
$refs anull-type-array component.
gofmt -l,go build ./...,go vet ./...,golangci-lint run(0 issues),go test ./...clean.Closes #28