Repository navigation
fix(compilers/openapi)!: fold allOf branches beside a union - #1689
Merged
Merged
Conversation
An inline allOf branch writes what it says of the position, its docs, deprecation, examples, bounds and what a use binds, as if written beside the allOf, and folds there. Beside a oneOf, an anyOf or a $dynamicRef it folded nothing: the branch stayed whole under Unmodeled["openapi:allOf/<n>"], on each composed variant where the union distributes and on the Model where the union or the $dynamicRef is kept verbatim. The same keywords written beside the allOf reached the Union, its carriers and each variant's constraints, so two spellings JSON Schema reads alike lowered apart, and a consumer found the description of a distributed schema on no typed field. allOfDecides inherited allOfConjoins' exclusion of a union and a $dynamicRef beside the allOf. Those exclusions belong to refinement, which keeps composing beside either, but the composition is still what the position lowers. allOfDecides now excludes only a $ref beside the allOf, which keeps the allOf verbatim, and the distributed variants read the branches' bounds as lowerAllOf does. readAllOf marks which branches fold for both composers. A variant is the conjunction, so what a branch says of the position is said of each variant; it now lands once, on the Union where Docs and a carrier already read it, and a bound on each variant, where one beside the allOf lands. Keeping the branch whole on each variant let any one variant rebuild the source, but copied one fact onto nodes no reader of the position looks at. docs/ir/models.md records the choice. BREAKING CHANGE: IRVersion moves to 0.38.0 with no change of shape. A consumer pinned to 0.37.0 that read such a branch under its Unmodeled key no longer finds it there.
The differential holding a branch's keywords to the same keywords beside the allOf compared the nodes field by field: docs, deprecation, secrecy, XML hints, example count, constraints and kept entries. A spelling that moved a property, a base, a mixin or a catch-all apart passed it. It now compares each node whole with cmp, provenance aside, which names where each spelling was written. readAllOf's comment now says why it marks folds only where the allOf decides the position: that is the gate positionVoices delivers voices by, so a branch gives up a keyword only where a reader takes it up.
The differential ignored provenance whole, so a branch whose fold marked a fact inferred, or named another writer, where the same keyword beside the allOf does not, passed it. It now ignores only the pointer, position and node, which differ between the two spellings by where each was written.
This was referenced Nov 10, 2026
…anch Two fixtures took a union with an inline branch as one that degrades beside the allOf. Distributing a union's inline branches is planned (#661), and a fixture that then distributes would test a second distributed case under the degraded name, or fail where the nullable test requires the degraded report. Both now write a oneOf beside an anyOf, which degrades for a reason that change leaves alone. Each outcome the fold differential runs at now asserts the report that shows it reached that outcome, so a row that stops degrading fails rather than passing as another.
A use of a schema through a $ref reads what its folding branches lend (lendingBranches), and the change to fold a branch beside a union or a $dynamicRef widened that too: a property naming such a schema now takes the branch's readOnly, default, docs, deprecation and secrecy, as it takes them written beside the allOf. No test reached it. With lendingBranches given back the union and $dynamicRef exclusions, every test passed while the property lost each of them. The differential now compares a component holding a $ref use of S whole, beside S and its variants, and holds readOnly and default as it holds the other keywords. That plant now fails the readOnly, default, title, description, externalDocs, deprecated and redaction rows at each union and $dynamicRef outcome, and passes alone.
allOfConjoins reported whether an allOf's branches conjoin with what its position lowers, and excluded a union and a $dynamicRef beside it on that ground. Branches beside either now fold into the composition, so the predicate answers only whether the allOf may refine, as its comment already says, and both its callers ask exactly that. A name saying it conjoins invites the next reader to gate folding on it again, which is the exclusion allOfDecides had inherited.
The rule and the 0.38.0 entry read "as they do with neither there" after naming three keywords, a oneOf, an anyOf and a $dynamicRef; they now say none of them. The comment on the distributed residue test read as if the whole branch landed on the Union; it now says the branch is kept on each variant while what it folds reaches the Union.
buildComposedVariant read positionVoices once for every variant, though what it reads is the same for each: the schema, its position and an empty target. composedBody already carries the flattened allOf built once for every variant; it now carries the voices beside it, read once in lowerDistributedUnion. On a union of 500 variants beside an allOf of 20 branches that do not fold, the read per variant took about 1% more instructions and 1.3% more peak memory to compile; read once, both are where they were before such branches folded. No spec under testdata, the real-world corpus or a grid of the fold's outcomes at every schema position changes.
The rule says a oneOf, an anyOf or a $dynamicRef beside an allOf changes nothing but whether it refines: a branch disagreeing with a conjunct, or reading a $ref against another base, stays whole with its report, and two bounds meet, as they do with nothing beside the allOf. Every test of those held an allOf with nothing beside it, so a voicing that let a disagreeing branch fold, ignored another base, or kept two meeting bounds apart only where a union or a $dynamicRef stands beside the allOf passed every test under compilers/openapi. TestVoices_KeptAndMetBranchesHoldAtEveryOutcome runs a disagreement and two meeting bounds in both orders, and a branch under another base, at each outcome besideTheAllOf names, holding each Model the composition lowers to. Each of those three defects now fails it at every union and $dynamicRef outcome and passes alone. The fold differential's comment now names those outcomes too, not the unions alone.
The sentence on a union beside the allOf had been inserted between "an enum or a const beside it" and "An allOf that value set beats", which refers back to it. It now follows both.
composedModels, through which TestVoices_KeptAndMetBranchesHoldAtEveryOutcome reads the Models a composition lowers to, failed with a bare "Should be true" where S or one of its variants lowered to another kind, naming neither the node nor what it was. Each check now says which node lowered to which kind, so the failure diagnoses itself.
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
Closes #1686.
An inline
allOfbranch says what it writes for the position as if written beside theallOf, so itsdescription, deprecation, examples, extensions, bounds and use-bound keywords fold where the same keyword beside theallOflands (docs/compilers/openapi.md#allof-inline-residue). Beside aoneOf, ananyOfor a$dynamicRef, a branch folded nothing. It stayed whole underUnmodeled["openapi:allOf/<n>"]: on every composed variant where the union distributes, and on theModelwhere the union or the$dynamicRefis kept verbatim. The issue's example:On
main,Slowers to aUnionwith an emptyDocs.Description, and each variant keeps the branch whole. Withdescriptionandx-sensitivewritten beside theallOf, the same schema puts both on theUnion. JSON Schema reads the two spellings alike, but they lowered apart.Decision
The branches fold. A variant is the conjunction, so what a branch says of the position is said of each variant. Folding puts it in one place, where the beside spelling already puts it:
Union;Unionwhere none uses it;A branch with no residue is then kept on no variant. Keeping the branch whole on each variant let any one variant rebuild the source, but it copied one fact onto nodes no reader of the position looks at, and it made two spellings that mean the same thing lower differently.
docs/ir/models.md#composition-beside-a-unionrecords the choice and the rejected reading.Change
allOfDecides(voices.go) took overallOfConjoins' exclusion of a union and a$dynamicRefbeside theallOf. Those exclusions belong to refinement, which keeps composing beside either (allof-refinement, cases 1 and 4). The composition is still what the position lowers, soallOfDecidesnow excludes only a$refbeside theallOf, whose$refsite keeps theallOfverbatim.allOfConjoinskeeps all three exclusions and is renamedallOfMayRefine, since refinement is all it now answers for.readAllOfmarks which branches fold for both composers,lowerAllOfand the distributed variants. It moves besideallOfBranchesinallof.go.buildComposedVariantreads the folding branches' bounds, aslowerAllOfdoes, from voiceslowerDistributedUnionreads once for every variant, beside the flattenedallOfcomposedBodyalready carries.$refreads what the folding branches lend (lendingBranches), so a property naming such a schema takes the branch'sreadOnly,default, docs, deprecation and secrecy, as it takes them written beside theallOf.$dynamicRefis in scope because it is the same predicate clause, and the same divergence shows there:{$dynamicRef: '#meta', allOf: [{$ref: Obj}, {title: t, maxProperties: 3}]}kept the branch whole, whiletitleandmaxPropertiesbeside theallOfreachedDocsandConstraints.What does not change: a union or
$dynamicRefbeside theallOfstill keeps it from refining. Anenumorconstbeside both still wins the election and keeps theallOfverbatim. A branch that disagrees with a conjunct, or reads its$refs against another base, still stays whole with its report.Effect on real input
main's apart fromirVersion. It writes no folding branch beside a union today. The fivefilters/itemsrequest-body positions openapi: decide whether a composition distributed across a union folds its branches at the union #1686 names reach this rule only once the branch spelling of openapi: a union written as an allOf branch is kept verbatim, though beside the body it distributes #1682 lowers as a union beside the body.sweep.pyreads from the reference corpus, plus the 237 specs undertestdata: two documents change. One is the new conformance case. In the other, a schema written{allOf: [{multipleOf: 2}], anyOf: […], oneOf: […]}now holdsmultipleOf: 2in itsModel'sConstraints, as the same bound written beside theallOfdoes onmain.$refvariants beside anallOfof a$refand 20 inline branches takes a quarter fewer instructions than onmainwhere the inline branches fold (medians of five runs, 1,068M against 1,425M), since no variant copies them, and as many where they do not (1,965M against 1,967M).Breaking
IRVersionmoves to 0.38.0 with no change of shape (docs/ir/version-history.md). A consumer pinned to 0.37.0 that read such a branch underUnmodeled["openapi:allOf/<n>"]no longer finds it there; it finds what the branch said on theUnion, its carriers, each variant'sConstraintsor theModel.Test plan
At head d480c86:
TestVoices_AKeywordFoldsWhereTheAllOfWouldKeepItcompiles 22 keywords on a branch and beside theallOf, at five outcomes: alone, distributed, beside a degraded union (aoneOfbeside ananyOf), beside a narrowing union and beside a$dynamicRef, each row first asserting the report that shows it reached its outcome. It compares the position's node, each variant and a component holding a$refuse of the position whole withcmp, but for the pointer, position and node a provenance places it at, and the diagnostics by severity and code. WithallOfDecidesgiven back the union and$dynamicRefexclusions, all 66 union rows and all 22$dynamicRefrows fail while the 22 alone rows pass. With the variant's constraints read without voices, the distributedmaxPropertiesrow fails. With the fold marks cleared for the distributed composition only, the 22 distributed rows fail. WithlendingBranchesgiven back the same exclusions, thereadOnly,default,title,description,externalDocs,deprecatedand redaction rows fail at each union and$dynamicRefoutcome.TestVoices_KeptAndMetBranchesHoldAtEveryOutcomeruns, at the same five outcomes, a branch disagreeing with a conjunct and two meetingmaxProperties, each in both orders, and a branch reading a$refagainst another base, holding eachModelthe composition lowers to. A voicing that let a disagreeing branch fold, ignored another base, or kept two meeting bounds apart, each only where a union or a$dynamicRefstands beside theallOf, passed every test undercompilers/openapibefore it. Each now fails its rows at every union and$dynamicRefoutcome, and passes alone.TestVoices_AUseBindingKeywordReachesItsCarrierOrItsNodegains a distributed carrier: the branch'sreadOnlyanddefaultreach the property.TestNullableWidening_ABranchWidensTheAllOfHoldingItnow holds a 3.0 branchnullable: truebeside a degraded union (aoneOfbeside ananyOf) and a distributed one to widen the use, asnullable: truebeside theallOfalready did onmain. Each row asserts the outcome its name claims.TestAllOf_BranchResidueRidesEveryDistributedVariantkeeps a residue keyword and asserts the description also reaches theUnion.testdata/conformance/openapi/allof-inline-residue.yamlgainsDistributed. Its golden shows the description on theUnion,maxProperties: 4on each variant, and no residue report.$refuse,items,additionalProperties,prefixItems, an inline union branch, a parameter, a header, a request body and a response body), with where provenance places a node projected away: 33 keyword rows under 3.1 and 12 under 3.0, one in each writing several keywords together, at nine outcomes (alone, distributed by aoneOfand by ananyOf, both combinators, a narrowing union, an inline union branch, an unresolved branch, a discriminator, and a$dynamicRefunder 3.1 only). Each branch spelling's IR and diagnostic codes equal its beside spelling's, but for the keywords that do not fold alone either ($comment, aformatother than a redaction,minItems,uniqueItems,unevaluatedProperties: false); every beside spelling's equalsmain's; and a branch spelling's differs frommain's only beside a union or a$dynamicRef.go run ./cmd/morphic-harnesspasses on the new case and on every branch spelling above but the unresolved ones, which stop at their error by design. Reversing each spec's component order changes no document, operations included, and writing the folding branch before the$refmoves only a kept branch'sallOf/<n>key, as it does with no union.go testwith own-package coverage (100%),go vetandgolangci-lint runoncompilers/openapi/internal/schema,TestDocumentsandTestDocCommentsininternal/archtest,TestLoweringSpec, andscripts/vet-rules.share clean; the full gate is CI's.The degraded fixtures write a
oneOfbeside ananyOfrather than a union with an inline branch, since distributing inline branches is planned (#661) and would turn those rows into a second distributed case. The subject of commit 3766457 is 73 characters, one over the cap. The branch is pushed, so it stays, and the squash body set at merge keeps it offmain.