fix(compilers/openapi): lower a null-only union to nullable any - #460
Open
fuad-daoud wants to merge 2 commits into
Open
fix(compilers/openapi): lower a null-only union to nullable any#460fuad-daoud wants to merge 2 commits into
fuad-daoud wants to merge 2 commits into
Conversation
An anyOf/oneOf whose only branches were a bare `type: null` schema lowered to a Union with no variants: nullUnionCollapse only collapses a set with exactly one non-null branch, so a set with none fell through to buildUnion, which strips every null branch and interned a Union with nothing left. irverify's ir/union-no-variants rule rejects that document, but `morphic compile` still exited 0 (GitHub #416). The branch set admits exactly one value — null — the same value space a bare `{type: null}` schema at that position admits, so this position now lowers the same way: the shared `any` primitive with Nullable set. The oneOf/anyOf itself carries no shape a Scalar's fields could hold, so it is kept verbatim under Unmodeled (`degraded_lowering`) instead of dropped, with an info diagnostic. The guard sits where the null branches are stripped (lowerOneOfAnyOf, ahead of the buildUnion fallback) so the shape is structurally unreachable rather than caught after the fact, and it reads the combinator unionBranches actually elects, so it also covers a schema that declares both oneOf and anyOf when the elected one is null-only. Adds a conformance case (anyOf with one null branch, oneOf with one, anyOf with two), a minimized fuzz seed for FuzzLowerSchema, and internal/schema unit coverage for the new lowering and its degenerate guard branches. Closes #416 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJZJ8HaugGZTjzVueafLAT
Review follow-up on the null-only-union fix (#416): - Un-quarantine FuzzLowerSchema in scripts/fuzz.sh now that #416 is closed, per the script's own contract; make fuzz confirms it runs clean. - Add BothNull to the null-only-union conformance case: a schema declaring oneOf and anyOf at once, both null-only. unionBranches elects oneOf here, so lowerNullOnlyUnion runs the same way as the single-combinator cases, but buildUnion (and its preserveUnusedCombinator) is never reached, so preserveBranchSets's own loop over both keywords is what keeps anyOf from being dropped. Assert both openapi:oneOf and openapi:anyOf are kept, and regenerate the golden. - Name the null-only arm in lowerOneOfAnyOf's GoDoc. - lowerNullOnlyUnion no longer guards the alias hoist on a Lookup check copied from a sibling function where it could be false; here inner is the shared `any` primitive, which no schema pointer ever owns, so the guard was always true. Hoist unconditionally and say why. - Use the row key every neighbouring nullability conformance case cites (optionality-vs-nullability) instead of nil. - preserveBranchSets now returns the keywords it actually kept, so the null-only diagnostic names them ("oneOf and anyOf kept verbatim under Unmodeled") instead of saying "the union" when two branch sets survive. Closes #416 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJZJ8HaugGZTjzVueafLAT
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
1 participant
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
An
anyOf/oneOfwhose only branches were a baretype: nullschema lowered to aUnionwith no variants:nullUnionCollapsecollapses only a set with exactly one non-null branch, so a set with none fell through tobuildUnion, which strips every null branch and interned a Union with nothing left.irverify'sir/union-no-variantsrule rejects that document, butmorphic compilestill exited 0, and a bounded fuzz ofFuzzLowerSchemafound it in under two seconds (#416).The branch set admits exactly one value — null — which is the value space a bare
{type: null}schema at that position admits (established by compiling one: the sharedanyprimitive withNullableset). This position now lowers the same way, byte-identical in node kind andNullablebit. TheoneOf/anyOfitself carries no shape a Scalar's fields could hold, so it is kept verbatim underUnmodeled(degraded_lowering) with an infodegraded-construct, rather than dropped. The guard sits where the null branches are stripped — inlowerOneOfAnyOf, ahead of thebuildUnionfallback — so a variant-less union is structurally unreachable rather than caught after the fact, and it reads the combinatorunionBranchesactually elects, so a schema declaring bothoneOfandanyOfwith the elected one null-only is covered too (and keeps both keywords).scripts/fuzz.shquarantinedFuzzLowerSchemapending this issue; the script's contract is that closing the issue retires the entry, so the quarantine is lifted here and the gate's fuzz smoke step searches the target again (5 of 5 targets, no crasher).ir-design.md's OpenAPI row records the lowering.Test plan
null-only-union(witnessesoptionality-vs-nullability):anyOfwith one null branch,oneOfwith one,anyOfwith two, and both combinators null-only; each pins*ir.Scalar→t/prim/anywithNullable, the kept combinator(s) underUnmodeled, and exactly one infodegraded-constructat the position.morphic-harnessreportsok.FuzzLowerSchemaseed.ir/union-no-variantson all four schemas).make gatepasses: lint 0 issues, coverage at 100%, fuzz (quarantine lifted), bench-smoke.Closes #416
🤖 Generated with Claude Code
https://claude.ai/code/session_01TJZJ8HaugGZTjzVueafLAT