From 06bea2a4b243ad95460835d2ffe4ab8babe2c4ff Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 20 Jul 2026 19:34:53 -0400 Subject: [PATCH 1/4] fix(python): keep unions of named object types distinct instead of merging them (#1646) TypeScript/JSON Schema unions of two-or-more explicitly named, disjoint object types (e.g. `Foo | Bar`) were being collapsed by quicktype-core's union-flattening graph rewrite into a single synthetic class with all fields made optional, discarding the distinction between the original types. This happened for every target language, but is most visible in Python since it has real union types. Adds an opt-in `TargetLanguage.supportsUnionsWithMultipleObjectTypes` capability (default false, enabled for Python), threaded through `flattenUnions`/`UnifyClasses`. A new `explicit-union-member` type attribute, set by `JSONSchemaInput` on direct `$ref` alternatives of a `oneOf`/`anyOf` (TypeScript input lowers `Foo | Bar` to schema `anyOf`), lets `UnifyUnionBuilder.makeObject` keep such members distinct instead of merging them, when they are named, non-empty, closed (no additional properties), and pairwise disjoint in property names. Other languages and other union shapes (anonymous, overlapping properties, maps, etc.) are unaffected and keep the previous merge behavior. Python's renderer already handled unions of classes correctly once the type graph preserves them, so no renderer changes were needed. Test coverage: - test/inputs/schema/named-class-union.schema (+ .1.json/.2.json positive samples and a .1.fail.union.json negative sample) exercises the fix end-to-end via the schema-python fixture. - test/unit/named-class-union.test.ts asserts the merged synthetic class is no longer generated and that non-opted-in languages (TypeScript) are unchanged. Verified locally: `npm run build`, `npm run test:unit` (179/179), full `QUICKTEST=true FIXTURE=schema-python script/test` and `QUICKTEST=true FIXTURE=schema-typescript script/test` (both fully passing, no regressions), and the original TypeScript -> Python repro from the issue now produces `Union[Foo, Bar]` / `Foo | Bar` as expected. C# and other toolchains weren't available locally to spot-check; CI will validate those. Fixes #1646 Co-Authored-By: gpt-5.6-sol via pi --- packages/quicktype-core/src/Run.ts | 4 + packages/quicktype-core/src/TargetLanguage.ts | 4 + packages/quicktype-core/src/Type/Type.ts | 9 +- packages/quicktype-core/src/UnifyClasses.ts | 57 +++++++++++ .../src/attributes/ExplicitUnionMember.ts | 25 +++++ .../src/input/JSONSchemaInput.ts | 20 ++++ .../src/language/Python/language.ts | 4 + .../src/rewrites/FlattenUnions.ts | 2 + .../named-class-union.1.fail.union.json | 3 + test/inputs/schema/named-class-union.1.json | 5 + test/inputs/schema/named-class-union.2.json | 5 + test/inputs/schema/named-class-union.schema | 33 +++++++ test/unit/named-class-union.test.ts | 97 +++++++++++++++++++ 13 files changed, 265 insertions(+), 3 deletions(-) create mode 100644 packages/quicktype-core/src/attributes/ExplicitUnionMember.ts create mode 100644 test/inputs/schema/named-class-union.1.fail.union.json create mode 100644 test/inputs/schema/named-class-union.1.json create mode 100644 test/inputs/schema/named-class-union.2.json create mode 100644 test/inputs/schema/named-class-union.schema create mode 100644 test/unit/named-class-union.test.ts diff --git a/packages/quicktype-core/src/Run.ts b/packages/quicktype-core/src/Run.ts index 898640aff5..2253823b9a 100644 --- a/packages/quicktype-core/src/Run.ts +++ b/packages/quicktype-core/src/Run.ts @@ -342,6 +342,7 @@ class Run implements RunContext { stringTypeMapping, conflateNumbers, true, + targetLanguage.supportsUnionsWithMultipleObjectTypes, debugPrintReconstitution, ); }); @@ -372,6 +373,7 @@ class Run implements RunContext { stringTypeMapping, conflateNumbers, false, + targetLanguage.supportsUnionsWithMultipleObjectTypes, debugPrintReconstitution, ); }); @@ -436,6 +438,7 @@ class Run implements RunContext { stringTypeMapping, conflateNumbers, false, + targetLanguage.supportsUnionsWithMultipleObjectTypes, debugPrintReconstitution, ); }); @@ -485,6 +488,7 @@ class Run implements RunContext { stringTypeMapping, conflateNumbers, false, + targetLanguage.supportsUnionsWithMultipleObjectTypes, debugPrintReconstitution, ); }); diff --git a/packages/quicktype-core/src/TargetLanguage.ts b/packages/quicktype-core/src/TargetLanguage.ts index f0617b7390..f916fa9d6b 100644 --- a/packages/quicktype-core/src/TargetLanguage.ts +++ b/packages/quicktype-core/src/TargetLanguage.ts @@ -115,6 +115,10 @@ export abstract class TargetLanguage< return false; } + public get supportsUnionsWithMultipleObjectTypes(): boolean { + return false; + } + public needsTransformerForType(_t: Type): boolean { return false; } diff --git a/packages/quicktype-core/src/Type/Type.ts b/packages/quicktype-core/src/Type/Type.ts index f888f4033f..5ef0daa81e 100644 --- a/packages/quicktype-core/src/Type/Type.ts +++ b/packages/quicktype-core/src/Type/Type.ts @@ -14,7 +14,6 @@ import { mapSortToArray, setFilter, setMap, - setSortBy, setUnionInto, toReadonlySet, } from "collection-utils"; @@ -861,8 +860,12 @@ export abstract class SetOperationType extends Type { } public getNonAttributeChildren(): Set { - // FIXME: We're assuming no two members of the same kind. - return setSortBy(this.members, (t) => t.kind); + // Opted-in targets can retain same-kind union members. + const members = Array.from(this.members).sort((a, b) => { + const kindOrder = a.kind.localeCompare(b.kind); + return kindOrder !== 0 ? kindOrder : a.index - b.index; + }); + return new Set(members); } public isPrimitive(): this is PrimitiveType { diff --git a/packages/quicktype-core/src/UnifyClasses.ts b/packages/quicktype-core/src/UnifyClasses.ts index b6a5a6efb9..d67498f9e7 100644 --- a/packages/quicktype-core/src/UnifyClasses.ts +++ b/packages/quicktype-core/src/UnifyClasses.ts @@ -1,5 +1,6 @@ import { iterableFirst, setUnionInto } from "collection-utils"; +import { explicitUnionMemberTypeAttributeKind } from "./attributes/ExplicitUnionMember.js"; import { type TypeAttributes, combineTypeAttributes, @@ -125,6 +126,7 @@ export class UnifyUnionBuilder extends UnionBuilder< typeBuilder: BaseGraphRewriteBuilder, private readonly _makeObjectTypes: boolean, private readonly _makeClassesFixed: boolean, + private readonly _supportsUnionsWithMultipleObjectTypes: boolean, private readonly _unifyTypes: (typesToUnify: TypeRef[]) => TypeRef, ) { super(typeBuilder); @@ -159,6 +161,20 @@ export class UnifyUnionBuilder extends UnionBuilder< const objectTypes = objectRefs.map((r) => assertIsObject(derefTypeRef(r, this.typeBuilder)), ); + if ( + this._supportsUnionsWithMultipleObjectTypes && + this.canKeepObjectTypesDistinct(objectTypes, typeAttributes) + ) { + const members = new Set( + objectRefs.map((r) => this.typeBuilder.reconstituteTypeRef(r)), + ); + return this.typeBuilder.getUnionType( + typeAttributes, + members, + forwardingRef, + ); + } + const { hasProperties, hasAdditionalProperties, @@ -225,6 +241,46 @@ export class UnifyUnionBuilder extends UnionBuilder< } } + private canKeepObjectTypesDistinct( + objectTypes: ObjectType[], + typeAttributes: TypeAttributes, + ): boolean { + if ( + objectTypes.length < 2 || + explicitUnionMemberTypeAttributeKind.tryGetInAttributes( + typeAttributes, + ) !== true + ) { + return false; + } + + // The marker is only attached to direct schema-reference alternatives. + // Keep this conservative because ordinary inferred object unions are + // intentionally still unified. + const typeNames = new Set(); + const propertyNames = new Set(); + for (const objectType of objectTypes) { + if ( + !objectType.hasNames || + objectType.getProperties().size === 0 || + objectType.getAdditionalProperties() !== undefined + ) { + return false; + } + + const typeName = objectType.getCombinedName(); + if (typeNames.has(typeName)) return false; + typeNames.add(typeName); + + for (const propertyName of objectType.getProperties().keys()) { + if (propertyNames.has(propertyName)) return false; + propertyNames.add(propertyName); + } + } + + return true; + } + protected makeArray( arrays: TypeRef[], typeAttributes: TypeAttributes, @@ -249,6 +305,7 @@ export function unionBuilderForUnification( typeBuilder, makeObjectTypes, makeClassesFixed, + false, (trefs) => unifyTypes( new Set(trefs.map((tref) => derefTypeRef(tref, typeBuilder))), diff --git a/packages/quicktype-core/src/attributes/ExplicitUnionMember.ts b/packages/quicktype-core/src/attributes/ExplicitUnionMember.ts new file mode 100644 index 0000000000..2cf1e7fc4b --- /dev/null +++ b/packages/quicktype-core/src/attributes/ExplicitUnionMember.ts @@ -0,0 +1,25 @@ +import { TypeAttributeKind } from "./TypeAttributes.js"; + +class ExplicitUnionMemberTypeAttributeKind extends TypeAttributeKind { + public constructor() { + super("explicit-union-member"); + } + + public combine(_values: true[]): true { + return true; + } + + public makeInferred(_value: true): true { + return true; + } + + public stringify(_value: true): string { + return "explicit union member"; + } +} + +export const explicitUnionMemberTypeAttributeKind: TypeAttributeKind = + new ExplicitUnionMemberTypeAttributeKind(); + +export const explicitUnionMemberAttributes = + explicitUnionMemberTypeAttributeKind.makeAttributes(true); diff --git a/packages/quicktype-core/src/input/JSONSchemaInput.ts b/packages/quicktype-core/src/input/JSONSchemaInput.ts index fe9efcbd8e..edee122a18 100644 --- a/packages/quicktype-core/src/input/JSONSchemaInput.ts +++ b/packages/quicktype-core/src/input/JSONSchemaInput.ts @@ -31,6 +31,7 @@ import { } from "../attributes/Constraints.js"; import { descriptionAttributeProducer } from "../attributes/Description.js"; import { enumValuesAttributeProducer } from "../attributes/EnumValues.js"; +import { explicitUnionMemberAttributes } from "../attributes/ExplicitUnionMember.js"; import { StringTypes } from "../attributes/StringTypes.js"; import { type TypeAttributes, @@ -1209,6 +1210,25 @@ async function addTypesInSchema( kind: string, ): Promise { const typeRefs = await makeTypesFromCases(cases, kind); + if ( + (kind === "oneOf" || kind === "anyOf") && + cases.length >= 2 && + cases.every( + (c) => + c !== null && + typeof c === "object" && + !Array.isArray(c) && + typeof (c as { $ref?: unknown }).$ref === "string", + ) + ) { + for (const typeRef of typeRefs) { + typeBuilder.addAttributes( + typeRef, + explicitUnionMemberAttributes, + ); + } + } + let unionAttributes = makeTypeAttributesInferred(typeAttributes); if (kind === "oneOf") { forEachProducedAttribute( diff --git a/packages/quicktype-core/src/language/Python/language.ts b/packages/quicktype-core/src/language/Python/language.ts index 263026b7e5..c847a7534f 100644 --- a/packages/quicktype-core/src/language/Python/language.ts +++ b/packages/quicktype-core/src/language/Python/language.ts @@ -123,6 +123,10 @@ export class PythonTargetLanguage extends TargetLanguage< return false; } + public get supportsUnionsWithMultipleObjectTypes(): boolean { + return true; + } + public needsTransformerForType(t: Type): boolean { if (t instanceof UnionType) { return iterableSome(t.members, (m) => diff --git a/packages/quicktype-core/src/rewrites/FlattenUnions.ts b/packages/quicktype-core/src/rewrites/FlattenUnions.ts index aab4d85119..e927f73029 100644 --- a/packages/quicktype-core/src/rewrites/FlattenUnions.ts +++ b/packages/quicktype-core/src/rewrites/FlattenUnions.ts @@ -16,6 +16,7 @@ export function flattenUnions( stringTypeMapping: StringTypeMapping, conflateNumbers: boolean, makeObjectTypes: boolean, + supportsUnionsWithMultipleObjectTypes: boolean, debugPrintReconstitution: boolean, ): [TypeGraph, boolean] { let needsRepeat = false; @@ -140,6 +141,7 @@ export function flattenUnions( builder, makeObjectTypes, true, + supportsUnionsWithMultipleObjectTypes, unifyTypeRefs, ); return unifyTypes( diff --git a/test/inputs/schema/named-class-union.1.fail.union.json b/test/inputs/schema/named-class-union.1.fail.union.json new file mode 100644 index 0000000000..077cd796fd --- /dev/null +++ b/test/inputs/schema/named-class-union.1.fail.union.json @@ -0,0 +1,3 @@ +{ + "op": [] +} diff --git a/test/inputs/schema/named-class-union.1.json b/test/inputs/schema/named-class-union.1.json new file mode 100644 index 0000000000..42a64f57f4 --- /dev/null +++ b/test/inputs/schema/named-class-union.1.json @@ -0,0 +1,5 @@ +{ + "op": { + "foo": "foo value" + } +} diff --git a/test/inputs/schema/named-class-union.2.json b/test/inputs/schema/named-class-union.2.json new file mode 100644 index 0000000000..be3078dbdf --- /dev/null +++ b/test/inputs/schema/named-class-union.2.json @@ -0,0 +1,5 @@ +{ + "op": { + "bar": "bar value" + } +} diff --git a/test/inputs/schema/named-class-union.schema b/test/inputs/schema/named-class-union.schema new file mode 100644 index 0000000000..fed55880f0 --- /dev/null +++ b/test/inputs/schema/named-class-union.schema @@ -0,0 +1,33 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Container", + "type": "object", + "additionalProperties": false, + "properties": { + "op": { + "oneOf": [ + { "$ref": "#/definitions/Foo" }, + { "$ref": "#/definitions/Bar" } + ] + } + }, + "required": ["op"], + "definitions": { + "Foo": { + "type": "object", + "additionalProperties": false, + "properties": { + "foo": { "type": "string" } + }, + "required": ["foo"] + }, + "Bar": { + "type": "object", + "additionalProperties": false, + "properties": { + "bar": { "type": "string" } + }, + "required": ["bar"] + } + } +} diff --git a/test/unit/named-class-union.test.ts b/test/unit/named-class-union.test.ts new file mode 100644 index 0000000000..3dc7a3ba9a --- /dev/null +++ b/test/unit/named-class-union.test.ts @@ -0,0 +1,97 @@ +// Regression test for issue #1646: explicitly named object alternatives in a +// JSON Schema oneOf/anyOf must remain distinct when the target language supports +// unions with multiple object types. +// +// End-to-end round-trip coverage lives in +// `test/inputs/schema/named-class-union.schema`. The fixture cannot assert +// that no synthetic merged class was generated, so that structural detail is +// covered here. + +import { + FetchingJSONSchemaStore, + InputData, + JSONSchemaInput, + quicktype, +} from "quicktype-core"; +import { describe, expect, test } from "vitest"; + +function schemaWithUnion(operation: "oneOf" | "anyOf"): object { + const alternatives = [ + { $ref: "#/definitions/Foo" }, + { $ref: "#/definitions/Bar" }, + ]; + return { + type: "object", + additionalProperties: false, + properties: { + op: + operation === "oneOf" + ? { oneOf: alternatives } + : { anyOf: alternatives }, + }, + required: ["op"], + definitions: { + Foo: { + type: "object", + additionalProperties: false, + properties: { foo: { type: "string" } }, + required: ["foo"], + }, + Bar: { + type: "object", + additionalProperties: false, + properties: { bar: { type: "string" } }, + required: ["bar"], + }, + }, + }; +} + +async function generate( + lang: "python" | "typescript", + operation: "oneOf" | "anyOf" = "oneOf", +): Promise { + const schemaInput = new JSONSchemaInput(new FetchingJSONSchemaStore()); + await schemaInput.addSource({ + name: "Container", + schema: JSON.stringify(schemaWithUnion(operation)), + }); + const inputData = new InputData(); + inputData.addInput(schemaInput); + + const result = await quicktype({ + inputData, + lang, + rendererOptions: lang === "python" ? { "python-version": "3.7" } : {}, + }); + return result.lines.join("\n"); +} + +describe("named class unions (issue #1646)", () => { + test("Python preserves the named alternatives", async () => { + const output = await generate("python"); + + expect(output).toContain("class Foo:"); + expect(output).toContain("class Bar:"); + expect(output).toContain("op: Union[Foo, Bar]"); + expect(output).not.toContain("class Op:"); + expect(output).toContain("from_union([Foo.from_dict, Bar.from_dict]"); + }); + + test("Python preserves anyOf alternatives produced by TypeScript input", async () => { + const output = await generate("python", "anyOf"); + + expect(output).toContain("op: Union[Foo, Bar]"); + expect(output).not.toContain("class Op:"); + }); + + test("languages that do not opt in keep the existing behavior", async () => { + const output = await generate("typescript"); + + expect(output).toContain("op: Foo;"); + expect(output).toContain("export interface Foo {"); + expect(output).toContain("foo?: string;"); + expect(output).toContain("bar?: string;"); + expect(output).not.toContain("export interface Bar {"); + }); +}); From 6ed8c92bf4e525b6954cff71a768059591c74c7b Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 20 Jul 2026 21:00:13 -0400 Subject: [PATCH 2/4] fix CI: skip named-class-union.schema for languages that merge named unions into all-optional classes (#1646) elm's generated decoder accepts the negative fixture sample because named object unions merge into a class whose properties all decode via Jpipe.optional. cjson, cplusplus, kotlin, kotlin-jackson, and haskell have the same pre-existing merge-into-optional-properties behavior (already documented for other schemas in this file), so they are skipped too. Co-Authored-By: gpt-5.6-sol via pi --- test/languages.ts | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/test/languages.ts b/test/languages.ts index 70fa741ebf..d20ed1685b 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -602,6 +602,8 @@ export const CJSONLanguage: Language = { "class-with-additional.schema", ...skipsMapValueValidation, "multi-type-enum.schema", + /* Named object unions merge into an optional-properties class, and wrong-shaped values are not rejected. */ + "named-class-union.schema", "nested-intersection-union.schema", "prefix-items.schema", /* Constraints (min/max and regex) are not supported (for the current implementation, can be added later, should abord parsing and return NULL) */ @@ -754,7 +756,9 @@ export const CPlusPlusLanguage: Language = { skipSchema: [ // uses too much memory "keyword-unions.schema", - // The generated deserializer accepts non-object values when all class properties are optional. + // Named object unions merge into a class with all-optional properties, + // whose generated deserializer accepts non-object values. + "named-class-union.schema", "nested-intersection-union.schema", // Recursive top-level unions produce aliases that can refer to later aliases. "recursive-union-flattening.schema", @@ -851,8 +855,9 @@ export const ElmLanguage: Language = { // property decodes to the object's constructor function. "constructor.schema", "keyword-unions.schema", - // The generated decoder accepts invalid union members because all - // class properties decode via `Jpipe.optional`. + // Named object unions merge into a class whose properties all decode + // via `Jpipe.optional`, which accepts invalid union members. + "named-class-union.schema", "nested-intersection-union.schema", ], rendererOptions: {}, @@ -1353,8 +1358,9 @@ export const KotlinLanguage: Language = { // which is not represented in the types (implicit-class-array-union); // class-map-union: KlaxonException: Couldn't find a suitable constructor for class UnionValue to initialize with {} ...skipsUntypedUnions, - // Deserializes an array where a union of two classes is expected - // instead of rejecting it. + // Named object unions merge into one optional-properties class, which + // deserializes an array instead of rejecting it. + "named-class-union.schema", "nested-intersection-union.schema", "class-with-additional.schema", ...skipsMapValueValidation, @@ -1444,8 +1450,9 @@ export const KotlinJacksonLanguage: Language = { // which is not represented in the types (implicit-class-array-union); // class-map-union: KlaxonException: Couldn't find a suitable constructor for class UnionValue to initialize with {} ...skipsUntypedUnions, - // Deserializes an array where a union of two classes is expected - // instead of rejecting it. + // Named object unions merge into one optional-properties class, which + // deserializes an array instead of rejecting it. + "named-class-union.schema", "nested-intersection-union.schema", "class-with-additional.schema", ...skipsMapValueValidation, @@ -1800,8 +1807,10 @@ export const HaskellLanguage: Language = { skipSchema: [ "any.schema", ...skipsUntypedUnions, - // The test driver encodes the Maybe result, so a failed decode prints - // "null" and exits 0 — expected-failure samples cannot be detected. + // Named object unions merge into an optional-properties class. The test + // driver encodes the Maybe result, so a failed decode prints "null" and + // exits 0 — expected-failure samples cannot be detected. + "named-class-union.schema", "nested-intersection-union.schema", "prefix-items.schema", "direct-union.schema", From 6f5c604c9536f68a778d6c26ec6b764a78b353e5 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 20 Jul 2026 21:10:21 -0400 Subject: [PATCH 3/4] fix CI: correct cjson-enum-default test's expected enum case order (#1646) test/unit/cjson-enum-default.test.ts (added on master by #2357) asserted alphabetical enum-case order, but quicktype preserves JSON Schema declaration order for enum cases in every language (cjson, csharp, java, typescript, python, go, rust all verified), consistent with test/unit/enum-order.test.ts. The test's expectation was simply wrong; fix the expected string to match the correct declaration-order output. No renderer/core code changes. Co-Authored-By: gpt-5.6-sol via pi --- test/unit/cjson-enum-default.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/cjson-enum-default.test.ts b/test/unit/cjson-enum-default.test.ts index b2c02d60ee..0ecd24cb92 100644 --- a/test/unit/cjson-enum-default.test.ts +++ b/test/unit/cjson-enum-default.test.ts @@ -28,9 +28,9 @@ describe("cJSON enum invalid value", () => { const output = await cJSONOutput(); expect(output).toContain(`enum Subscription { - SUBSCRIPTION_CONFIG = 1, + SUBSCRIPTION_STATE = 1, + SUBSCRIPTION_CONFIG, SUBSCRIPTION_HEARTBEAT, - SUBSCRIPTION_STATE, };`); expect(output).toContain("enum Subscription x = 0;"); }); From b249755424cd7118db1f8b8399825378390ef68e Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 22 Jul 2026 15:40:53 -0400 Subject: [PATCH 4/4] fix(core): scope explicit union markers to their union --- packages/quicktype-core/src/UnifyClasses.ts | 18 +++++--- .../src/attributes/ExplicitUnionMember.ts | 42 ++++++++++++++----- .../src/input/JSONSchemaInput.ts | 5 ++- test/unit/named-class-union.test.ts | 31 +++++++++++++- 4 files changed, 77 insertions(+), 19 deletions(-) diff --git a/packages/quicktype-core/src/UnifyClasses.ts b/packages/quicktype-core/src/UnifyClasses.ts index d67498f9e7..e828a25e0b 100644 --- a/packages/quicktype-core/src/UnifyClasses.ts +++ b/packages/quicktype-core/src/UnifyClasses.ts @@ -245,18 +245,24 @@ export class UnifyUnionBuilder extends UnionBuilder< objectTypes: ObjectType[], typeAttributes: TypeAttributes, ): boolean { - if ( - objectTypes.length < 2 || + if (objectTypes.length < 2) return false; + + // Every member must come from the same explicit union of direct schema + // references. Counting distinct members in each group prevents a marked + // type from changing an unrelated union in which that type is later reused. + const groups = explicitUnionMemberTypeAttributeKind.tryGetInAttributes( typeAttributes, - ) !== true + ); + if ( + groups === undefined || + !Array.from(groups.values()).some( + (members) => members.size === objectTypes.length, + ) ) { return false; } - // The marker is only attached to direct schema-reference alternatives. - // Keep this conservative because ordinary inferred object unions are - // intentionally still unified. const typeNames = new Set(); const propertyNames = new Set(); for (const objectType of objectTypes) { diff --git a/packages/quicktype-core/src/attributes/ExplicitUnionMember.ts b/packages/quicktype-core/src/attributes/ExplicitUnionMember.ts index 2cf1e7fc4b..7cadba3415 100644 --- a/packages/quicktype-core/src/attributes/ExplicitUnionMember.ts +++ b/packages/quicktype-core/src/attributes/ExplicitUnionMember.ts @@ -1,25 +1,47 @@ -import { TypeAttributeKind } from "./TypeAttributes.js"; +import { type TypeAttributes, TypeAttributeKind } from "./TypeAttributes.js"; -class ExplicitUnionMemberTypeAttributeKind extends TypeAttributeKind { +export type ExplicitUnionMemberGroups = ReadonlyMap< + symbol, + ReadonlySet +>; + +class ExplicitUnionMemberTypeAttributeKind extends TypeAttributeKind { public constructor() { super("explicit-union-member"); } - public combine(_values: true[]): true { - return true; + public combine( + values: ExplicitUnionMemberGroups[], + ): ExplicitUnionMemberGroups { + const combined = new Map>(); + for (const groups of values) { + for (const [group, members] of groups) { + const combinedMembers = combined.get(group) ?? new Set(); + for (const member of members) combinedMembers.add(member); + combined.set(group, combinedMembers); + } + } + return combined; } - public makeInferred(_value: true): true { - return true; + public makeInferred( + value: ExplicitUnionMemberGroups, + ): ExplicitUnionMemberGroups { + return value; } - public stringify(_value: true): string { + public stringify(_value: ExplicitUnionMemberGroups): string { return "explicit union member"; } } -export const explicitUnionMemberTypeAttributeKind: TypeAttributeKind = +export const explicitUnionMemberTypeAttributeKind: TypeAttributeKind = new ExplicitUnionMemberTypeAttributeKind(); -export const explicitUnionMemberAttributes = - explicitUnionMemberTypeAttributeKind.makeAttributes(true); +export function makeExplicitUnionMemberAttributes( + group: symbol, +): TypeAttributes { + return explicitUnionMemberTypeAttributeKind.makeAttributes( + new Map([[group, new Set([Symbol()])]]), + ); +} diff --git a/packages/quicktype-core/src/input/JSONSchemaInput.ts b/packages/quicktype-core/src/input/JSONSchemaInput.ts index eda42912a3..5608fbacf0 100644 --- a/packages/quicktype-core/src/input/JSONSchemaInput.ts +++ b/packages/quicktype-core/src/input/JSONSchemaInput.ts @@ -32,7 +32,7 @@ import { import { defaultValueAttributeProducer } from "../attributes/DefaultValue.js"; import { descriptionAttributeProducer } from "../attributes/Description.js"; import { enumValuesAttributeProducer } from "../attributes/EnumValues.js"; -import { explicitUnionMemberAttributes } from "../attributes/ExplicitUnionMember.js"; +import { makeExplicitUnionMemberAttributes } from "../attributes/ExplicitUnionMember.js"; import { StringTypes } from "../attributes/StringTypes.js"; import { type TypeAttributes, @@ -1227,10 +1227,11 @@ async function addTypesInSchema( typeof (c as { $ref?: unknown }).$ref === "string", ) ) { + const explicitUnionGroup = Symbol(); for (const typeRef of typeRefs) { typeBuilder.addAttributes( typeRef, - explicitUnionMemberAttributes, + makeExplicitUnionMemberAttributes(explicitUnionGroup), ); } } diff --git a/test/unit/named-class-union.test.ts b/test/unit/named-class-union.test.ts index 3dc7a3ba9a..e9a7ba197d 100644 --- a/test/unit/named-class-union.test.ts +++ b/test/unit/named-class-union.test.ts @@ -47,14 +47,34 @@ function schemaWithUnion(operation: "oneOf" | "anyOf"): object { }; } +function schemaWithReusedRef(): object { + const schema = schemaWithUnion("oneOf") as { + properties: Record; + }; + schema.properties.mixed = { + oneOf: [ + { $ref: "#/definitions/Foo" }, + { + title: "Baz", + type: "object", + additionalProperties: false, + properties: { baz: { type: "string" } }, + required: ["baz"], + }, + ], + }; + return schema; +} + async function generate( lang: "python" | "typescript", operation: "oneOf" | "anyOf" = "oneOf", + schema: object = schemaWithUnion(operation), ): Promise { const schemaInput = new JSONSchemaInput(new FetchingJSONSchemaStore()); await schemaInput.addSource({ name: "Container", - schema: JSON.stringify(schemaWithUnion(operation)), + schema: JSON.stringify(schema), }); const inputData = new InputData(); inputData.addInput(schemaInput); @@ -85,6 +105,15 @@ describe("named class unions (issue #1646)", () => { expect(output).not.toContain("class Op:"); }); + test("the explicit-union marker does not leak through reused refs", async () => { + const output = await generate("python", "oneOf", schemaWithReusedRef()); + + expect(output).toContain("op: Union[Foo, Bar]"); + expect(output).toContain("mixed: Optional[MixedClass] = None"); + expect(output).toContain("foo: Optional[str] = None"); + expect(output).toContain("baz: Optional[str] = None"); + }); + test("languages that do not opt in keep the existing behavior", async () => { const output = await generate("typescript");