Fix numeric enum deserialization - #24394
Merged
wing328 merged 4 commits intoJul 25, 2026
Merged
Conversation
Signed-off-by: sakshichitnis27 <sakshichitnis27@users.noreply.github.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
|
thanks for the PR cc @devhl-labs |
Member
|
please follow step 3 to update the samples |
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
|
Thanks a lot for this. Here are 'my' thoughts, I swear I'm not a bot. Thanks for this — the refactoring into 1. Null token on non-nullable fields The new template skips assignment when a null JSON token arrives for a non-nullable field ( if (utf8JsonReader.TokenType == JsonTokenType.Null)
enumInteger = new Option<EnumTest.EnumIntegerEnum?>(null); // IsSet=true, Value=nullThat assignment isn't wrong — if (enumInteger.IsSet && enumInteger.Value == null)
throw new ArgumentNullException(nameof(enumInteger), "Property is not nullable for class EnumTest.");So the old path surfaced the schema violation correctly through the existing validation layer. The new silent-skip path has two regressions:
Suggest restoring the 2. Unrecognized enum values should throw Removing the If forward-compatibility is the motivation (new server enum values shouldn't crash old clients), that's worth supporting — but silent skip isn't a real implementation of leniency. Lenient handling needs to do something meaningful with the unknown value: an |
Contributor
There was a problem hiding this comment.
31 issues found across 160 files (changes from recent commits).
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/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Model/EnumTest.cs:750">
P1: Unknown numeric enum values now throw `JsonException` instead of leaving the non-nullable option unset. This makes forward-compatible payloads with a newly introduced numeric enum value fail deserialization; preserving the existing `if (enum...Value != null)` assignment behavior would match the intended handling.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/SourceGeneration/src/Org.OpenAPITools/Model/Pet.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/SourceGeneration/src/Org.OpenAPITools/Model/Pet.cs:312">
P2: Unknown values for this nullable enum now throw `JsonException`, which breaks forward-compatible deserialization and contradicts the nullable-enum behavior described by this PR. Assign the option only when `statusValue` is non-null so an unknown value leaves `Status` as `null`/unset.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/SourceGeneration/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1">
P2: Unknown numeric enum values now abort deserialization with `JsonException` instead of leaving the non-nullable option unset. The numeric branches should only assign when the `FromStringOrDefault` result is non-null; the same change is needed for `enumIntegerOnly` and `enumNumber`.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/SourceGeneration/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P1: Unknown nullable numeric enum values now fail with `JsonException` instead of deserializing to `null`. Make the unknown-value branch conditional on schema nullability: nullable properties should set `Option<Enum?>(null)`, while non-nullable properties should remain unset.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1">
P2: A JSON `null` for each numeric enum is now stored as an explicitly set null and then rejected by the converter's final validation. Leaving the option unset for a null token avoids this unintended `ArgumentNullException` and preserves the non-nullable field's unset behavior.</violation>
<violation number="2">
P2: Unknown numeric enum values now raise `JsonException` instead of leaving the option unset. Matching the existing string-enum behavior would assign the option only when `enumIntegerValue` is non-null, with the same correction needed for `enumIntegerOnly` and `enumNumber`.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P1: Unknown numeric values for nullable inner enums now fail deserialization instead of producing a set nullable option with `null`. Preserve the null-token branch, but map an unrecognized `FromStringOrDefault` result to `null` for nullable enum properties and regenerate this sample.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net4.7/Petstore/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P1: Unknown values for nullable numeric enums now abort deserialization with `JsonException` rather than becoming `null`. Preserve the nullable assignment for an unmapped value; non-nullable fields should instead retain their unset `Option` state.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/EnumTest.cs:758">
P2: Unknown numeric enum values now fail deserialization instead of leaving the optional non-nullable property unset. Ignoring the `null` result from `FromStringOrDefault` for these fields would preserve the behavior described by this fix.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/SourceGeneration/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P1: Unknown numeric values for nullable enum fields now fail deserialization with `JsonException`, rather than being represented as `null`. Remove these null guards so the existing nullable `Option` assignment handles an unrecognized value.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P1: Unknown values for nullable numeric enums now abort deserialization with `JsonException` instead of producing a set null value. Preserve the nullable fallback for `FromStringOrDefault(...) == null`; this regresses the behavior this change is intended to provide.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net4.8/Petstore/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P1: Unknown nullable numeric enum values now throw instead of deserializing as null. Remove this rejection path so the following nullable `Option` assignment receives the mapper's null result.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P2: Unknown nullable numeric enum values now throw rather than deserialize as `null`. Let the nullable `Option` receive the null conversion result so forward-compatible enum values retain the documented nullable behavior.</violation>
<violation number="2">
P2: Unknown values for optional non-nullable numeric enums now abort deserialization with `JsonException` instead of leaving the option unset. Preserve the unset state when `FromStringOrDefault` returns null, matching the stated fallback behavior.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Model/Descendant1.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Model/Descendant1.cs:129">
P2: An unrecognized `objectType` value now aborts deserialization with `JsonException` instead of leaving the required option unset for the converter's normal validation path. Testing the nullable result before assignment (`if (objectTypeValue != null)`) would preserve the documented unknown non-nullable enum behavior.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache:259">
P2: Unknown nullable string enum values now raise `JsonException` instead of deserializing as null, regressing the existing inner-enum behavior and making this path inconsistent with the nullable handling described for numeric enums. The null result should be assigned to the optional property rather than treated as an exception for nullable fields.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Model/ZeroBasedEnumClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Model/ZeroBasedEnumClass.cs:211">
P2: Deserializing an unknown value for this nullable enum now throws `JsonException` rather than yielding `null`. This null check should be omitted for nullable enums so the `null` returned by `ZeroBasedEnumEnumFromStringOrDefault` can be stored in the option.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P1: Unknown nullable enum values now fail deserialization with `JsonException` instead of becoming null. Preserve the `Option<T?>` assignment when `FromStringOrDefault` returns null; apply that consistently to every nullable enum branch.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/Petstore/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1">
P2: An unknown numeric enum value now throws `JsonException` instead of leaving the option unset. Checking for a null parsed enum and skipping assignment preserves the documented non-nullable behavior; apply the same handling to `enumIntegerOnly` and `enumNumber`.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P2: A JSON `null` for a non-nullable numeric enum is now treated as a present value, bypassing required-property validation (and marking optional non-nullable fields as explicitly set to null). Keep the option unset for null tokens, matching the adjacent string-enum path and the intended non-nullable semantics.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Order.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Order.cs:333">
P2: Unknown `status` values still abort deserialization instead of leaving `StatusOption` unset, so a forward-compatible enum payload cannot be read. Guard the assignment on `statusValue != null` rather than throwing when the mapping returns null.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1">
P2: A JSON `null` for the non-nullable `enumInteger` field is now converted into a set-null option and rejected by the final validation, rather than being ignored and left unset. The null branch should leave `enumInteger` at its default unset state; the corresponding `enumIntegerOnly` and `enumNumber` branches need the same handling.</violation>
<violation number="2" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/EnumTest.cs:759">
P1: An unknown numeric enum value now aborts `EnumTest` deserialization with `JsonException`, instead of leaving the non-nullable option unset as intended. Assign the option only when `enumIntegerValue` is non-null (and apply the same condition to `enumIntegerOnly` and `enumNumber`).</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/Petstore/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1">
P1: Unknown numeric values for nullable enum properties now fail deserialization with `JsonException` rather than deserialize as `null`. Preserve the null result from `FromStringOrDefault` when assigning the nullable `Option`, matching the stated nullable-enum behavior.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1">
P2: An explicit JSON `null` for these numeric enum fields now produces `ArgumentNullException` because the option is marked set with a null value. Skipping the assignment on `JsonTokenType.Null` keeps the option unset and avoids this unintended failure path.</violation>
<violation number="2">
P1: Unknown numeric enum values now abort deserialization with `JsonException` instead of leaving the corresponding option unset. Keeping the assignment conditional on a non-null parsed value, as the previous code did, preserves the intended unknown-value behavior.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1">
P1: A server response containing a newly added numeric enum value now fails deserialization with `JsonException`, preventing forward-compatible clients from reading the rest of the model. For these non-nullable numeric fields, the unknown-value branch should only assign when the parsed enum is non-null and otherwise leave the `Option` unset; the same correction is needed in the `enumIntegerOnly` and `enumNumber` branches.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/standard2.0/Petstore/src/Org.OpenAPITools/Model/Descendant2.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/standard2.0/Petstore/src/Org.OpenAPITools/Model/Descendant2.cs:129">
P2: An unknown `objectType` enum literal now causes deserialization to throw `JsonException` because `objectTypeValue == null` is rejected. The generated nullable/non-nullable handling should leave the option unset or retain the nullable `null` value instead of throwing here.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/Petstore/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/Petstore/src/Org.OpenAPITools/Model/EnumTest.cs:761">
P2: Null-token handling differs between string and numeric enum fields. When the JSON has `"enum_string": null`, `GetString()` returns null and the option stays unset (IsSet=false) — no error. But when `"enum_integer": null`, the code assigns `new Option<...>(null)`, setting IsSet=true, which triggers the ArgumentNullException validation check. This contradicts the PR's goal of making numeric enum deserialization consistent with string enums. To match the string behavior, skip assignment on null tokens for numeric enums too (leave IsSet=false).</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Model/EnumArrays.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Model/EnumArrays.cs:296">
P2: Unknown values for this nullable enum now fail deserialization with `JsonException` instead of remaining `null`, so forward-compatible responses containing a new `just_symbol` value cannot be read. Preserving the `null` result for nullable fields would match the intended nullable-enum behavior.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/NumericInnerEnumJsonRead.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/NumericInnerEnumJsonRead.mustache:7">
P1: The throw new JsonException() for unrecognized numeric enum values contradicts the PR's stated behavior. For nullable enums, unknown values should be deserialized as null (not throw). For non-nullable enums, unknown values should leave the option unset. Either restore the nullable-aware branching from the old template, or update the PR description to reflect the breaking change.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| { | ||
| EnumTest.EnumStringRequiredEnum? enumStringRequiredValue = EnumTest.EnumStringRequiredEnumFromStringOrDefault(enumStringRequiredRawValue); | ||
| if (enumStringRequiredValue == null) | ||
| throw new JsonException(); |
Contributor
There was a problem hiding this comment.
P1: Unknown numeric enum values now throw JsonException instead of leaving the non-nullable option unset. This makes forward-compatible payloads with a newly introduced numeric enum value fail deserialization; preserving the existing if (enum...Value != null) assignment behavior would match the intended handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/latest/UseDateTimeOffset/src/Org.OpenAPITools/Model/EnumTest.cs, line 750:
<comment>Unknown numeric enum values now throw `JsonException` instead of leaving the non-nullable option unset. This makes forward-compatible payloads with a newly introduced numeric enum value fail deserialization; preserving the existing `if (enum...Value != null)` assignment behavior would match the intended handling.</comment>
<file context>
@@ -744,39 +744,58 @@ public override EnumTest Read(ref Utf8JsonReader utf8JsonReader, Type typeToConv
+ {
+ EnumTest.EnumStringRequiredEnum? enumStringRequiredValue = EnumTest.EnumStringRequiredEnumFromStringOrDefault(enumStringRequiredRawValue);
+ if (enumStringRequiredValue == null)
+ throw new JsonException();
+ enumStringRequired = new Option<EnumTest.EnumStringRequiredEnum?>(enumStringRequiredValue);
+ }
</file context>
| @@ -1876,7 +1876,12 @@ public override RequiredClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT | |||
| case "required_notnullable_enum_string": | |||
Contributor
There was a problem hiding this comment.
P1: Unknown nullable numeric enum values now fail with JsonException instead of deserializing to null. Make the unknown-value branch conditional on schema nullability: nullable properties should set Option<Enum?>(null), while non-nullable properties should remain unset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net8/SourceGeneration/src/Org.OpenAPITools/Model/RequiredClass.cs, line 1978:
<comment>Unknown nullable numeric enum values now fail with `JsonException` instead of deserializing to `null`. Make the unknown-value branch conditional on schema nullability: nullable properties should set `Option<Enum?>(null)`, while non-nullable properties should remain unset.</comment>
<file context>
@@ -1953,6 +1975,8 @@ public override RequiredClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT
{
string notrequiredNullableEnumIntegerRawValue = utf8JsonReader.GetInt32().ToString(System.Globalization.CultureInfo.InvariantCulture);
RequiredClass.NotrequiredNullableEnumIntegerEnum? notrequiredNullableEnumIntegerValue = RequiredClass.NotrequiredNullableEnumIntegerEnumFromStringOrDefault(notrequiredNullableEnumIntegerRawValue);
+ if (notrequiredNullableEnumIntegerValue == null)
+ throw new JsonException();
notrequiredNullableEnumInteger = new Option<RequiredClass.NotrequiredNullableEnumIntegerEnum?>(notrequiredNullableEnumIntegerValue);
</file context>
| @@ -1875,7 +1875,12 @@ public override RequiredClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT | |||
| case "required_notnullable_enum_string": | |||
Contributor
There was a problem hiding this comment.
P1: Unknown numeric values for nullable inner enums now fail deserialization instead of producing a set nullable option with null. Preserve the null-token branch, but map an unrecognized FromStringOrDefault result to null for nullable enum properties and regenerate this sample.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/RequiredClass.cs, line 1977:
<comment>Unknown numeric values for nullable inner enums now fail deserialization instead of producing a set nullable option with `null`. Preserve the null-token branch, but map an unrecognized `FromStringOrDefault` result to `null` for nullable enum properties and regenerate this sample.</comment>
<file context>
@@ -1952,6 +1974,8 @@ public override RequiredClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT
{
string notrequiredNullableEnumIntegerRawValue = utf8JsonReader.GetInt32().ToString(System.Globalization.CultureInfo.InvariantCulture);
RequiredClass.NotrequiredNullableEnumIntegerEnum? notrequiredNullableEnumIntegerValue = RequiredClass.NotrequiredNullableEnumIntegerEnumFromStringOrDefault(notrequiredNullableEnumIntegerRawValue);
+ if (notrequiredNullableEnumIntegerValue == null)
+ throw new JsonException();
notrequiredNullableEnumInteger = new Option<RequiredClass.NotrequiredNullableEnumIntegerEnum?>(notrequiredNullableEnumIntegerValue);
</file context>
| @@ -1873,7 +1873,12 @@ public override RequiredClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT | |||
| case "required_notnullable_enum_string": | |||
Contributor
There was a problem hiding this comment.
P1: Unknown values for nullable numeric enums now abort deserialization with JsonException rather than becoming null. Preserve the nullable assignment for an unmapped value; non-nullable fields should instead retain their unset Option state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net4.7/Petstore/src/Org.OpenAPITools/Model/RequiredClass.cs, line 1975:
<comment>Unknown values for nullable numeric enums now abort deserialization with `JsonException` rather than becoming `null`. Preserve the nullable assignment for an unmapped value; non-nullable fields should instead retain their unset `Option` state.</comment>
<file context>
@@ -1950,6 +1972,8 @@ public override RequiredClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT
{
string notrequiredNullableEnumIntegerRawValue = utf8JsonReader.GetInt32().ToString(System.Globalization.CultureInfo.InvariantCulture);
RequiredClass.NotrequiredNullableEnumIntegerEnum? notrequiredNullableEnumIntegerValue = RequiredClass.NotrequiredNullableEnumIntegerEnumFromStringOrDefault(notrequiredNullableEnumIntegerRawValue);
+ if (notrequiredNullableEnumIntegerValue == null)
+ throw new JsonException();
notrequiredNullableEnumInteger = new Option<RequiredClass.NotrequiredNullableEnumIntegerEnum?>(notrequiredNullableEnumIntegerValue);
</file context>
| @@ -1876,7 +1876,12 @@ public override RequiredClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT | |||
| case "required_notnullable_enum_string": | |||
Contributor
There was a problem hiding this comment.
P1: Unknown numeric values for nullable enum fields now fail deserialization with JsonException, rather than being represented as null. Remove these null guards so the existing nullable Option assignment handles an unrecognized value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net9/SourceGeneration/src/Org.OpenAPITools/Model/RequiredClass.cs, line 1978:
<comment>Unknown numeric values for nullable enum fields now fail deserialization with `JsonException`, rather than being represented as `null`. Remove these null guards so the existing nullable `Option` assignment handles an unrecognized value.</comment>
<file context>
@@ -1953,6 +1975,8 @@ public override RequiredClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT
{
string notrequiredNullableEnumIntegerRawValue = utf8JsonReader.GetInt32().ToString(System.Globalization.CultureInfo.InvariantCulture);
RequiredClass.NotrequiredNullableEnumIntegerEnum? notrequiredNullableEnumIntegerValue = RequiredClass.NotrequiredNullableEnumIntegerEnumFromStringOrDefault(notrequiredNullableEnumIntegerRawValue);
+ if (notrequiredNullableEnumIntegerValue == null)
+ throw new JsonException();
notrequiredNullableEnumInteger = new Option<RequiredClass.NotrequiredNullableEnumIntegerEnum?>(notrequiredNullableEnumIntegerValue);
</file context>
| @@ -753,7 +753,12 @@ public override EnumTest Read(ref Utf8JsonReader utf8JsonReader, Type typeToConv | |||
| case "enum_string_required": | |||
Contributor
There was a problem hiding this comment.
P2: A JSON null for the non-nullable enumInteger field is now converted into a set-null option and rejected by the final validation, rather than being ignored and left unset. The null branch should leave enumInteger at its default unset state; the corresponding enumIntegerOnly and enumNumber branches need the same handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/EnumTest.cs, line 765:
<comment>A JSON `null` for the non-nullable `enumInteger` field is now converted into a set-null option and rejected by the final validation, rather than being ignored and left unset. The null branch should leave `enumInteger` at its default unset state; the corresponding `enumIntegerOnly` and `enumNumber` branches need the same handling.</comment>
<file context>
@@ -753,39 +753,58 @@ public override EnumTest Read(ref Utf8JsonReader utf8JsonReader, Type typeToConv
case "enum_integer":
- if (utf8JsonReader.TokenType != JsonTokenType.Null)
+ if (utf8JsonReader.TokenType == JsonTokenType.Null)
+ enumInteger = new Option<EnumTest.EnumIntegerEnum?>(null);
+ else
{
</file context>
| @@ -752,7 +752,12 @@ public override EnumTest Read(ref Utf8JsonReader utf8JsonReader, Type typeToConv | |||
| case "enum_string_required": | |||
Contributor
There was a problem hiding this comment.
P2: An explicit JSON null for these numeric enum fields now produces ArgumentNullException because the option is marked set with a null value. Skipping the assignment on JsonTokenType.Null keeps the option unset and avoids this unintended failure path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net9/NullReferenceTypes/src/Org.OpenAPITools/Model/EnumTest.cs, line 764:
<comment>An explicit JSON `null` for these numeric enum fields now produces `ArgumentNullException` because the option is marked set with a null value. Skipping the assignment on `JsonTokenType.Null` keeps the option unset and avoids this unintended failure path.</comment>
<file context>
@@ -752,39 +752,58 @@ public override EnumTest Read(ref Utf8JsonReader utf8JsonReader, Type typeToConv
case "enum_integer":
- if (utf8JsonReader.TokenType != JsonTokenType.Null)
+ if (utf8JsonReader.TokenType == JsonTokenType.Null)
+ enumInteger = new Option<EnumTest.EnumIntegerEnum?>(null);
+ else
{
</file context>
| objectType = new Option<Descendant2.ObjectTypeEnum?>(Descendant2.ObjectTypeEnumFromStringOrDefault(objectTypeRawValue)); | ||
| { | ||
| Descendant2.ObjectTypeEnum? objectTypeValue = Descendant2.ObjectTypeEnumFromStringOrDefault(objectTypeRawValue); | ||
| if (objectTypeValue == null) |
Contributor
There was a problem hiding this comment.
P2: An unknown objectType enum literal now causes deserialization to throw JsonException because objectTypeValue == null is rejected. The generated nullable/non-nullable handling should leave the option unset or retain the nullable null value instead of throwing here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/standard2.0/Petstore/src/Org.OpenAPITools/Model/Descendant2.cs, line 129:
<comment>An unknown `objectType` enum literal now causes deserialization to throw `JsonException` because `objectTypeValue == null` is rejected. The generated nullable/non-nullable handling should leave the option unset or retain the nullable `null` value instead of throwing here.</comment>
<file context>
@@ -124,7 +124,12 @@ public override Descendant2 Read(ref Utf8JsonReader utf8JsonReader, Type typeToC
- objectType = new Option<Descendant2.ObjectTypeEnum?>(Descendant2.ObjectTypeEnumFromStringOrDefault(objectTypeRawValue));
+ {
+ Descendant2.ObjectTypeEnum? objectTypeValue = Descendant2.ObjectTypeEnumFromStringOrDefault(objectTypeRawValue);
+ if (objectTypeValue == null)
+ throw new JsonException();
+ objectType = new Option<Descendant2.ObjectTypeEnum?>(objectTypeValue);
</file context>
| } | ||
| break; | ||
| case "enum_integer": | ||
| if (utf8JsonReader.TokenType == JsonTokenType.Null) |
Contributor
There was a problem hiding this comment.
P2: Null-token handling differs between string and numeric enum fields. When the JSON has "enum_string": null, GetString() returns null and the option stays unset (IsSet=false) — no error. But when "enum_integer": null, the code assigns new Option<...>(null), setting IsSet=true, which triggers the ArgumentNullException validation check. This contradicts the PR's goal of making numeric enum deserialization consistent with string enums. To match the string behavior, skip assignment on null tokens for numeric enums too (leave IsSet=false).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net10/Petstore/src/Org.OpenAPITools/Model/EnumTest.cs, line 761:
<comment>Null-token handling differs between string and numeric enum fields. When the JSON has `"enum_string": null`, `GetString()` returns null and the option stays unset (IsSet=false) — no error. But when `"enum_integer": null`, the code assigns `new Option<...>(null)`, setting IsSet=true, which triggers the ArgumentNullException validation check. This contradicts the PR's goal of making numeric enum deserialization consistent with string enums. To match the string behavior, skip assignment on null tokens for numeric enums too (leave IsSet=false).</comment>
<file context>
@@ -750,39 +750,58 @@ public override EnumTest Read(ref Utf8JsonReader utf8JsonReader, Type typeToConv
break;
case "enum_integer":
- if (utf8JsonReader.TokenType != JsonTokenType.Null)
+ if (utf8JsonReader.TokenType == JsonTokenType.Null)
+ enumInteger = new Option<EnumTest.EnumIntegerEnum?>(null);
+ else
</file context>
| { | ||
| EnumArrays.JustSymbolEnum? justSymbolValue = EnumArrays.JustSymbolEnumFromStringOrDefault(justSymbolRawValue); | ||
| if (justSymbolValue == null) | ||
| throw new JsonException(); |
Contributor
There was a problem hiding this comment.
P2: Unknown values for this nullable enum now fail deserialization with JsonException instead of remaining null, so forward-compatible responses containing a new just_symbol value cannot be read. Preserving the null result for nullable fields would match the intended nullable-enum behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net9/Petstore/src/Org.OpenAPITools/Model/EnumArrays.cs, line 296:
<comment>Unknown values for this nullable enum now fail deserialization with `JsonException` instead of remaining `null`, so forward-compatible responses containing a new `just_symbol` value cannot be read. Preserving the `null` result for nullable fields would match the intended nullable-enum behavior.</comment>
<file context>
@@ -290,7 +290,12 @@ public override EnumArrays Read(ref Utf8JsonReader utf8JsonReader, Type typeToCo
+ {
+ EnumArrays.JustSymbolEnum? justSymbolValue = EnumArrays.JustSymbolEnumFromStringOrDefault(justSymbolRawValue);
+ if (justSymbolValue == null)
+ throw new JsonException();
+ justSymbol = new Option<EnumArrays.JustSymbolEnum?>(justSymbolValue);
+ }
</file context>
Contributor
Author
|
Hello @devhl-labs |
Contributor
|
LGTM thanks! |
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
3 participants
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.
Fixes #24344
Description
Makes numeric inner-enum deserialization consistent with string enums in the C#
generichostgenerator.null.Validation
CSharpClientCodegenTest: 10 tests passed, 0 failures.null.git diff --checkpassed.PR checklist
Summary by cubic
Align inner enum deserialization in C#
generichostfor numeric and string enums. Numeric unknowns → null (nullable) or skipped (non‑nullable/byte‑backed); string unknowns now throw; numeric parsing moved toNumericInnerEnumJsonRead.mustache; tests updated for inline int and string enums.Written for commit 8ddebe6. Summary will update on new commits.