From 69420e7080c6a2502f6c718ea2614af1fb28403c Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 20 Jul 2026 19:16:49 -0400 Subject: [PATCH 1/3] fix(core): stop heuristically merging distinct named top-level classes (#1630) When multiple named top-level JSON inputs were structurally similar (one a property subset of another), CombineClasses' similarity-clique heuristic would merge them into a single class, silently dropping one of the named top-level types and misnaming shared descendant types after the wrong survivor. Exclude named top-level classes from the heuristic similarity-merge candidate pool in findSimilarityCliques; exact-duplicate top-levels are still deduplicated during inference as before, and heuristic merging of non-top-level (inferred/nested) classes is unaffected. Added a regression test (test/unit/multiple-json-top-levels.test.ts) using the Plaid API request JSON samples from the issue, asserting AccountsGetRequest stays distinct from AuthGetRequest and that AuthGetRequest's nested options class is named AuthGetRequestOptions rather than AccountsGetRequestOptions. Fixes #1630 Co-Authored-By: gpt-5.6-sol via pi --- .../src/rewrites/CombineClasses.ts | 8 ++- .../issue-1630/AccountsGet.Request.json | 5 ++ .../priority/issue-1630/AuthGet.Request.json | 8 +++ .../issue-1630/TransactionsGet.Request.json | 12 ++++ test/unit/multiple-json-top-levels.test.ts | 63 +++++++++++++++++++ 5 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 test/inputs/json/priority/issue-1630/AccountsGet.Request.json create mode 100644 test/inputs/json/priority/issue-1630/AuthGet.Request.json create mode 100644 test/inputs/json/priority/issue-1630/TransactionsGet.Request.json create mode 100644 test/unit/multiple-json-top-levels.test.ts diff --git a/packages/quicktype-core/src/rewrites/CombineClasses.ts b/packages/quicktype-core/src/rewrites/CombineClasses.ts index df8540a2e0..98fa09b4ea 100644 --- a/packages/quicktype-core/src/rewrites/CombineClasses.ts +++ b/packages/quicktype-core/src/rewrites/CombineClasses.ts @@ -132,10 +132,16 @@ function findSimilarityCliques( onlyWithSameProperties: boolean, includeFixedClasses: boolean, ): ClassType[][] { + // Distinct named top-levels must not be heuristically combined. Exactly + // identical top-levels can already have been deduplicated during inference. + const topLevels = new Set(graph.topLevels.values()); const classCandidates = Array.from( graph.allNamedTypesSeparated().objects, ).filter( - (o) => o instanceof ClassType && (includeFixedClasses || !o.isFixed), + (o) => + o instanceof ClassType && + !topLevels.has(o) && + (includeFixedClasses || !o.isFixed), ) as ClassType[]; const cliques: Clique[] = []; diff --git a/test/inputs/json/priority/issue-1630/AccountsGet.Request.json b/test/inputs/json/priority/issue-1630/AccountsGet.Request.json new file mode 100644 index 0000000000..54f80497bf --- /dev/null +++ b/test/inputs/json/priority/issue-1630/AccountsGet.Request.json @@ -0,0 +1,5 @@ +{ + "client_id": "1234abcd5678efgh90ij", + "secret": "1234abcd5678efgh90ij", + "access_token": "1234abcd5678efgh90ij" +} diff --git a/test/inputs/json/priority/issue-1630/AuthGet.Request.json b/test/inputs/json/priority/issue-1630/AuthGet.Request.json new file mode 100644 index 0000000000..10a13ab10b --- /dev/null +++ b/test/inputs/json/priority/issue-1630/AuthGet.Request.json @@ -0,0 +1,8 @@ +{ + "client_id": "1234abcd5678efgh90ij", + "secret": "1234abcd5678efgh90ij", + "access_token": "1234abcd5678efgh90ij", + "options": { + "account_ids": ["abc", "def", "ghi"] + } +} diff --git a/test/inputs/json/priority/issue-1630/TransactionsGet.Request.json b/test/inputs/json/priority/issue-1630/TransactionsGet.Request.json new file mode 100644 index 0000000000..9b8e9b853c --- /dev/null +++ b/test/inputs/json/priority/issue-1630/TransactionsGet.Request.json @@ -0,0 +1,12 @@ +{ + "client_id": "1234abcd5678efgh90ij", + "secret": "1234abcd5678efgh90ij", + "access_token": "1234abcd5678efgh90ij", + "start_date": "2020-01-01", + "end_date": "2020-01-31", + "options": { + "account_ids": ["abc", "def", "ghi"], + "count": 100, + "offset": 0 + } +} diff --git a/test/unit/multiple-json-top-levels.test.ts b/test/unit/multiple-json-top-levels.test.ts new file mode 100644 index 0000000000..53ee922e41 --- /dev/null +++ b/test/unit/multiple-json-top-levels.test.ts @@ -0,0 +1,63 @@ +import fs from "node:fs"; +import path from "node:path"; + +import { describe, expect, test } from "vitest"; + +import { + InputData, + jsonInputForTargetLanguage, + quicktype, +} from "../../packages/quicktype-core/src/index.js"; + +const fixtureDirectory = "test/inputs/json/priority/issue-1630"; +const fixtureFiles = [ + "AccountsGet.Request.json", + "AuthGet.Request.json", + "TransactionsGet.Request.json", +]; + +async function renderCSharp(): Promise { + const jsonInput = jsonInputForTargetLanguage("csharp"); + for (const filename of fixtureFiles) { + await jsonInput.addSource({ + name: path.basename(filename, ".json"), + samples: [ + fs.readFileSync(path.join(fixtureDirectory, filename), "utf8"), + ], + }); + } + + const inputData = new InputData(); + inputData.addInput(jsonInput); + + const result = await quicktype({ + inputData, + lang: "csharp", + rendererOptions: { framework: "SystemTextJson", namespace: "Plaid" }, + }); + return result.lines.join("\n"); +} + +describe("multiple named JSON top-levels", () => { + test("keeps similar request types distinct", async () => { + const output = await renderCSharp(); + + expect(output).toMatch( + /public partial class AccountsGetRequest\s*\{[\s\S]*?JsonPropertyName\("client_id"\)[\s\S]*?JsonPropertyName\("secret"\)[\s\S]*?JsonPropertyName\("access_token"\)/, + ); + expect(output).toContain( + "public static AccountsGetRequest FromJson(string json) => JsonSerializer.Deserialize(json, Plaid.Converter.Settings);", + ); + expect(output).toContain( + "public AuthGetRequestOptions Options { get; set; }", + ); + expect(output).toContain("public partial class AuthGetRequestOptions"); + expect(output).not.toContain("AccountsGetRequestOptions"); + expect(output).toContain( + "public TransactionsGetRequestOptions Options { get; set; }", + ); + expect(output).toContain( + "public partial class TransactionsGetRequestOptions", + ); + }); +}); From 3db1b7d463d7afb9779f7dbedc72507c0d18cee9 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 22 Jul 2026 15:35:59 -0400 Subject: [PATCH 2/3] fix(core): preserve top-level recursive class merging --- .../src/rewrites/CombineClasses.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/quicktype-core/src/rewrites/CombineClasses.ts b/packages/quicktype-core/src/rewrites/CombineClasses.ts index 98fa09b4ea..7ac8fe0f0b 100644 --- a/packages/quicktype-core/src/rewrites/CombineClasses.ts +++ b/packages/quicktype-core/src/rewrites/CombineClasses.ts @@ -108,7 +108,15 @@ function tryAddToClique( c: ClassType, clique: Clique, onlyWithSameProperties: boolean, + topLevels: ReadonlySet, ): boolean { + if ( + topLevels.has(c) && + clique.members.some((member) => topLevels.has(member)) + ) { + return false; + } + for (const prototype of clique.prototypes) { if (prototype.structurallyCompatible(c)) { clique.members.push(c); @@ -134,21 +142,21 @@ function findSimilarityCliques( ): ClassType[][] { // Distinct named top-levels must not be heuristically combined. Exactly // identical top-levels can already have been deduplicated during inference. + // Top-levels can still combine with nested classes to preserve recursion. const topLevels = new Set(graph.topLevels.values()); const classCandidates = Array.from( graph.allNamedTypesSeparated().objects, ).filter( - (o) => - o instanceof ClassType && - !topLevels.has(o) && - (includeFixedClasses || !o.isFixed), + (o) => o instanceof ClassType && (includeFixedClasses || !o.isFixed), ) as ClassType[]; const cliques: Clique[] = []; for (const c of classCandidates) { let cliqueIndex: number | undefined; for (let i = 0; i < cliques.length; i++) { - if (tryAddToClique(c, cliques[i], onlyWithSameProperties)) { + if ( + tryAddToClique(c, cliques[i], onlyWithSameProperties, topLevels) + ) { cliqueIndex = i; break; } From 906f8ffa1d723160992f8a2621cc5169e214db51 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 22 Jul 2026 17:27:58 -0400 Subject: [PATCH 3/3] test: cover multiple named JSON top levels in fixtures --- test/fixtures.ts | 84 +++++++++++++++++++ .../.gitignore | 2 + .../Program.cs | 43 ++++++++++ .../test.csproj | 9 ++ test/unit/multiple-json-top-levels.test.ts | 63 -------------- 5 files changed, 138 insertions(+), 63 deletions(-) create mode 100644 test/fixtures/csharp-multiple-json-top-levels/.gitignore create mode 100644 test/fixtures/csharp-multiple-json-top-levels/Program.cs create mode 100644 test/fixtures/csharp-multiple-json-top-levels/test.csproj delete mode 100644 test/unit/multiple-json-top-levels.test.ts diff --git a/test/fixtures.ts b/test/fixtures.ts index 788bebd537..edb314fd98 100644 --- a/test/fixtures.ts +++ b/test/fixtures.ts @@ -351,6 +351,89 @@ abstract class LanguageFixture extends Fixture { } } +class MultipleJSONTopLevelsFixture extends LanguageFixture { + readonly name = "csharp-multiple-json-top-levels"; + private readonly fixtureDirectory = "test/inputs/json/priority/issue-1630"; + + constructor() { + super({ + ...languages.CSharpLanguageSystemTextJson, + base: "test/fixtures/csharp-multiple-json-top-levels", + }); + } + + runForName(name: string): boolean { + return ( + name === this.name || + name === "csharp-SystemTextJson" || + name === "json" + ); + } + + getSamples(sources: string[]): { priority: Sample[]; others: Sample[] } { + if (sources.length > 0) return { priority: [], others: [] }; + + return { + priority: [ + { + path: this.fixtureDirectory, + additionalRendererOptions: {}, + saveOutput: true, + }, + ], + others: [], + }; + } + + shouldSkipTest(_sample: Sample): boolean { + return false; + } + + async runQuicktype( + directory: string, + additionalRendererOptions: RendererOptions, + ): Promise { + await quicktype({ + srcLang: "json", + lang: this.language.name, + src: [directory], + out: this.language.output, + alphabetizeProperties: true, + rendererOptions: _.merge( + {}, + this.language.rendererOptions, + additionalRendererOptions, + ), + quiet: true, + telemetry: "disable", + debug: "provenance", + }); + } + + async test( + directory: string, + additionalRendererOptions: RendererOptions, + _additionalFiles: string[], + ): Promise { + if (this.language.compileCommand) { + await execAsync(this.language.compileCommand); + } + + const samples = testsInDir(directory, "json"); + for (const sample of samples) { + compareJsonFileToJson( + comparisonArgs( + this.language, + sample, + sample, + additionalRendererOptions, + ), + ); + } + return samples.length; + } +} + class JSONFixture extends LanguageFixture { constructor( language: languages.Language, @@ -1739,6 +1822,7 @@ export const allFixtures: Fixture[] = [ languages.CSharpLanguageSystemTextJson, "csharp-SystemTextJson", ), + new MultipleJSONTopLevelsFixture(), new JSONFixture(languages.JavaLanguage), new JSONFixture( languages.JavaLanguageWithLegacyDateTime, diff --git a/test/fixtures/csharp-multiple-json-top-levels/.gitignore b/test/fixtures/csharp-multiple-json-top-levels/.gitignore new file mode 100644 index 0000000000..1746e3269e --- /dev/null +++ b/test/fixtures/csharp-multiple-json-top-levels/.gitignore @@ -0,0 +1,2 @@ +bin +obj diff --git a/test/fixtures/csharp-multiple-json-top-levels/Program.cs b/test/fixtures/csharp-multiple-json-top-levels/Program.cs new file mode 100644 index 0000000000..577f429332 --- /dev/null +++ b/test/fixtures/csharp-multiple-json-top-levels/Program.cs @@ -0,0 +1,43 @@ +using System; +using System.IO; + +namespace QuickType +{ + class Program + { + static void Main(string[] args) + { + var path = args[0]; + var json = File.ReadAllText(path); + string output; + + switch (Path.GetFileName(path)) + { + case "AccountsGet.Request.json": + { + AccountsGetRequest value = AccountsGetRequest.FromJson(json); + output = value.ToJson(); + break; + } + case "AuthGet.Request.json": + { + AuthGetRequest value = AuthGetRequest.FromJson(json); + AuthGetRequestOptions options = value.Options; + output = value.ToJson(); + break; + } + case "TransactionsGet.Request.json": + { + TransactionsGetRequest value = TransactionsGetRequest.FromJson(json); + TransactionsGetRequestOptions options = value.Options; + output = value.ToJson(); + break; + } + default: + throw new ArgumentException($"Unexpected fixture input: {path}"); + } + + Console.WriteLine(output); + } + } +} diff --git a/test/fixtures/csharp-multiple-json-top-levels/test.csproj b/test/fixtures/csharp-multiple-json-top-levels/test.csproj new file mode 100644 index 0000000000..360f28bc77 --- /dev/null +++ b/test/fixtures/csharp-multiple-json-top-levels/test.csproj @@ -0,0 +1,9 @@ + + + Exe + net8.0 + + + + + diff --git a/test/unit/multiple-json-top-levels.test.ts b/test/unit/multiple-json-top-levels.test.ts deleted file mode 100644 index 53ee922e41..0000000000 --- a/test/unit/multiple-json-top-levels.test.ts +++ /dev/null @@ -1,63 +0,0 @@ -import fs from "node:fs"; -import path from "node:path"; - -import { describe, expect, test } from "vitest"; - -import { - InputData, - jsonInputForTargetLanguage, - quicktype, -} from "../../packages/quicktype-core/src/index.js"; - -const fixtureDirectory = "test/inputs/json/priority/issue-1630"; -const fixtureFiles = [ - "AccountsGet.Request.json", - "AuthGet.Request.json", - "TransactionsGet.Request.json", -]; - -async function renderCSharp(): Promise { - const jsonInput = jsonInputForTargetLanguage("csharp"); - for (const filename of fixtureFiles) { - await jsonInput.addSource({ - name: path.basename(filename, ".json"), - samples: [ - fs.readFileSync(path.join(fixtureDirectory, filename), "utf8"), - ], - }); - } - - const inputData = new InputData(); - inputData.addInput(jsonInput); - - const result = await quicktype({ - inputData, - lang: "csharp", - rendererOptions: { framework: "SystemTextJson", namespace: "Plaid" }, - }); - return result.lines.join("\n"); -} - -describe("multiple named JSON top-levels", () => { - test("keeps similar request types distinct", async () => { - const output = await renderCSharp(); - - expect(output).toMatch( - /public partial class AccountsGetRequest\s*\{[\s\S]*?JsonPropertyName\("client_id"\)[\s\S]*?JsonPropertyName\("secret"\)[\s\S]*?JsonPropertyName\("access_token"\)/, - ); - expect(output).toContain( - "public static AccountsGetRequest FromJson(string json) => JsonSerializer.Deserialize(json, Plaid.Converter.Settings);", - ); - expect(output).toContain( - "public AuthGetRequestOptions Options { get; set; }", - ); - expect(output).toContain("public partial class AuthGetRequestOptions"); - expect(output).not.toContain("AccountsGetRequestOptions"); - expect(output).toContain( - "public TransactionsGetRequestOptions Options { get; set; }", - ); - expect(output).toContain( - "public partial class TransactionsGetRequestOptions", - ); - }); -});