fix(compilers/openapi): lower a nullable enum as an Enum

[OmarAlJarrah]

Summary

OpenAPI 3.1 spells a nullable enum by listing null among the members of a null-admitting type
array. enumMembers classified that member as inadmissible, so the whole enum fell to the
union-of-literals fallback and lost its enum-ness. Compiling the issue's repro on main:

components:
  schemas:
    Color:
      type: [string, "null"]
      enum: [red, green, null]

produced a Union at t/openapi/components/schemas/Color over three hoisted Literals
(.../Color/enum/0 = "red", /1 = "green", /2 = null), plus
info openapi/degraded-construct: heterogeneous or non-scalar enum lowered as a union of literals.
The 3.0 spelling (type: string, nullable: true beside the same member list, which is how 3.0
writes a nullable enum at all) degraded identically. ir-design.md §3.3 says a null variant is
normalized onto TypeRef.Nullable, and the remaining members here are homogeneous strings, so the
lowering contradicted the documented normalization.

The fix strips the null member and builds the Enum from the rest, gated on schemaAdmitsNull.
That predicate is deliberate rather than incidental: it is the single place the declaration
position, a union, and refNullable at a $ref use site all compute the Nullable bit, so the
null dropped from the member list is exactly the null a reference to the enum puts back. Color
now lowers to Enum{red, green} with nullable: true on the reference — nothing else changed,
since the use-site half of the mechanism was already correct after #101.

Two smaller corrections come with it. Member kinds are reconciled against the members actually kept
rather than the one at index 0, so enum: [null, red, green] is the same string enum as
enum: [red, green, null] — without this, a leading null fixed the kind to one no scalar member
could match and the enum degraded anyway. And a set that keeps no member reports failure, so
{type: ["null"], enum: [null]} still degrades instead of producing a memberless Enum.

What deliberately still degrades

