-
Notifications
You must be signed in to change notification settings - Fork 107
fix: preserve multi-type type union when schema has subschemas (#954)
#1001
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ndreno
wants to merge
1
commit into
oxidecomputer:main
Choose a base branch
from
barbacane-dev:fix/issue-954-type-union-with-subschemas-clean
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,029
−3
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| { | ||
| "$schema": "http://json-schema.org/draft-07/schema#", | ||
| "$comment": "Regression coverage for issue #954: schemas with a multi-type `type: [...]` array on the same object as `oneOf` / `anyOf` / `allOf` / `not` previously discarded the `type` constraint (TODO at convert.rs wildcarded `instance_type`). Single-type + subschema cases must still pass through the earlier arm unchanged.", | ||
| "definitions": { | ||
| "TypeArrayOneOfItems": { | ||
| "$comment": "Canonical issue #954 shape. Each oneOf branch only constrains `items`, so the type union must be folded into every branch rather than dropped.", | ||
| "type": [ "string", "number", "boolean", "array" ], | ||
| "oneOf": [ | ||
| { "items": { "type": "string" } }, | ||
| { "items": { "type": "number" } }, | ||
| { "items": { "type": "boolean" } } | ||
| ] | ||
| }, | ||
| "TypeArrayAnyOfItems": { | ||
| "$comment": "Same shape as TypeArrayOneOfItems but using anyOf. anyOf travels through try_merge_with_each_subschema on a sibling path from oneOf; it should fold the type union the same way.", | ||
| "type": [ "string", "number", "array" ], | ||
| "anyOf": [ | ||
| { "items": { "type": "string" } }, | ||
| { "items": { "type": "number" } } | ||
| ] | ||
| }, | ||
| "TypeArrayAllOfRefinement": { | ||
| "$comment": "allOf is folded pairwise into the parent rather than producing branches. The type union must survive and the array-only constraints should apply when the Array variant is selected.", | ||
| "type": [ "string", "array" ], | ||
| "allOf": [ | ||
| { "items": { "type": "string" } }, | ||
| { "minItems": 1 } | ||
| ] | ||
| }, | ||
| "TypeArrayNotExclusion": { | ||
| "$comment": "not: object is redundant when the outer type union excludes object, but merging must not drop the type union when the not branch is applied.", | ||
| "type": [ "string", "number", "array" ], | ||
| "not": { "type": "object" } | ||
| }, | ||
| "SingleTypeOneOfArrayBranch": { | ||
| "$comment": "Regression guard (rust-collisions pattern). Outer singleton type + oneOf where one branch has a conflicting explicit type. This must continue to pass through the earlier arm (no merge), otherwise the array branch becomes unsatisfiable and is silently dropped.", | ||
| "type": "object", | ||
| "oneOf": [ | ||
| { | ||
| "type": "object", | ||
| "properties": { "kind": { "type": "string" } }, | ||
| "required": [ "kind" ] | ||
| }, | ||
| { | ||
| "type": "array", | ||
| "items": { "type": "string" }, | ||
| "minItems": 2, | ||
| "maxItems": 2 | ||
| } | ||
| ] | ||
| }, | ||
| "TypeArrayOneOfExplicitArrayBranches": { | ||
| "$comment": "Case 7: each oneOf branch pins `type: array`, so the intersection with the outer type union must prune the non-array primitives. Only array variants should be emitted.", | ||
| "type": [ "string", "array" ], | ||
| "oneOf": [ | ||
| { "type": "array", "items": { "type": "string" } }, | ||
| { "type": "array", "items": { "type": "number" } } | ||
| ] | ||
| }, | ||
| "TypeArrayPartiallyUnsatisfiableOneOf": { | ||
| "$comment": "Some oneOf branches conflict with the outer type union and should be dropped during merge; the surviving branch must carry the outer type union. The two eliminated branches use object/number which the outer `[string, array]` disallows.", | ||
| "type": [ "string", "array" ], | ||
| "oneOf": [ | ||
| { "type": "object", "properties": { "name": { "type": "string" } } }, | ||
| { "items": { "type": "string" } }, | ||
| { "type": "number" } | ||
| ] | ||
| }, | ||
| "TypeArrayFullyUnsatisfiableOneOf": { | ||
| "$comment": "Case 9: every branch conflicts with the outer type union, so `try_merge_with_each_subschema` returns empty and the schema resolves to never. Must emit an empty enum cleanly rather than panic.", | ||
| "type": [ "string", "number" ], | ||
| "oneOf": [ | ||
| { "type": "array", "items": { "type": "string" } }, | ||
| { "type": "object", "properties": { "k": { "type": "string" } } } | ||
| ] | ||
| }, | ||
| "TypeArrayOneOfAndAllOf": { | ||
| "$comment": "Case 10: oneOf and allOf on the same object, both alongside a multi-type `type` array. Exercises the full merge path (allOf folded first, then oneOf fanned out) with the Vec instance_type flowing through the merge arm.", | ||
| "type": [ "string", "array" ], | ||
| "allOf": [ | ||
| { "minLength": 1 } | ||
| ], | ||
| "oneOf": [ | ||
| { "items": { "type": "string" } }, | ||
| { "items": { "type": "number" } } | ||
| ] | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.