Improve error message for subschema's with discriminator#398
Open
holodorum wants to merge 5 commits into
Open
Conversation
A oneOf whose branches are keyed by a shared property pinned to distinct
const values (a discriminated union) produced an unusable error when no
branch matched: every branch's errors were dumped together. For a real
schema with ~115 task types this meant a wall of "must equal X" lines,
burying the actual mistake.
When the oneOf is a discriminated union, report against the branch the
document's discriminator value selects instead:
- value matches one branch's const -> report only that branch's errors,
which is where the real failure lives (e.g. a missing required field)
- value matches no branch and every branch pins the discriminator
(a closed union) -> one "must be one of ..." error at the value
- value matches no branch but a wildcard/negative branch leaves the
union open -> fall back to the existing per-branch dump, since we
can't prove the value invalid
Detection tolerates branches that don't pin the property (wildcard,
negative, or non-object branches) and requires the discriminator's consts
to be distinct, so a property with repeated consts is not mistaken for
the discriminator.
The fallback that lists every sub-schema's errors packed them onto one
line as `'Label': ['msg' at l.c,...]`, which is hard to scan when several
branches each fail several checks. Render it as a bulleted list instead:
one bullet per sub-schema, each error on an indented sub-bullet with a
`(line:column)` location.
Also add a discriminator test for the `{ type: string, const: X }`
property shape (a const with a sibling type), the form real schemas use,
which the existing tests didn't cover.
Discriminator-aware reporting previously recognized only `const`-pinned
properties and lived only in oneOf. Generalize it:
- a new JsonSchemaValidator.pinnedValues() exposes the finite value set
a validator restricts to, implemented by ConstValidator (its const)
and EnumValidator (its members). Detection now works on value sets,
requiring branches' sets to be pairwise disjoint, so single- and
multi-value enum discriminators are handled alongside const. This also
lets ConstValidator.const go back to private.
- the detect-and-select logic moves to a shared reportDiscriminatedUnion-
Error helper used by both OneOfValidator and AnyOfValidator, so an
anyOf-modeled discriminated union gets the same focused error instead
of dumping every branch.
if/then and allOf already report focused errors and are untouched; $ref
and non-value-pinned (structural) unions keep falling back to the dump.
Discriminated unions are overwhelmingly written as `oneOf`/`anyOf` of `$ref`s into `$defs`, but detection only inspected a branch's own `properties`, so a `$ref` branch surfaced no discriminator pins and every such union fell back to the full per-branch error dump. Resolve a lone-`$ref` branch a single level through the ref and read the target's own pins, so `$ref`-based unions get the same focused reporting as inline ones. Resolution is one hop only: `ownPinnedProperties` reads a schema's own validators without ever following a `$ref`, so a target that is itself only another `$ref` contributes no pins and the union degrades to the dump — the dominant single-hop shape is covered without chasing alias chains or risking unbounded recursion. The sole-`$ref` check is shared with branchName via a small soleRefValidator helper.
When a oneOf/anyOf has no value-based discriminator (const/enum) to key on, fall back to a presence-based tier that reports only the branches the document plausibly targets instead of dumping every branch. A branch matches when the document carries a "distinguishing" property — one known (declared or required) by some branches but not all. Matching on known properties rather than required alone means an optional-but-declared property like `region` correctly points at every branch that accepts it, not just the one that requires it. When the match is empty or covers all branches, we fall back to the full dump; value-based discrimination keeps priority.
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.
The error messages for
oneOfandanyOfschema don't improve when there is more information or the existing document filters to only a subset of available subschema's. For example, when a schema contains theoneOf-construct this type of error message is shown:After using the discriminator (e.g.
kind: "kubernetes") the error message stays still lists all subschema's.This pull request improves that. Without using a discriminator we show the following error message:
After discriminating to one of the schema's (e.g.
kind:"kubernetes"), this becomes:The correct subschema can be discriminated through
enum's,const, andproperties.