diff --git a/packages/quicktype-core/src/rewrites/CombineClasses.ts b/packages/quicktype-core/src/rewrites/CombineClasses.ts index df8540a2e..7ac8fe0f0 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); @@ -132,6 +140,10 @@ 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. + // 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( @@ -142,7 +154,9 @@ function findSimilarityCliques( 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; } diff --git a/test/fixtures.ts b/test/fixtures.ts index 84616537e..54be2f76c 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, @@ -1742,6 +1825,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 000000000..1746e3269 --- /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 000000000..577f42933 --- /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 000000000..360f28bc7 --- /dev/null +++ b/test/fixtures/csharp-multiple-json-top-levels/test.csproj @@ -0,0 +1,9 @@ + + + Exe + net8.0 + + + + + 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 000000000..54f80497b --- /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 000000000..10a13ab10 --- /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 000000000..9b8e9b853 --- /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 + } +}