From 7600dd15254e8d192565f7807ab8a101e58354f7 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 20 Jul 2026 18:35:54 -0400 Subject: [PATCH 1/2] fix(csharp): omit superfluous usings when no converter class is emitted (#800) Co-Authored-By: gpt-5.6-sol via pi --- .../CSharp/NewtonSoftCSharpRenderer.ts | 26 +++++--- .../CSharp/SystemTextJsonCSharpRenderer.ts | 17 ++++-- test/languages.ts | 2 + test/unit/csharp-superfluous-usings.test.ts | 61 +++++++++++++++++++ 4 files changed, 91 insertions(+), 15 deletions(-) create mode 100644 test/unit/csharp-superfluous-usings.test.ts diff --git a/packages/quicktype-core/src/language/CSharp/NewtonSoftCSharpRenderer.ts b/packages/quicktype-core/src/language/CSharp/NewtonSoftCSharpRenderer.ts index 4fbd16c23..0da8c1b39 100644 --- a/packages/quicktype-core/src/language/CSharp/NewtonSoftCSharpRenderer.ts +++ b/packages/quicktype-core/src/language/CSharp/NewtonSoftCSharpRenderer.ts @@ -62,6 +62,13 @@ export class NewtonsoftCSharpRenderer extends CSharpRenderer { private readonly _needNamespaces: boolean; + private get needConverterClass(): boolean { + return ( + this._needHelpers || + (this._needAttributes && (this.haveNamedUnions || this.haveEnums)) + ); + } + public constructor( targetLanguage: TargetLanguage, renderContext: RenderContext, @@ -160,12 +167,14 @@ export class NewtonsoftCSharpRenderer extends CSharpRenderer { super.emitUsings(); this.ensureBlankLine(); - for (const ns of [ - "System.Globalization", - "Newtonsoft.Json", - "Newtonsoft.Json.Converters", - ]) { - this.emitUsing(ns); + if (this.needConverterClass) { + this.emitUsing("System.Globalization"); + } + + this.emitUsing("Newtonsoft.Json"); + + if (this.needConverterClass) { + this.emitUsing("Newtonsoft.Json.Converters"); } if (this._options.dense) { @@ -1177,10 +1186,7 @@ export class NewtonsoftCSharpRenderer extends CSharpRenderer { this.emitSerializeClass(); } - if ( - this._needHelpers || - (this._needAttributes && (this.haveNamedUnions || this.haveEnums)) - ) { + if (this.needConverterClass) { this.ensureBlankLine(); this.emitConverterClass(); this.forEachTransformation("leading-and-interposing", (n, t) => diff --git a/packages/quicktype-core/src/language/CSharp/SystemTextJsonCSharpRenderer.ts b/packages/quicktype-core/src/language/CSharp/SystemTextJsonCSharpRenderer.ts index 0a217e0ed..5e0ab1ad4 100644 --- a/packages/quicktype-core/src/language/CSharp/SystemTextJsonCSharpRenderer.ts +++ b/packages/quicktype-core/src/language/CSharp/SystemTextJsonCSharpRenderer.ts @@ -61,6 +61,13 @@ export class SystemTextJsonCSharpRenderer extends CSharpRenderer { private readonly _needNamespaces: boolean; + private get needConverterClass(): boolean { + return ( + this._needHelpers || + (this._needAttributes && (this.haveNamedUnions || this.haveEnums)) + ); + } + public constructor( targetLanguage: TargetLanguage, renderContext: RenderContext, @@ -165,11 +172,14 @@ export class SystemTextJsonCSharpRenderer extends CSharpRenderer { for (const ns of [ "System.Text.Json", "System.Text.Json.Serialization", - "System.Globalization", ]) { this.emitUsing(ns); } + if (this.needConverterClass) { + this.emitUsing("System.Globalization"); + } + if (this._options.dense) { this.emitUsing([ denseJsonPropertyName, @@ -1299,10 +1309,7 @@ export class SystemTextJsonCSharpRenderer extends CSharpRenderer { this.emitSerializeClass(); } - if ( - this._needHelpers || - (this._needAttributes && (this.haveNamedUnions || this.haveEnums)) - ) { + if (this.needConverterClass) { this.ensureBlankLine(); this.emitConverterClass(); this.forEachTransformation("leading-and-interposing", (n, t) => diff --git a/test/languages.ts b/test/languages.ts index 7f80f5e3b..f13c2a0fe 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -128,6 +128,7 @@ export const CSharpLanguage: Language = { rendererOptions: { "check-required": "true", framework: "NewtonSoft" }, quickTestRendererOptions: [ { "array-type": "list" }, + ["simple-object.json", { features: "attributes-only" }], // The default is csharp-version=8; these keep the older // language-version code paths covered. { "csharp-version": "5" }, @@ -183,6 +184,7 @@ export const CSharpLanguageSystemTextJson: Language = { rendererOptions: { "check-required": "true", framework: "SystemTextJson" }, quickTestRendererOptions: [ { "array-type": "list" }, + ["simple-object.json", { features: "attributes-only" }], // The default is csharp-version=8; these keep the older // language-version code paths covered. { "csharp-version": "5" }, diff --git a/test/unit/csharp-superfluous-usings.test.ts b/test/unit/csharp-superfluous-usings.test.ts new file mode 100644 index 000000000..999d2acfa --- /dev/null +++ b/test/unit/csharp-superfluous-usings.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, test } from "vitest"; + +import { + InputData, + type RendererOptions, + jsonInputForTargetLanguage, + quicktype, +} from "../../packages/quicktype-core/src/index.js"; + +async function renderCSharp(rendererOptions: RendererOptions): Promise { + const jsonInput = jsonInputForTargetLanguage("csharp"); + await jsonInput.addSource({ + name: "Sample", + samples: ['{"name":"Alice","age":30,"active":true}'], + }); + + const inputData = new InputData(); + inputData.addInput(jsonInput); + const result = await quicktype({ + inputData, + lang: "csharp", + rendererOptions, + }); + return result.lines.join("\n"); +} + +describe("C# using statements", () => { + test("NewtonSoft attributes-only omits converter usings when no converter is emitted", async () => { + const output = await renderCSharp({ + framework: "NewtonSoft", + features: "attributes-only", + }); + + expect(output).toContain("using Newtonsoft.Json;"); + expect(output).not.toContain("using System.Globalization;"); + expect(output).not.toContain("using Newtonsoft.Json.Converters;"); + }); + + test("SystemTextJson attributes-only omits globalization when no converter is emitted", async () => { + const output = await renderCSharp({ + framework: "SystemTextJson", + features: "attributes-only", + }); + + expect(output).toContain("using System.Text.Json.Serialization;"); + expect(output).not.toContain("using System.Globalization;"); + }); + + test.each([ + ["NewtonSoft", "using Newtonsoft.Json.Converters;"], + ["SystemTextJson", "using System.Globalization;"], + ] as Array< + ["NewtonSoft" | "SystemTextJson", string] + >)("%s complete output keeps converter usings", async (framework, expectedUsing) => { + const output = await renderCSharp({ framework }); + + expect(output).toContain("using System.Globalization;"); + expect(output).toContain(expectedUsing); + expect(output).toContain("IsoDateTime"); + }); +}); From bf5df485fd11470b9e2082a70be7ef73fcbbd4c1 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 20 Jul 2026 20:03:26 -0400 Subject: [PATCH 2/2] test(csharp): drop attributes-only round-trip fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The attributes-only C# mode emits only the annotated model classes and no FromJson/ToJson serialization helpers, so the round-trip fixture driver (TopLevel.FromJson(json).ToJson()) cannot compile against it — CI failed with CS0117 'TopLevel' does not contain a definition for 'FromJson'. Remove the two `["simple-object.json", { features: "attributes-only" }]` quickTestRendererOptions entries added for NewtonSoft and SystemTextJson. The using-omission behavior is fully covered by the unit tests in test/unit/csharp-superfluous-usings.test.ts, which is the appropriate mechanism for asserting that code is *not* generated (a round-trip fixture cannot exercise attributes-only mode by design). Co-Authored-By: Claude --- test/languages.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/languages.ts b/test/languages.ts index f13c2a0fe..7f80f5e3b 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -128,7 +128,6 @@ export const CSharpLanguage: Language = { rendererOptions: { "check-required": "true", framework: "NewtonSoft" }, quickTestRendererOptions: [ { "array-type": "list" }, - ["simple-object.json", { features: "attributes-only" }], // The default is csharp-version=8; these keep the older // language-version code paths covered. { "csharp-version": "5" }, @@ -184,7 +183,6 @@ export const CSharpLanguageSystemTextJson: Language = { rendererOptions: { "check-required": "true", framework: "SystemTextJson" }, quickTestRendererOptions: [ { "array-type": "list" }, - ["simple-object.json", { features: "attributes-only" }], // The default is csharp-version=8; these keep the older // language-version code paths covered. { "csharp-version": "5" },