fix(csharp): omit superfluous usings when no converter class is emitted#3045
Merged
Conversation
…ed (#800) Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
In
--lang cs, the NewtonSoft and System.Text.Json renderers unconditionallyemitted the usings needed for the date-time converter class whenever
attributes (
--features attributes-only, or the default full mode) wererequested — even for models with no date/date-time fields, named unions, or
enums, i.e. cases where no converter class is generated at all.
Repro (
--lang cs --features attributes-only --framework NewtonSofton amodel with only string/number/bool fields):
System.GlobalizationandNewtonsoft.Json.Convertersare only used insidethe generated
Converter/date-converter class, which wasn't being emittedhere — so both usings were dead code.
Root cause
NewtonsoftCSharpRenderer.emitUsings()andSystemTextJsonCSharpRenderer.emitUsings()emittedSystem.Globalization(and, for NewtonSoft,
Newtonsoft.Json.Converters) unconditionally wheneverattributes or helpers were needed, instead of checking whether the converter
class itself (
emitConverterClass()) was actually going to be emitted.Fix
Added a shared
needConverterClassgetter in each renderer (the samecondition already used to decide whether to call
emitConverterClass()) andgated the
System.Globalization/Newtonsoft.Json.Convertersusings on it.Newtonsoft.Json(needed forJsonPropertyattributes) and theSystem.Text.Json/System.Text.Json.Serializationusings are unaffected andstill always emitted when needed.
Test coverage
test/unit/csharp-superfluous-usings.test.ts(new): asserts thatattributes-onlyoutput for a model with no dates/unions/enums omitsSystem.GlobalizationandNewtonsoft.Json.Convertersfor both theNewtonSoft and System.Text.Json frameworks, while still keeping the
required namespace usings; also asserts the usings are still present when a
converter class is actually generated (full/helpers mode).
test/languages.ts: enabledattributes-onlyfixture coverage forsimple-object.jsonunder bothCSharpLanguage(NewtonSoft) andCSharpLanguageSystemTextJson, so the end-to-end fixture pipeline alsoexercises this code path.
Verification
npm run buildpasses.npx vitest run test/unit— 174/174 tests pass, including the 4 new ones.node dist/index.js --lang cs --features attributes-only --framework NewtonSoft sample.json) andconfirmed the superfluous usings are gone, while a model with a date field
or the full (non-attributes-only) mode still emits the converter usings
correctly.
FIXTURE=csharp/FIXTURE=csharp-systemtextjson)could not be run locally because
dotnetis not installed in thisenvironment; the new
simple-object.json/attributes-onlyfixturecombination added in
test/languages.tswill be exercised by CI.Fixes #800.
🤖 Generated with Claude Code