From 7fb3fe4713334ae844f9df930c22997043c3b1e0 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 20 Jul 2026 18:18:32 -0400 Subject: [PATCH 1/3] fix(core): honor --no-combine-classes for structurally identical classes (#1265) Structurally identical objects inferred from JSON at different paths (e.g. `amount: { initialValue }` and `rate: { initialValue }`) were deduplicated into a single class by TypeBuilder's identity-based type cache during raw JSON inference, before the optional combineClasses rewrite pass ever ran. Since --no-combine-classes only disables that later rewrite pass, it had no effect on this case, even though it correctly controlled merging of classes with overlapping-but-not-identical properties. Thread combineClasses through Run -> InputData -> JSONInput -> TypeInference, and when it's disabled, mark freshly inferred non-fixed class types with a type attribute that forces unique identity, so they're no longer collapsed by TypeBuilder's structural cache or later graph rewrites. When combineClasses is enabled (the default), CombineClasses.ts's existing unconditional "structurally compatible" merge re-unifies these classes, so default output is unaffected. Co-Authored-By: gpt-5.6-sol via pi --- packages/quicktype-core/src/Run.ts | 2 + .../quicktype-core/src/input/Inference.ts | 39 ++++++++++++++++++- packages/quicktype-core/src/input/Inputs.ts | 10 +++++ test/unit/combine-classes.test.ts | 31 +++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 test/unit/combine-classes.test.ts diff --git a/packages/quicktype-core/src/Run.ts b/packages/quicktype-core/src/Run.ts index 898640aff5..63583b57fa 100644 --- a/packages/quicktype-core/src/Run.ts +++ b/packages/quicktype-core/src/Run.ts @@ -265,6 +265,7 @@ class Run implements RunContext { graphInputs.typeBuilder, this._options.inferMaps, this._options.inferEnums, + this._options.combineClasses, this._options.fixedTopLevels, ), ); @@ -281,6 +282,7 @@ class Run implements RunContext { graphInputs.typeBuilder, this._options.inferMaps, this._options.inferEnums, + this._options.combineClasses, this._options.fixedTopLevels, ), ); diff --git a/packages/quicktype-core/src/input/Inference.ts b/packages/quicktype-core/src/input/Inference.ts index 4efca3c4f2..f189b3a9e7 100644 --- a/packages/quicktype-core/src/input/Inference.ts +++ b/packages/quicktype-core/src/input/Inference.ts @@ -3,6 +3,7 @@ import { inferTransformedStringTypeKindForString, } from "../attributes/StringTypes.js"; import { + TypeAttributeKind, type TypeAttributes, emptyTypeAttributes, } from "../attributes/TypeAttributes.js"; @@ -110,6 +111,27 @@ function canBeEnumCase(_s: string): boolean { return true; } +class UniqueInferenceClassTypeAttributeKind extends TypeAttributeKind { + public constructor() { + super("uniqueInferenceClass"); + } + + public combine(_attrs: true[]): true { + return true; + } + + public makeInferred(_attr: true): true { + return true; + } + + public requiresUniqueIdentity(_attr: true): boolean { + return true; + } +} + +const uniqueInferenceClassTypeAttributeKind = + new UniqueInferenceClassTypeAttributeKind(); + export type Accumulator = UnionAccumulator; export class TypeInference { @@ -120,6 +142,7 @@ export class TypeInference { private readonly _typeBuilder: TypeBuilder, private readonly _inferMaps: boolean, private readonly _inferEnums: boolean, + private readonly _combineClasses: boolean, ) {} private addValuesToAccumulator( @@ -435,8 +458,22 @@ export class TypeInference { ); } + if (this._combineClasses) { + return this._typeBuilder.getClassType( + typeAttributes, + properties, + forwardingRef, + ); + } + + // Graph rewrites otherwise deduplicate non-fixed classes again. + const uniqueAttributes = + uniqueInferenceClassTypeAttributeKind.setDefaultInAttributes( + typeAttributes, + () => true, + ); return this._typeBuilder.getClassType( - typeAttributes, + uniqueAttributes, properties, forwardingRef, ); diff --git a/packages/quicktype-core/src/input/Inputs.ts b/packages/quicktype-core/src/input/Inputs.ts index 65c31448de..0c2c69c608 100644 --- a/packages/quicktype-core/src/input/Inputs.ts +++ b/packages/quicktype-core/src/input/Inputs.ts @@ -32,6 +32,7 @@ export interface Input { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, + combineClasses: boolean, fixedTopLevels: boolean, ) => Promise; @@ -40,6 +41,7 @@ export interface Input { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, + combineClasses: boolean, fixedTopLevels: boolean, ) => void; readonly kind: string; @@ -160,6 +162,7 @@ export class JSONInput implements Input> { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, + combineClasses: boolean, fixedTopLevels: boolean, ): Promise { this.addTypesSync( @@ -167,6 +170,7 @@ export class JSONInput implements Input> { typeBuilder, inferMaps, inferEnums, + combineClasses, fixedTopLevels, ); } @@ -176,6 +180,7 @@ export class JSONInput implements Input> { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, + combineClasses: boolean, fixedTopLevels: boolean, ): void { const inference = new TypeInference( @@ -183,6 +188,7 @@ export class JSONInput implements Input> { typeBuilder, inferMaps, inferEnums, + combineClasses, ); for (const [name, { samples, description }] of this._topLevels) { @@ -268,6 +274,7 @@ export class InputData { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, + combineClasses: boolean, fixedTopLevels: boolean, ): Promise { for (const input of this._inputs) { @@ -276,6 +283,7 @@ export class InputData { typeBuilder, inferMaps, inferEnums, + combineClasses, fixedTopLevels, ); } @@ -286,6 +294,7 @@ export class InputData { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, + combineClasses: boolean, fixedTopLevels: boolean, ): void { for (const input of this._inputs) { @@ -294,6 +303,7 @@ export class InputData { typeBuilder, inferMaps, inferEnums, + combineClasses, fixedTopLevels, ); } diff --git a/test/unit/combine-classes.test.ts b/test/unit/combine-classes.test.ts new file mode 100644 index 0000000000..a2648fe88c --- /dev/null +++ b/test/unit/combine-classes.test.ts @@ -0,0 +1,31 @@ +import { expect, test } from "vitest"; + +import { + InputData, + jsonInputForTargetLanguage, + quicktype, +} from "quicktype-core"; + +test("does not merge structurally identical inferred classes when combineClasses is false", async () => { + const jsonInput = jsonInputForTargetLanguage("cs"); + await jsonInput.addSource({ + name: "Trade", + // Keep both values non-integral so their object shapes are identical. + samples: [ + '{"amount":{"initialValue":1.5},"rate":{"initialValue":0.012}}', + ], + }); + + const inputData = new InputData(); + inputData.addInput(jsonInput); + + const result = await quicktype({ + inputData, + lang: "cs", + combineClasses: false, + }); + const output = result.lines.join("\n"); + + expect(output).toContain("public partial class Amount"); + expect(output).toContain("public partial class Rate"); +}); From 9558836513c6b6ef7997130f8807e33d6bf8ce33 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 22 Jul 2026 15:58:12 -0400 Subject: [PATCH 2/3] fix(core): preserve custom input argument order --- packages/quicktype-core/src/Run.ts | 4 +-- packages/quicktype-core/src/input/Inputs.ts | 18 +++++----- test/unit/combine-classes.test.ts | 38 +++++++++++++++++++++ 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/packages/quicktype-core/src/Run.ts b/packages/quicktype-core/src/Run.ts index 63583b57fa..596d1f8fcb 100644 --- a/packages/quicktype-core/src/Run.ts +++ b/packages/quicktype-core/src/Run.ts @@ -265,8 +265,8 @@ class Run implements RunContext { graphInputs.typeBuilder, this._options.inferMaps, this._options.inferEnums, - this._options.combineClasses, this._options.fixedTopLevels, + this._options.combineClasses, ), ); @@ -282,8 +282,8 @@ class Run implements RunContext { graphInputs.typeBuilder, this._options.inferMaps, this._options.inferEnums, - this._options.combineClasses, this._options.fixedTopLevels, + this._options.combineClasses, ), ); diff --git a/packages/quicktype-core/src/input/Inputs.ts b/packages/quicktype-core/src/input/Inputs.ts index 0c2c69c608..445819487f 100644 --- a/packages/quicktype-core/src/input/Inputs.ts +++ b/packages/quicktype-core/src/input/Inputs.ts @@ -32,8 +32,8 @@ export interface Input { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, - combineClasses: boolean, fixedTopLevels: boolean, + combineClasses: boolean, ) => Promise; addTypesSync: ( @@ -41,8 +41,8 @@ export interface Input { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, - combineClasses: boolean, fixedTopLevels: boolean, + combineClasses: boolean, ) => void; readonly kind: string; @@ -162,16 +162,16 @@ export class JSONInput implements Input> { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, - combineClasses: boolean, fixedTopLevels: boolean, + combineClasses: boolean, ): Promise { this.addTypesSync( ctx, typeBuilder, inferMaps, inferEnums, - combineClasses, fixedTopLevels, + combineClasses, ); } @@ -180,8 +180,8 @@ export class JSONInput implements Input> { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, - combineClasses: boolean, fixedTopLevels: boolean, + combineClasses: boolean, ): void { const inference = new TypeInference( this._compressedJSON, @@ -274,8 +274,8 @@ export class InputData { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, - combineClasses: boolean, fixedTopLevels: boolean, + combineClasses: boolean, ): Promise { for (const input of this._inputs) { await input.addTypes( @@ -283,8 +283,8 @@ export class InputData { typeBuilder, inferMaps, inferEnums, - combineClasses, fixedTopLevels, + combineClasses, ); } } @@ -294,8 +294,8 @@ export class InputData { typeBuilder: TypeBuilder, inferMaps: boolean, inferEnums: boolean, - combineClasses: boolean, fixedTopLevels: boolean, + combineClasses: boolean, ): void { for (const input of this._inputs) { input.addTypesSync( @@ -303,8 +303,8 @@ export class InputData { typeBuilder, inferMaps, inferEnums, - combineClasses, fixedTopLevels, + combineClasses, ); } } diff --git a/test/unit/combine-classes.test.ts b/test/unit/combine-classes.test.ts index a2648fe88c..d4ba33b9bd 100644 --- a/test/unit/combine-classes.test.ts +++ b/test/unit/combine-classes.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "vitest"; import { + type Input, InputData, jsonInputForTargetLanguage, quicktype, @@ -29,3 +30,40 @@ test("does not merge structurally identical inferred classes when combineClasses expect(output).toContain("public partial class Amount"); expect(output).toContain("public partial class Rate"); }); + +test("preserves the fixedTopLevels argument position for custom inputs", async () => { + let receivedFixedTopLevels: boolean | undefined; + const input: Input = { + kind: "custom", + needIR: false, + needSchemaProcessing: false, + addSource: async () => {}, + addSourceSync: () => {}, + addTypes: async ( + _ctx, + typeBuilder, + _inferMaps, + _inferEnums, + fixedTopLevels, + ) => { + receivedFixedTopLevels = fixedTopLevels; + typeBuilder.addTopLevel( + "Value", + typeBuilder.getPrimitiveType("integer"), + ); + }, + addTypesSync: () => {}, + singleStringSchemaSource: () => undefined, + }; + const inputData = new InputData(); + inputData.addInput(input); + + await quicktype({ + inputData, + lang: "typescript", + combineClasses: false, + fixedTopLevels: true, + }); + + expect(receivedFixedTopLevels).toBe(true); +}); From fc94ea9079f3f912e128bc9ed33b0049a058197a Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 22 Jul 2026 17:41:18 -0400 Subject: [PATCH 3/3] test: cover no-combine-classes output --- test/fixtures.ts | 14 ++++++++++++++ test/languages.ts | 11 +++++++++++ test/utils.ts | 4 ++++ 3 files changed, 29 insertions(+) diff --git a/test/fixtures.ts b/test/fixtures.ts index 788bebd537..66835e9738 100644 --- a/test/fixtures.ts +++ b/test/fixtures.ts @@ -558,6 +558,19 @@ class JSONFixture extends LanguageFixture { } } +class NoCombineClassesJSONFixture extends JSONFixture { + constructor() { + super( + languages.CSharpNoCombineClassesLanguage, + "csharp-no-combine-classes", + ); + } + + runForName(name: string): boolean { + return name === "csharp" || super.runForName(name); + } +} + // This fixture tests generating code for language X from JSON, // then generating code for Y from the code for X, making sure // that the resulting code for Y accepts the JSON by running it @@ -1735,6 +1748,7 @@ class CommandSuccessfulLanguageFixture extends LanguageFixture { export const allFixtures: Fixture[] = [ // new JSONFixture(languages.CrystalLanguage), new JSONFixture(languages.CSharpLanguage), + new NoCombineClassesJSONFixture(), new JSONFixture( languages.CSharpLanguageSystemTextJson, "csharp-SystemTextJson", diff --git a/test/languages.ts b/test/languages.ts index d413a66bcd..d436806536 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -90,6 +90,7 @@ export interface Language { skipMiscJSON: boolean; skipSchema: string[]; rendererOptions: RendererOptions; + combineClasses?: boolean; quickTestRendererOptions: (RendererOptions | [string, RendererOptions])[]; sourceFiles?: string[]; } @@ -163,6 +164,16 @@ export const CSharpLanguage: Language = { sourceFiles: ["src/language/CSharp/index.ts"], }; +export const CSharpNoCombineClassesLanguage: Language = { + ...CSharpLanguage, + diffViaSchema: false, + includeJSON: ["combined-enum.json"], + skipJSON: undefined, + skipMiscJSON: true, + combineClasses: false, + quickTestRendererOptions: [], +}; + export const CSharpLanguageSystemTextJson: Language = { name: "csharp", base: "test/fixtures/csharp-SystemTextJson", diff --git a/test/utils.ts b/test/utils.ts index 3a6d1f4bc2..8fb5693d14 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -162,6 +162,10 @@ export async function quicktypeForLanguage( // GraphQL input can leave unreachable types in the graph, which means // their provenance won't be propagated. It does that for non-nullables. debug: graphqlSchema === undefined ? "provenance" : undefined, + noCombineClasses: + language.combineClasses === undefined + ? undefined + : !language.combineClasses, }); } catch (e) { failWith("quicktype threw an exception", {