Narrow unions through $ref and allOf - #419
Open
holodorum wants to merge 1 commit into
Open
Conversation
Union narrowing only read a branch's own properties and required, plus
one hop through a lone $ref. That misses what code generators emit for
a base type with variants:
allOf: [{$ref: Base}, {oneOf: [...]}]
The properties live on Base, and the union reaches the branch through a
$ref, so the branch looks empty. Nothing eliminates it, nothing matches
it, and we fall back to dumping every branch's errors.
Follow both edges instead, transitively, with a visited set, and read
everything reached as one declaration. They all constrain the same
document, so a property pinned by several of them is pinned to the
intersection of their sets. Property schemas are still not followed, so
child: {$ref: node} stays out of the walk.
Tidied up while in here. The terms this code uses (pin, known property,
discriminator) now have one glossary at the top of UnionNarrowing.kt
rather than a definition in whichever KDoc mentioned them first, and
narrowByElimination, which also did presence matching, is split in
three. Left a todo on pinnedValues: it only exists because a schema
keeps its compiled validators and not the declarations behind them.
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.
Union narrowing picks which
oneOf/anyOfbranch a failing document meant, so we report that branch's error instead of every branch's. To do that it needs to know what each branch declares, and it could only see declarations written directly on the branch, or one$refaway. It could not see declarations reached through anallOf, which is where a base type's shared fields normally live:The properties live on
Baseand the union reaches the branch through a$ref, so the branch looks empty. Nothing eliminates it, nothing matches it, and we dump every branch.Before (two-branch union, document with a bad
mode):After:
Now we follow both composing edges, a lone
$refandallOfmembership, transitively with a visited set, and read everything reached as one declaration. They all constrain the same document, so a property pinned by several of them is pinned to the intersection of their sets. Property schemas are still not followed, sochild: {$ref: node}stays out of the walk.The rest of the diff is tidy-up with no behaviour change, reviewable separately: the terms this code uses (pin, known property, discriminator) get one glossary at the top of
UnionNarrowing.kt, andnarrowByElimination, which also did presence matching, is split in three. Todo left onpinnedValues, which only sits on the validator interface because a schema keeps its compiled validators and not the declarations behind them.Four tests in
UnionNarrowingThroughAllOfTest. Each fails without the traversal.