[csharp] fix: conditional serialization invents value-type defaults on all-optional models

[wiebren]

With conditionalSerialization=true, the public constructor of an all-optional model takes every property as a plain value type and raises the serialization flag behind if (this.AutoRenew != null). For a value type (bool, numbers, DateTime, Guid, enums) that guard is always true (CS0472), so every value-typed property counts as "set" as soon as the model is constructed. Reproduced on current master with the httpclient library (Newtonsoft):

constructed as	request body before	request body after
new UpdateThingCommand(note: "renamed")	{"autoRenew":false,"note":"renamed"}	{"note":"renamed"}
new UpdateThingCommand(autoRenew: false)	{"autoRenew":false}	{"autoRenew":false}
new UpdateThingCommand { Period = 12 }	{"autoRenew":false,"period":12}	{"period":12}

A partial update meant to rename the thing also switches off auto-renewal. Models with required properties avoid the bug because Json.NET populates them through their [JsonConstructor], so it affects all-optional models, which is the partial-update shape.

Fix

modelGeneric.mustache, constructor only:

public UpdateThingCommand(bool? autoRenew = default, int? period = default, string note = default)
{
    if (autoRenew != null)
    {
        this._AutoRenew = autoRenew.Value;
        this._flagAutoRenew = true;
    }

Only optional value-typed parameters without a spec default change. Required parameters, defaulted ones, reference types, nullable non-enum value types (int?), the public property surface and all output without the flag are unchanged.

Compatibility

Source-compatible: bool converts implicitly to bool?. The CLR signature changes, so dependent assemblies must be rebuilt. There is deliberately no overload with the old signature: it would make calls such as new Model() ambiguous (CS0121) and would bring the bug back for calls that pass a plain value.

Tests

CSharpClientCodegenTest, on a new spec 3_0/csharp/conditional-serialization-value-types.yaml:

• testConditionalSerializationValueTypeConstructorFlags: nullable parameters, guarded flag assignment for a value type and an enum, int? keeps its null check, unchanged property surface and required-model constructor. Fails without the fix.
• testValueTypeConstructorUnchangedWithoutConditionalSerialization: without the flag, the constructor is unchanged.

Known gaps

Required value-typed properties never raise their flag in the public constructor either. The generated code for them is identical before and after this PR, so that fix belongs in a separate PR.

PR checklist

• Read the contribution guidelines.
• Built the project and updated samples (./bin/generate-samples.sh ./bin/configs/csharp*.yaml). Only csharp-restsharp-netstandard2.0-conditionalSerialization uses the option; it changes in the constructor only (27 model files).
• Technical committee: @mandrean @shibayan @Blackclaws @lucamazzanti

---

Generated with Claude Code

[cubic-dev-ai]

3 issues found across 30 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/Order.cs">

<violation number="1" location="samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/Order.cs:97">
P2: Changing `Order`'s public constructor parameters from `long`, `int`, and `DateTime` to nullable types changes its CLR signature. Applications that update this generated library without recompiling now fail to resolve the old constructor; retain a compatibility overload or document the binary break.</violation>
</file>

<file name="samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/ApiResponse.cs">

<violation number="1" location="samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/ApiResponse.cs:41">
P2: Changing `int` to `int?` changes the public CLR constructor signature, so an already-compiled consumer calling `ApiResponse(int, string, string)` fails with `MissingMethodException` after upgrading this library. Preserve the old constructor signature as a compatibility overload and delegate it to the nullable implementation.</violation>
</file>

<file name="modules/openapi-generator/src/main/resources/csharp/modelGeneric.mustache">

<violation number="1" location="modules/openapi-generator/src/main/resources/csharp/modelGeneric.mustache:143">
P2: For nullable value-type properties (spec `nullable: true`), the new `?` appended here may collide with an existing `?` already present in `datatypeWithEnum`/`dataType`, producing an invalid `int??` parameter type (compile error) instead of `int?`. The C# codegen appends `?` to nullable value-type dataTypes (CSharpClientCodegen around line 654: `if (isSupportNullable() && ModelUtils.isNullable(p) && this.getNullableTypes().contains(type))`), so the type may already end in `?`. Unlike the enum branch, which is unguarded here, the new value-type branch should only append `?` when `datatypeWithEnum` does not already end in `?`. The conditional-serialization sample only covers plain `bool`/`integer` (non-nullable), so this path is untested. Please verify that an optional nullable value-type property does not generate `??` and guard the `?` accordingly (e.g. also require the type not to already end with `?`).</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Changing Order's public constructor parameters from long, int, and DateTime to nullable types changes its CLR signature. Applications that update this generated library without recompiling now fail to resolve the old constructor; retain a compatibility overload or document the binary break.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/Order.cs, line 97:

<comment>Changing `Order`'s public constructor parameters from `long`, `int`, and `DateTime` to nullable types changes its CLR signature. Applications that update this generated library without recompiling now fail to resolve the old constructor; retain a compatibility overload or document the binary break.</comment>

<file context>
@@ -94,31 +94,31 @@ public bool ShouldSerializeStatus()
         /// <param name="status">Order Status.</param>
         /// <param name="complete">complete (default to false).</param>
-        public Order(long id = default, long petId = default, int quantity = default, DateTime shipDate = default, StatusEnum? status = default, bool complete = false)
+        public Order(long? id = default, long? petId = default, int? quantity = default, DateTime? shipDate = default, StatusEnum? status = default, bool complete = false)
         {
-            this._Id = id;
</file context>

[wiebren]

The CLR signature does change, but I am deliberately not adding a compatibility overload. The overload would break source compatibility and bring the bug back. I tested this with the exact pair it would generate (C# 12, net8.0):

public ApiResponse(int? code = default, string type = default, string message = default)  // new
public ApiResponse(int code = default, string type = default, string message = default)   // compat overload

• new ApiResponse() and new ApiResponse(message: "x"): error CS0121, "The call is ambiguous between ApiResponse(int?, string, string) and ApiResponse(int, string, string)". Every model with an optional value-typed property would stop compiling for callers that skip that argument. Partial-update calls are exactly that shape.
• new ApiResponse(code: 5) binds to the int overload, because the identity conversion is better than int to int?. So every caller who passes a value-typed argument goes back to the flag-raising constructor this PR fixes.

A consumer who recompiles against the regenerated model (the normal workflow for generated code) picks up the new constructor with no source changes, because int converts implicitly to int?. The only scenario left is swapping in a regenerated assembly without rebuilding its callers. The generator already gives no guarantees for that: the model has a single all-parameter constructor, so adding any optional property to the spec changes its CLR signature in the same way. The change is also limited to conditionalSerialization=true, which only one sample in the repo uses. I will call out the signature change in the PR description so it can go into the release notes.

[cubic-dev-ai]

P2: Changing int to int? changes the public CLR constructor signature, so an already-compiled consumer calling ApiResponse(int, string, string) fails with MissingMethodException after upgrading this library. Preserve the old constructor signature as a compatibility overload and delegate it to the nullable implementation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/ApiResponse.cs, line 41:

<comment>Changing `int` to `int?` changes the public CLR constructor signature, so an already-compiled consumer calling `ApiResponse(int, string, string)` fails with `MissingMethodException` after upgrading this library. Preserve the old constructor signature as a compatibility overload and delegate it to the nullable implementation.</comment>

<file context>
@@ -38,11 +38,11 @@ public partial class ApiResponse : IEquatable<ApiResponse>, IValidatableObject
         /// <param name="type">type.</param>
         /// <param name="message">message.</param>
-        public ApiResponse(int code = default, string type = default, string message = default)
+        public ApiResponse(int? code = default, string type = default, string message = default)
         {
-            this._Code = code;
</file context>

[wiebren]

The CLR signature does change, but I am deliberately not adding a compatibility overload. The overload would break source compatibility and bring the bug back. I tested this with the exact pair it would generate (C# 12, net8.0):

public ApiResponse(int? code = default, string type = default, string message = default)  // new
public ApiResponse(int code = default, string type = default, string message = default)   // compat overload

• new ApiResponse() and new ApiResponse(message: "x"): error CS0121, "The call is ambiguous between ApiResponse(int?, string, string) and ApiResponse(int, string, string)". Every model with an optional value-typed property would stop compiling for callers that skip that argument. Partial-update calls are exactly that shape.
• new ApiResponse(code: 5) binds to the int overload, because the identity conversion is better than int to int?. So every caller who passes a value-typed argument goes back to the flag-raising constructor this PR fixes.

A consumer who recompiles against the regenerated model (the normal workflow for generated code) picks up the new constructor with no source changes, because int converts implicitly to int?. The only scenario left is swapping in a regenerated assembly without rebuilding its callers. The generator already gives no guarantees for that: the model has a single all-parameter constructor, so adding any optional property to the spec changes its CLR signature in the same way. The change is also limited to conditionalSerialization=true, which only one sample in the repo uses. I will call out the signature change in the PR description so it can go into the release notes.

[cubic-dev-ai]

P2: For nullable value-type properties (spec nullable: true), the new ? appended here may collide with an existing ? already present in datatypeWithEnum/dataType, producing an invalid int?? parameter type (compile error) instead of int?. The C# codegen appends ? to nullable value-type dataTypes (CSharpClientCodegen around line 654: if (isSupportNullable() && ModelUtils.isNullable(p) && this.getNullableTypes().contains(type))), so the type may already end in ?. Unlike the enum branch, which is unguarded here, the new value-type branch should only append ? when datatypeWithEnum does not already end in ?. The conditional-serialization sample only covers plain bool/integer (non-nullable), so this path is untested. Please verify that an optional nullable value-type property does not generate ?? and guard the ? accordingly (e.g. also require the type not to already end with ?).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/csharp/modelGeneric.mustache, line 143:

<comment>For nullable value-type properties (spec `nullable: true`), the new `?` appended here may collide with an existing `?` already present in `datatypeWithEnum`/`dataType`, producing an invalid `int??` parameter type (compile error) instead of `int?`. The C# codegen appends `?` to nullable value-type dataTypes (CSharpClientCodegen around line 654: `if (isSupportNullable() && ModelUtils.isNullable(p) && this.getNullableTypes().contains(type))`), so the type may already end in `?`. Unlike the enum branch, which is unguarded here, the new value-type branch should only append `?` when `datatypeWithEnum` does not already end in `?`. The conditional-serialization sample only covers plain `bool`/`integer` (non-nullable), so this path is untested. Please verify that an optional nullable value-type property does not generate `??` and guard the `?` accordingly (e.g. also require the type not to already end with `?`).</comment>

<file context>
@@ -140,7 +140,7 @@
         [JsonConstructorAttribute]
     {{/hasOnlyReadOnly}}
-        public {{classname}}({{#readWriteVars}}{{{datatypeWithEnum}}}{{#isEnum}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}}{{/isEnum}} {{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}} = {{#defaultValue}}{{^isDateTime}}{{#isString}}{{^isEnum}}@{{/isEnum}}{{/isString}}{{{defaultValue}}}{{/isDateTime}}{{#isDateTime}}default{{/isDateTime}}{{/defaultValue}}{{^defaultValue}}default{{/defaultValue}}{{^-last}}, {{/-last}}{{/readWriteVars}}){{#parent}} : base({{#parentVars}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{^-last}}, {{/-last}}{{/parentVars}}){{/parent}}
+        public {{classname}}({{#readWriteVars}}{{{datatypeWithEnum}}}{{#isEnum}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}}{{/isEnum}}{{^isEnum}}{{#conditionalSerialization}}{{#vendorExtensions.x-csharp-value-type}}{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}{{/vendorExtensions.x-csharp-value-type}}{{/conditionalSerialization}}{{/isEnum}} {{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}} = {{#defaultValue}}{{^isDateTime}}{{#isString}}{{^isEnum}}@{{/isEnum}}{{/isString}}{{{defaultValue}}}{{/isDateTime}}{{#isDateTime}}default{{/isDateTime}}{{/defaultValue}}{{^defaultValue}}default{{/defaultValue}}{{^-last}}, {{/-last}}{{/readWriteVars}}){{#parent}} : base({{#parentVars}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{^-last}}, {{/-last}}{{/parentVars}}){{/parent}}
         {
             {{#vars}}
</file context>

[wiebren]

Checked this against the generator; the T?? case does not occur. The appended ? sits inside {{#vendorExtensions.x-csharp-value-type}}, and CSharpClientCodegen.patchProperty sets that flag only when getNullableTypes() contains property.dataType. That set holds the bare names (int, bool, DateTime, ...), so a nullable: true property whose dataType is already int? never gets the flag. It keeps the original template path: int? graceDays as the parameter, with the existing if (this.GraceDays != null) check, which is meaningful for a nullable type. Enums never end in ? in datatypeWithEnum (the name is StatusEnum even when nullable), so the enum branch appends exactly one ?, as it did before this PR.

I verified this by generating a model with nullable int/bool, an inline enum, a nullable inline enum and a $ref enum for httpclient, restsharp and unityWebRequest, with and without nullableReferenceTypes. No ?? appears, and the httpclient/net8.0 output builds with 0 errors and 0 warnings.

In dfc897f I added a nullable integer and a nullable enum to the test fixture, and asserted that no ?? is emitted and that the nullable value type keeps its original null check. That locks this in, so a future change to the value-type extension cannot silently regress it.