Skip to content

fix(csharp): make generated Serialize/Converter classes partial#3022

Merged
schani merged 1 commit into
masterfrom
agent/fix-issue-1520
Jul 21, 2026
Merged

fix(csharp): make generated Serialize/Converter classes partial#3022
schani merged 1 commit into
masterfrom
agent/fix-issue-1520

Conversation

@schani

@schani schani commented Jul 20, 2026

Copy link
Copy Markdown
Member

The bug

quicktype's C# generator emits data classes (e.g. Something) as public partial class, but the generated Serialize and Converter helper classes were emitted as plain public static class / internal static class — missing the partial keyword.

This matters because a common workflow generates quicktype output for multiple JSON samples into the same --namespace as separate files (e.g. running quicktype once per sample, all sharing --namespace QuickType). Each generated file redefines Serialize and Converter; without partial, C# fails to compile with duplicate-type-definition errors, even though the data classes themselves compile fine because they already use partial.

Root cause

In both NewtonSoftCSharpRenderer.ts and SystemTextJsonCSharpRenderer.ts, emitSerializeClass and emitConverterClass called this.emitType(..., "static class", ...) with the literal string "static class", instead of "static partial class" like the data-class emission path uses.

The fix

Changed the "static class" literal to "static partial class" in both renderers' emitSerializeClass (produces Serialize) and emitConverterClass (produces Converter) methods — 4 one-line changes total, matching how data classes already get partial.

Test coverage

  • test/unit/csharp-partial-helpers.test.ts: generates C# for both the NewtonSoft and SystemTextJson frameworks and asserts the output contains public static partial class Serialize and internal static partial class Converter. Verified this test fails against the pre-fix code and passes after the fix.
  • test/fixtures/csharp/Issue1520.cs and test/fixtures/csharp-SystemTextJson/Issue1520.cs: added to the C# driver project directories (copied into every C# fixture test run, since the .csproj globs all .cs files). Each declares empty public static partial class Serialize { } / internal static partial class Converter { } stubs in the QuickType namespace, simulating a second generated file sharing the namespace. If the generated Serialize/Converter classes ever regress to non-partial, every C# JSON/schema fixture test will fail to compile with a "missing partial modifier" error, giving durable end-to-end coverage of this exact bug (not just a single named fixture).

Verification performed locally

  • npm run build passes with no TypeScript errors.
  • Re-ran the issue's repro (node dist/index.js --lang cs --namespace QuickType something.json, both frameworks) and confirmed the generated output now reads public static partial class Serialize and internal static partial class Converter.
  • Ran the new unit test (npx vitest run test/unit/csharp-partial-helpers.test.ts) — passes on the fix, and confirmed it fails (by stashing the renderer changes, rebuilding, and re-running) against the unfixed code.
  • No dotnet toolchain is available in this environment, so the C# fixture suite (which compiles/runs the generated + driver .cs files) could not be executed locally; CI will exercise FIXTURE=csharp / FIXTURE=csharp-SystemTextJson and the schema variants, which will compile the new Issue1520.cs files alongside every generated sample.

Fixes #1520

🤖 Generated with Claude Code

Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
@schani
schani merged commit d7e2762 into master Jul 21, 2026
28 checks passed
@schani
schani deleted the agent/fix-issue-1520 branch July 21, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[c#] missing partial keyword on generated "Serialize" class

1 participant