Skip to content

fix(schema): union allOf properties instead of intersecting them#3060

Open
schani wants to merge 5 commits into
masterfrom
agent/fix-issue-1152-v2
Open

fix(schema): union allOf properties instead of intersecting them#3060
schani wants to merge 5 commits into
masterfrom
agent/fix-issue-1152-v2

Conversation

@schani

@schani schani commented Jul 20, 2026

Copy link
Copy Markdown
Member

Bug

When a JSON Schema allOf combined a $ref to a closed object schema
(additionalProperties: false) with an inline closed object schema, quicktype
generated 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" }
    }
  }
}

Animal declares Foo/discriminator; the inline branch declares Bar.
Generating C# from this schema produced:

public partial class Dog
{
}

Foo, discriminator, and Bar were all missing. Removing
additionalProperties: false worked around the bug (all properties then
merged correctly), which pointed at the root cause.

Root cause

IntersectionAccumulator.updateObjectProperties in
packages/quicktype-core/src/rewrites/ResolveIntersections.ts computes the
combined property set for an allOf intersection. For any property name that
appeared 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: false is only supposed to control whether names
outside that union are permitted; it should not cause a property declared by
only one allOf branch to be dropped entirely.

Fix

Named properties from every allOf branch are now always unioned into the
result. additionalProperties/closedness is still combined separately (as
before) 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 fixture
reproducing the bug (closed $ref base + closed inline object combined via
allOf), with:

  • allof-closed-objects.1.json — a positive sample proving properties from
    both allOf branches (including the required field on the $ref branch)
    round-trip correctly.
  • allof-closed-objects.1.fail.json — a negative sample (omits the required
    discriminator field) proving required-property enforcement still works.

This is discovered automatically by every registered JSONSchemaFixture (per
test/fixtures.ts), so it now runs across all schema-input languages in CI.

Verification

  • Reproduced the bug on unfixed master using the repro from the issue
    (node dist/index.js --src-lang schema --lang cs container.schema):
    confirmed Dog was generated empty.
  • Applied the fix; re-ran the same repro — Dog is now generated with
    Foo, Discriminator, and Bar.
  • npm run build passes.
  • Ran QUICKTEST=true FIXTURE=schema-typescript script/test locally (72
    fixtures, all passing, including the new allof-closed-objects.schema with
    both its positive and negative samples).
  • Other language fixtures (C#, etc.) require toolchains not available in this
    sandbox; CI will validate those.

Fixes #1152

🤖 Generated with Claude Code

schani and others added 5 commits July 20, 2026 19:32
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>
@github-actions

Copy link
Copy Markdown

Generated-output differences

31 files differ — 0 modified, 31 new, 0 deleted
2470 changed lines — +2470 / −0

Open the generated-output report →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generating dose not work or I do something wrong

1 participant