The union-of-literals fallback is unchanged for every member set the rule must not touch, and the
last two are scope decisions rather than oversights (both are stated in lowerEnum's doc comment):

• {type: string, enum: [red, green, null]} — keywords conjoin, so type: string excludes null and
  the member is dead. Normalizing here would claim the document admits null when it says it does
  not, which is the one direction of this change that could silently alter meaning.
• {enum: [red, green, null]} with no type keyword. In the 2020-12 dialect this does admit null,
  but schemaAdmitsNull reads only nullable: true and the type array, so stripping the member
  would set the bit nowhere — the null would vanish, which is the failure openapi: 3.1 type: [T, "null"] nullability is lost at every $ref site #28/fix(compilers/openapi): honor every null spelling at $ref sites #101 fixed for the
  other spellings. Making it work means teaching schemaAdmitsNull about enum members, which
  changes every site that computes nullability and has its own cases to settle. Filed as openapi: a bare enum with a null member does not normalize to a nullable Enum #265.
• Members heterogeneous for another reason (enum: [1, "a", null]), and an all-null set.

One position the normalized null does not reach

A conjunct is not a reference. Model.Base and Mixins name one side of a conjunction and carry no
Nullable bit at all, which conjoinBranch states as a rule of its own, so {allOf: [{$ref: T}]}
over a nullable T reaches no null and the property referencing the composition is not nullable
either. That predates this change and holds for every spelling of T's nullability — type: [string, "null"] on a scalar or a model degrades the same way today, with no diagnostic. What is
new is only that the nullable-enum spelling now joins them, since the enum no longer carries a null
literal variant of its own to be found by following the composition. It is filed as #279 and named
in lowerEnum's doc comment rather than fixed here: deciding it means deciding whether a
composition derives nullability from its conjuncts, which is a change to allOf lowering for every
target kind, not to the enum rule.

Test plan

Full gate green locally: gofmt -l, go vet ./..., golangci-lint run (0 issues),
go build ./..., ./scripts/check-coverage.sh (100.0%, 4620/4620 statements).

New coverage:

• TestEnum_NullMemberNormalizesToNullable — four rows over the 3.1 type-array and 3.0 nullable
  spellings, each asserting the Enum (closed, value type, exact members), the property
  reference's Nullable bit, and the absence of a degraded-construct diagnostic. Two rows put the
  null first, which is the order that was broken by the index-0 reconciliation.
• TestEnum_NullMemberKeepsUnionFallback — the four sets above, asserting the Union, the info
  diagnostic, and that every declared member (including the null) survives as a Literal.
• testdata/conformance/openapi/nullable-enum-31.{yaml,golden.json} with assertNullableEnum31:
  the declaration is an Enum of two members and the bit reaches a property $ref, an array
  element, and a query parameter, with a negative control that the array itself is not nullable.
  Being in the corpus also puts it under the harness sweep and the two-order permutation oracle
  (go run ./cmd/morphic-harness testdata/conformance/openapi/nullable-enum-31.yaml → ok).
• One row added to TestSchema_RefNullableAcrossSpellings so the nullable-enum shape is registered
  in that table. Its comment records that the bit there comes from the type array, so the row
  covers the spelling rather than the enum lowering.

Each assertion was proved able to fail by planting the defect and watching it go red:

Mutation in compose.go	Reddens
enumMembers(s.GetEnum(), false) (revert the fix)	all four NormalizesToNullable rows + the conformance case
len(members) == 0 → i == 0 (the old reconciliation)	only the two leading-null rows
drop the len(members) == 0 failure	only KeepsUnionFallback/every member is null
enumMembers(s.GetEnum(), true) (permissive guard)	only the type keyword excludes null and no type keyword rows
skip the kind check while dropping nulls	only KeepsUnionFallback/heterogeneous members beside a null member

Deleting the null member from the corpus spec also reddens TestConformance/nullable-enum-31, so
the golden is genuinely compared. That golden cannot pin the strip on its own, though: the type
array already carries the bit, so the IR body is byte-identical with or without the member and only
the source hash moves. assertNullableEnum31's member count is what pins it — which mutation 1
above confirms — and the assertion's doc comment now says so rather than leaving it to be
rediscovered.

Beyond the tests, the use-site claim was checked by compiling rather than by reading: a spec
referencing the nullable enum as a property, an array element, a map value, a tuple element, a
query parameter, a request body, a response body, a response header, a union variant, a $ref
carrying sibling keywords, and a sub-schema pointer puts nullable: true on the TypeRef at every
one. The conjunct position above is the only one that does not, and it behaves identically for a
non-enum nullable target.

One further note on the predicate, since this change leans on it in one direction.
schemaAdmitsNull is exact where the strip needs it: a null member is dropped
precisely where a reference puts the bit back. It overstates in the other
direction — {type: [string, "null"], enum: [red, green]}, a nullable type array
whose enum excludes null, still reads as nullable at every reference although
the conjunction forbids that value. That is pre-existing and untouched here, this
change altering behaviour only when the member list does contain null, and it is
inconsistent with the oneOf spelling, where an enum sibling already suppresses
the null via hasUnionSiblings. Filed as #288.

Closes #44

[OmarAlJarrah]

Code review

Found 1 issue, fixed in 066a683.

1. enumMembers' lead sentence enumerated two conditions for ok=false — a non-scalar member, heterogeneous kinds — but this change adds a third: a set that keeps no member, which is exactly what makes an all-null enum degrade rather than become a memberless Enum. It was documented, but inside a later paragraph about the returned PrimKind, so the sentence a reader takes as the contract was the one that omitted it. (CLAUDE.md: "a doc comment that is imprecise ... 'Minor' is a severity, not a permission to ship it")

morphic/compilers/openapi/internal/schema/compose.go

Lines 1014 to 1027 in fa4b9f0

	// enumMembers converts enum nodes into scalar members, reporting ok=false when
	// any member is non-scalar or the members are heterogeneous (mixed kinds).
	//
	// dropNull skips `null` members instead of refusing them, for a schema whose
	// nullability the enclosing reference already carries (see lowerEnum). Kind
	// agreement is read off the members actually kept, so a leading `null` fixes
	// nothing: `enum: [null, red, green]` is the same string enum as
	// `enum: [red, green, null]`.
	//
	// The returned PrimKind is the one every kept member's kind maps to. It is
	// meaningful only when ok, and a set that keeps no member reports ok=false — an
	// all-null enum degrades rather than becoming a memberless Enum — so it is never
	// the zero PrimKind there.
	func enumMembers(nodes []values.Value, dropNull bool) ([]ir.EnumMember, ir.PrimKind, bool) {

Everything else in the change verified out. The load-bearing claim — that schemaAdmitsNull is both what strips the member and what a reference re-derives the bit from — holds at every site: refNullable reads it on use-site and target, and the inline positions in lowerSchemaBody read it directly, so the null dropped is the null put back.

Two claims I checked by compiling rather than reading:

• The conjunct exception is genuinely kind-agnostic. allOf: [{$ref: T}] reaches nullable: false for a nullable enum, a nullable scalar, and a nullable model alike, with a direct $ref to the same enum correctly nullable as the control. So the enum joins an existing gap rather than opening one, which is what openapi: an allOf over a nullable $ref reaches the IR with no record of the null #279 says and what makes leaving it right.
• The strip is unreachable through the oneOf spelling. declaresShape counts a non-empty enum, so hasUnionSiblings is always true for an enum schema and schemaAdmitsNull can only be true via schemaHasNull — the two spellings the tests already cover. There is no untested path into the new branch.

The mutation table reproduces exactly:

mutation	reddens
dropNull=false (revert)	all four NormalizesToNullable rows + TestConformance/nullable-enum-31
i == 0 reconciliation	only the two leading-null rows
drop the empty-set failure	only KeepsUnionFallback/every member is null
dropNull=true (permissive)	only type keyword excludes null and no type keyword
skip the kind check while dropping nulls	only heterogeneous members beside a null member

Two notes on reproducing that last row: spelled as an unconditional removal of the else if it fails to build (kind declared and not used), and spelled as an unconditional false && it also reddens TestEnum_NonScalarAndMidListMismatch and TestConformance/empty-names. Gated on !dropNull, which is what "while dropping nulls" describes, it reddens the one row as claimed. Deleting the null member from the corpus spec also reddens the conformance case, confirming the golden is genuinely compared.

Filed #288 while checking this, not fixed here: a nullable type array whose enum excludes null — {type: [string, "null"], enum: [red, green]} — still reads as nullable at every reference, admitting a value the conjunction forbids. Pre-existing and untouched by this change, which only alters behavior when the member list does contain null. Worth recording because it is the same predicate seen from the other side: exact in the direction this PR relies on, overstating in the other, and inconsistent with the oneOf spelling, where an enum sibling already suppresses the null.

Gate green at 066a683: gofmt, go vet, golangci-lint (0 issues), go build, 4620/4620 statements. The fix commit changes comments only — no executable line differs.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}