Skip to content

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

Merged
OmarAlJarrah merged 2 commits into
mainfrom
fix/openapi-nullability-predicate
Aug 9, 2026
Merged

fix(compilers/openapi): decide nullability from the whole schema#333
OmarAlJarrah merged 2 commits into
mainfrom
fix/openapi-nullability-predicate

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

TypeRef.Nullable says whether a usage admits the null value, and every site that computes it
funnels through one predicate, schemaAdmitsNull. That predicate read a schema's type keyword
and a bare type: null union branch and nothing else, so it answered the same constraint
differently depending on how the source spelled it. Four sites were wrong, in both directions:

  • {type: [string, "null"], enum: [red, green]} emitted nullable: true. JSON Schema conjoins
    keywords, so the enum is the stricter of the pair and null is not a valid instance. The
    equivalent oneOf spelling already read as non-nullable, so one document could contain two
    spellings of one constraint that disagreed.
  • {enum: [red, green, null]} — a null member with no type beside it — did not normalize at
    all. It degraded to a union of three literals with an info diagnostic, and every reference to
    it said nullable: false, while the type-array spelling of the same member list produced
    Enum{red, green} behind a nullable reference.
  • allOf: [{$ref: T}] over a null-admitting T reached the IR with no record of the null at any
    site and no diagnostic. Where T is a model or an enum there is no second hop to recover it
    from, so the null was simply absent.
  • The same loss in the distributed-union lowering: the TypeRef naming a synthesized variant was
    built with no Nullable bit at all, so a branch's null was dropped whatever the branch said —
    while the plain oneOf over 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 allOf conjuncts) returns admits / forbids / silent, and the schema
admits null when one family puts it in the value space and no other takes it out. refNullable is
the same fold read across a reference, so a definition site, a union variant, an allOf conjunct
and a $ref use site cannot answer one schema differently. The conjunct walk runs on an explicit
budget (maxNullConjuncts), which bounds its depth as well as its breadth and terminates a schema
whose allOf names itself.

Two rules are deliberately left alone. OAS 3.0 nullable: true still decides on its own — it
widens the schema it is written on rather than conjoining with it, which is what
{nullable: true, allOf: [{$ref: Pet}]} means. And Model.Base/Mixins still carry no Nullable
bit: 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 enum reads as silent rather than as "lists no null member", so a degenerate keyword
cannot 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

  • New TestSchema_NullabilityAgreesAcrossEnumSpellings compiles both spellings of one constraint
    in a single document and asserts they reach the same bit, for a set that admits null and one
    that does not.
  • New TestAllOf_NullabilityFollowsTheConjuncts covers a conjunction over a nullable scalar and
    over 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 Base stays bare.
  • New TestDistributedUnion_VariantCarriesTheBranchNullability asserts the distributed variants
    agree with the plain union's, and that an enclosing type: object forbids the null anyway — a
    fix that copied the branch's bit unconditionally fails the second half.
  • New TestConst_FixesWhetherTheValueSpaceHoldsNull; TestEnum_NullMemberKeepsUnionFallback now
    asserts the reference's bit beside the variants it keeps, and its bare-enum row moves to
    TestEnum_NullMemberNormalizesToNullable.
  • Internal tests pin the walk's budget and the absent-conjunct guard.
  • Corpus: new nullability-conjunction conformance case (composition + distributed union), and
    nullable-enum-31 gains the bare and the null-excluding spellings. Both were confirmed to redden
    on a deliberate deletion from the fixture, since neither behaviour had a witness before —
    regenerating the goldens changed no existing byte outside those two files.
  • Every new and changed assertion was re-run with the production change reverted and the tests kept;
    all of them went red.
  • Full gate green: gofmt, go vet, golangci-lint, go build, ./scripts/check-coverage.sh at
    100%.

Closes #265
Closes #279
Closes #288

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
OmarAlJarrah force-pushed the fix/openapi-nullability-predicate branch from 9cfa68c to 0e6f728 Compare August 9, 2026 11:16
@OmarAlJarrah
OmarAlJarrah merged commit d7ce616 into main Aug 9, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the fix/openapi-nullability-predicate branch August 9, 2026 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant