fix: preserve multi-type type union when schema has subschemas (#954)#2
Merged
Merged
Conversation
…decomputer#954) When a schema object had both `type: [a, b, c, ...]` and one of `oneOf`/`anyOf`/`allOf`/`not` on the same level, convert_schema_object matched an earlier arm that wildcarded the `instance_type` field (flagged by a pre-existing TODO: "we probably shouldn't ignore this"). The subschema branches were then converted in isolation, silently discarding the outer type union. For the schema in issue oxidecomputer#954: { "type": ["string", "number", "boolean", "array"], "oneOf": [ { "items": { "type": "string" } }, { "items": { "type": "number" } }, { "items": { "type": "boolean" } } ] } typify produced an enum with only three array variants, losing the string/number/boolean alternatives entirely. The fix tightens the match pattern to `None | Some(Single(_))`, letting `Some(Vec(_))` fall through to the existing merge arm which already folds the outer schema into each subschema branch via `try_merge_with_subschemas`. Single-type + subschema behaviour is preserved, so tolerant handling of schemas whose outer type conflicts with a branch (e.g. the rust-collisions fixture) is unchanged. Adds a fixture with eight cases covering the affected code path: - `type:[...]` combined with oneOf, anyOf, allOf, and not - explicit `type: array` in every oneOf branch (primitives pruned) - partially and fully unsatisfiable oneOf - simultaneous oneOf + allOf - singleton-type regression guard (rust-collisions pattern) No existing workspace fixture exercised `type: [...]` alongside subschemas on the same object, so this fix had zero pre-existing regression coverage.
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
type: [a, b, c, ...]together withoneOf/anyOf/allOf/not, an earlier match arm inconvert_schema_objectwildcarded theinstance_typefield and dropped the outer type union. The subschema branches were then converted in isolation, producing an enum that silently lost the non-array (or non-subschema) alternatives.None | Some(Single(_))soSome(Vec(_))falls through to the existing merge arm, which folds the outer schema into each subschema branch viatry_merge_with_subschemas. Single-type + subschema behaviour is unchanged (rust-collisions fixture still passes).type-array-with-subschemas.json) covering:type:[...]combined withoneOf,anyOf,allOf, andnottype: arrayin everyoneOfbranch (primitives pruned)oneOfoneOf+allOforigin/main.Test plan
cargo test— new fixture passes, existing fixtures unchangedcargo clippy --all-targetscargo fmt --all