fix(schema): union allOf properties instead of intersecting them#3060
Open
schani wants to merge 5 commits into
Open
fix(schema): union allOf properties instead of intersecting them#3060schani wants to merge 5 commits into
schani wants to merge 5 commits into
Conversation
When allOf combined a $ref to a closed object schema (additionalProperties: false) with an inline closed object schema, the intersection accumulator dropped any property that wasn't declared by every branch, instead of unioning the declared properties across branches. additionalProperties: false should only restrict names outside that union, not cause named properties from one branch to be discarded because another branch didn't also declare them. This produced classes with missing (sometimes all) properties. Add a schema fixture (allof-closed-objects.schema) with a positive sample proving all properties from both allOf branches round-trip, and a negative sample proving the required "discriminator" property is still enforced. Fixes #1152 Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
…-defaults feature (#1152) The negative fixture sample allof-closed-objects.1.fail.json was named without a feature suffix, so test/fixtures.ts treated it as "every language must reject this input" regardless of that language's `features` list. Not all languages enforce JSON Schema `required` at deserialization time; Go in particular silently defaults a missing string field to "". Renaming it to allof-closed-objects.1.fail.no-defaults.json matches the existing convention used by required.1.fail.no-defaults.json, so the negative case only runs for languages generated with the no-defaults feature. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
# Conflicts: # packages/quicktype-core/src/rewrites/ResolveIntersections.ts
…orce required properties The allof-closed-objects.1.fail.no-defaults.json negative sample expects the generated program to reject an object missing the required "discriminator" property. Gating it by the no-defaults feature is not sufficient: several languages list no-defaults for other reasons yet do not enforce required properties at deserialization time, so the sample does not fail for them and the fixture reports an unexpected success. These are exactly the languages that already skip required.schema / intersection.schema for the same reason: - cjson: required properties absent are not checked - swift: on Linux the missing-required failure case doesn't fail - haskell: a failed decode prints "null" and exits 0, so expected-failure samples cannot be detected - elixir: struct keys cannot be enforced at runtime and are set to null Add allof-closed-objects.schema to their skipSchema lists, mirroring the existing required.schema handling. The positive sample and the negative sample still run for every language that does enforce required (csharp, cplusplus, python, rust, kotlin, scala3, typescript, typescript-zod, typescript-effect-schema, ...), keeping at least one positive and one negative case running. Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # test/languages.ts
Generated-output differences31 files differ — 0 modified, 31 new, 0 deleted |
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.
Bug
When a JSON Schema
allOfcombined a$refto a closed object schema(
additionalProperties: false) with an inline closed object schema, quicktypegenerated a class missing properties — in the reported case, all properties
were dropped.
Example (simplified from the issue):
{ "Dog": { "allOf": [ { "$ref": "#/definitions/Animal" }, { "type": "object", "additionalProperties": false, "properties": { "Bar": { "type": ["null", "string"] } } } ] }, "Animal": { "type": "object", "additionalProperties": false, "required": ["discriminator"], "properties": { "Foo": { "type": ["null", "string"] }, "discriminator": { "type": "string" } } } }AnimaldeclaresFoo/discriminator; the inline branch declaresBar.Generating C# from this schema produced:
Foo,discriminator, andBarwere all missing. RemovingadditionalProperties: falseworked around the bug (all properties thenmerged correctly), which pointed at the root cause.
Root cause
IntersectionAccumulator.updateObjectPropertiesinpackages/quicktype-core/src/rewrites/ResolveIntersections.tscomputes thecombined property set for an
allOfintersection. For any property name thatappeared in the accumulated set but not in the current branch (or vice versa),
the old code deleted it from the accumulator instead of keeping it — i.e. it
intersected the named property sets across branches, rather than unioning
them.
additionalProperties: falseis only supposed to control whether namesoutside that union are permitted; it should not cause a property declared by
only one
allOfbranch to be dropped entirely.Fix
Named properties from every
allOfbranch are now always unioned into theresult.
additionalProperties/closedness is still combined separately (asbefore) and continues to determine whether names outside the union are
allowed — so the type produced from all-closed branches is still closed
overall, it just no longer discards properties that only one branch declared.
Test coverage
Added
test/inputs/schema/allof-closed-objects.schema, a JSON Schema fixturereproducing the bug (closed
$refbase + closed inline object combined viaallOf), with:allof-closed-objects.1.json— a positive sample proving properties fromboth
allOfbranches (including the required field on the$refbranch)round-trip correctly.
allof-closed-objects.1.fail.json— a negative sample (omits the requireddiscriminatorfield) proving required-property enforcement still works.This is discovered automatically by every registered
JSONSchemaFixture(pertest/fixtures.ts), so it now runs across all schema-input languages in CI.Verification
(
node dist/index.js --src-lang schema --lang cs container.schema):confirmed
Dogwas generated empty.Dogis now generated withFoo,Discriminator, andBar.npm run buildpasses.QUICKTEST=true FIXTURE=schema-typescript script/testlocally (72fixtures, all passing, including the new
allof-closed-objects.schemawithboth its positive and negative samples).
sandbox; CI will validate those.
Fixes #1152
🤖 Generated with Claude Code