fix(compilers/openapi): decide nullability from the whole schema - #333
Merged
Conversation
This was referenced Aug 9, 2026
Clean merge, and a tree that compiled nowhere: #328 consolidated the openapi test scaffolding into internal/openapitest, and this branch's new tests call six of those helpers by their old package-local names. Git reconciles both edits without a conflict, so the failure only shows on a build. Resolved by repointing the eight call sites — componentSpec, requireNoErrorDiags, propsByWire, emptyEitherSchema — across conformance_test.go, compose_test.go, schema_test.go and schema_internal_test.go. The signatures are unchanged, so these are renames. The archtest pin composed on its own: #331 widened loweringRecursions to see methods and this branch adds the four-function nullability cycle, and the merged order already satisfies #331's length-then-first-member comparator. TestLoweringRecursion_IsOnlyTheKnownCycles passing on the result is what checks that, rather than the eye.
OmarAlJarrah
force-pushed
the
fix/openapi-nullability-predicate
branch
from
August 9, 2026 11:16
9cfa68c to
0e6f728
Compare
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
TypeRef.Nullablesays whether a usage admits the null value, and every site that computes itfunnels through one predicate,
schemaAdmitsNull. That predicate read a schema'stypekeywordand a bare
type: nullunion branch and nothing else, so it answered the same constraintdifferently depending on how the source spelled it. Four sites were wrong, in both directions:
{type: [string, "null"], enum: [red, green]}emittednullable: true. JSON Schema conjoinskeywords, so the enum is the stricter of the pair and
nullis not a valid instance. Theequivalent
oneOfspelling already read as non-nullable, so one document could contain twospellings of one constraint that disagreed.
{enum: [red, green, null]}— a null member with notypebeside it — did not normalize atall. It degraded to a union of three literals with an
infodiagnostic, and every reference toit said
nullable: false, while the type-array spelling of the same member list producedEnum{red, green}behind a nullable reference.allOf: [{$ref: T}]over a null-admittingTreached the IR with no record of the null at anysite and no diagnostic. Where
Tis a model or an enum there is no second hop to recover itfrom, so the null was simply absent.
TypeRefnaming a synthesized variant wasbuilt with no
Nullablebit at all, so a branch's null was dropped whatever the branch said —while the plain
oneOfover the same branch carried it.The predicate now reads the whole schema. Each keyword family (
type/nullable,const,enum,a union's null branch, the
allOfconjuncts) returns admits / forbids / silent, and the schemaadmits null when one family puts it in the value space and no other takes it out.
refNullableisthe same fold read across a reference, so a definition site, a union variant, an
allOfconjunctand a
$refuse site cannot answer one schema differently. The conjunct walk runs on an explicitbudget (
maxNullConjuncts), which bounds its depth as well as its breadth and terminates a schemawhose
allOfnames itself.Two rules are deliberately left alone. OAS 3.0
nullable: truestill decides on its own — itwidens the schema it is written on rather than conjoining with it, which is what
{nullable: true, allOf: [{$ref: Pet}]}means. AndModel.Base/Mixinsstill carry noNullablebit: they name one side of a conjunction, and nullability belongs to the usage that names the whole
of it, so the distributed-union fix puts the bit on the union variant rather than on the variant
model's base.
An empty
enumreads as silent rather than as "lists no null member", so a degenerate keywordcannot strip a co-declared type array's null; what an empty enum should lower to is a separate open
question and this change does not settle it.
Test plan
TestSchema_NullabilityAgreesAcrossEnumSpellingscompiles both spellings of one constraintin a single document and asserts they reach the same bit, for a set that admits null and one
that does not.
TestAllOf_NullabilityFollowsTheConjunctscovers a conjunction over a nullable scalar andover a nullable model, with an object conjunct and a mixed pair as the controls that keep the
rule from being "a conjunction is nullable"; it also asserts
Basestays bare.TestDistributedUnion_VariantCarriesTheBranchNullabilityasserts the distributed variantsagree with the plain union's, and that an enclosing
type: objectforbids the null anyway — afix that copied the branch's bit unconditionally fails the second half.
TestConst_FixesWhetherTheValueSpaceHoldsNull;TestEnum_NullMemberKeepsUnionFallbacknowasserts the reference's bit beside the variants it keeps, and its bare-enum row moves to
TestEnum_NullMemberNormalizesToNullable.nullability-conjunctionconformance case (composition + distributed union), andnullable-enum-31gains the bare and the null-excluding spellings. Both were confirmed to reddenon a deliberate deletion from the fixture, since neither behaviour had a witness before —
regenerating the goldens changed no existing byte outside those two files.
all of them went red.
gofmt,go vet,golangci-lint,go build,./scripts/check-coverage.shat100%.
Closes #265
Closes #279
Closes #288