[CSHARP][GENERICHOST] Fix JSON validation for non-required nullable reference types#23189
[CSHARP][GENERICHOST] Fix JSON validation for non-required nullable reference types#23189alec-petersen wants to merge 4 commits intoOpenAPITools:masterfrom
Conversation
There was a problem hiding this comment.
8 issues found across 425 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
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/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/AppleReq.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/AppleReq.cs:151">
P2: Removing the null check for `mealy` allows `IsSet=true` with `Value=null`, but serialization still dereferences `MealyOption.Value!.Value`, which throws when null. This creates an invalid state and runtime exception path after deserialization.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/BananaReq.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/BananaReq.cs:151">
P2: Removing the null check for sweet allows SweetOption.Value to be null while serialization still dereferences Value!.Value, which will throw at runtime when Sweet is null and IsSet is true.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Tag.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Tag.cs:159">
P2: Removing the null guard allows deserialization of `id: null`, but serialization still dereferences `IdOption.Value!.Value`, which throws for null. This creates a read-success/write-fail runtime exception path when round-tripping JSON with `id` set to null.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/NullableClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/NullableClass.cs:369">
P2: Removed null guards allow schema-nonnullable `array_items_nullable`/`object_items_nullable` containers to be read/written as JSON null.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/AllOf/src/Org.OpenAPITools/Model/Child.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/AllOf/src/Org.OpenAPITools/Model/Child.cs:157">
P2: Removed null guards in `Read` now permit `IsSet` options with null underlying nullable values, but `WriteProperties` still dereferences `.Value!.Value`, causing runtime exceptions when serializing inputs like `{"age":null}` or `{"boosterSeat":null}`.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Return.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Return.cs:194">
P2: `Read` now allows `return: null`, but `WriteProperties` still dereferences nullable `return` as non-null and can throw during serialization.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/AnyOfNoCompare/src/Org.OpenAPITools/Model/Banana.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/AnyOfNoCompare/src/Org.OpenAPITools/Model/Banana.cs:138">
P2: Removing the null guard allows `count` to be set to null, but serialization still dereferences `banana.CountOption.Value!.Value`, which will throw when `count` is null. Reintroduce the null check or handle nulls when writing.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Banana.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Banana.cs:139">
P2: Removing the null check allows lengthCm to be deserialized as null while WriteProperties later dereferences LengthCmOption.Value!.Value, which will throw for null values. This reintroduces a failure path that the removed guard previously prevented.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
.../petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/AppleReq.cs
Show resolved
Hide resolved
...petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/BananaReq.cs
Show resolved
Hide resolved
...lient/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Tag.cs
Show resolved
Hide resolved
| if (objectItemsNullable.IsSet && objectItemsNullable.Value == null) | ||
| throw new ArgumentNullException(nameof(objectItemsNullable), "Property is not nullable for class NullableClass."); | ||
|
|
||
| return new NullableClass(arrayAndItemsNullableProp, arrayItemsNullable, arrayNullableProp, booleanProp, dateProp, datetimeProp, integerProp, numberProp, objectAndItemsNullableProp, objectItemsNullable, objectNullableProp, stringProp); |
There was a problem hiding this comment.
P2: Removed null guards allow schema-nonnullable array_items_nullable/object_items_nullable containers to be read/written as JSON null.
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/NullReferenceTypes/src/Org.OpenAPITools/Model/NullableClass.cs, line 369:
<comment>Removed null guards allow schema-nonnullable `array_items_nullable`/`object_items_nullable` containers to be read/written as JSON null.</comment>
<file context>
@@ -366,12 +366,6 @@ public override NullableClass Read(ref Utf8JsonReader utf8JsonReader, Type typeT
- if (objectItemsNullable.IsSet && objectItemsNullable.Value == null)
- throw new ArgumentNullException(nameof(objectItemsNullable), "Property is not nullable for class NullableClass.");
-
return new NullableClass(arrayAndItemsNullableProp, arrayItemsNullable, arrayNullableProp, booleanProp, dateProp, datetimeProp, integerProp, numberProp, objectAndItemsNullableProp, objectItemsNullable, objectNullableProp, stringProp);
}
</file context>
samples/client/petstore/csharp/generichost/net10/AllOf/src/Org.OpenAPITools/Model/Child.cs
Show resolved
Hide resolved
...nt/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Return.cs
Show resolved
Hide resolved
...client/petstore/csharp/generichost/net10/AnyOfNoCompare/src/Org.OpenAPITools/Model/Banana.cs
Show resolved
Hide resolved
...nt/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/Banana.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
16 issues found across 163 files (changes from recent commits).
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
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/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/MixedPropertiesAndAdditionalPropertiesClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/MixedPropertiesAndAdditionalPropertiesClass.cs:242">
P2: Converter writes `dateTime: null` but still deserializes `dateTime` as non-nullable `DateTime`, causing round-trip JSON failures.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/FormatTest.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/FormatTest.cs:902">
P2: Writing a null dateTime now produces JSON that the reader cannot parse because it deserializes with non-nullable DateTime. This creates a write/read mismatch and will throw on round-trip.</violation>
<violation number="2" location="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/FormatTest.cs:991">
P2: Serialization now emits null for string_formatted_as_decimal, but deserialization still calls GetDecimal() without a null-token guard, which will throw on JsonTokenType.Null. This creates a runtime mismatch when a null value is written.</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:384">
P2: `shipDate` is now serialized as JSON null, but deserialized as non-nullable `DateTime`, causing converter round-trip failures.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/DateOnlyClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/DateOnlyClass.cs:176">
P2: Write now emits JSON null for DateOnlyProperty, but read still deserializes as non-nullable DateOnly, which will throw on a null token. This breaks round-trip for explicitly-null optional values.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/MixedPropertiesAndAdditionalPropertiesClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/MixedPropertiesAndAdditionalPropertiesClass.cs:242">
P2: Serializer now writes `dateTime: null` but deserializer still uses `Deserialize<DateTime>`, causing `JsonException` on null and breaking round-trip for nullable `DateTime?`.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/MixedPropertiesAndAdditionalPropertiesClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/MixedPropertiesAndAdditionalPropertiesClass.cs:243">
P2: Added `dateTime` null serialization is not matched by deserialization logic, causing failures when `"dateTime": null` is read.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/FormatTest.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/FormatTest.cs:903">
P2: `dateTime` is serialized as explicit null but deserialized as non-nullable `DateTime`, causing null round-trip failure.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/Order.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/NullReferenceTypes/src/Org.OpenAPITools/Model/Order.cs:384">
P2: Writer now emits "shipDate": null, but the reader still uses JsonSerializer.Deserialize<DateTime> (non-nullable). System.Text.Json doesn’t support deserializing JSON null into non-nullable value types, so round‑tripping a null shipDate will throw.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/Order.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/Order.cs:385">
P2: The writer now emits "shipDate": null when ShipDateOption is set but null, but the reader still uses JsonSerializer.Deserialize<DateTime>(...) without null handling. System.Text.Json does not support deserializing a JSON null literal into a non-nullable DateTime, so round-tripping a null shipDate will throw JsonException.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/DateOnlyClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/DateOnlyClass.cs:176">
P2: Serializer now writes `dateOnlyProperty: null`, but deserializer still uses non-nullable `DateOnly`, causing parse failures for emitted null values.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/UseDateTimeForDate/src/Org.OpenAPITools/Model/NowGet200Response.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/UseDateTimeForDate/src/Org.OpenAPITools/Model/NowGet200Response.cs:192">
P2: Write now emits null for optional DateTime fields, but Read still deserializes them as non-nullable DateTime, so "now": null / "today": null will throw JsonException and break round-trip.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/DateOnlyClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/DateOnlyClass.cs:177">
P2: WriteProperties can now emit "dateOnlyProperty": null, but Read still deserializes with JsonSerializer.Deserialize<DateOnly>, which throws on null for non-nullable value types. This breaks round‑tripping and will fail on null payloads produced by this converter.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/FormatTest.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/FormatTest.cs:902">
P2: WriteProperties now emits explicit null for optional dateTime/string_formatted_as_decimal, but Read still uses non-nullable parsing (Deserialize<DateTime>/GetDecimal) without null checks, so round-tripping an Option set to null will throw on deserialization.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/EnumTest.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/EnumTest.cs:838">
P2: enum_number is serialized as a floating-point JSON number (1.1/-1.2) but deserialized with GetInt32, which throws on decimal numbers, breaking round-trip deserialization.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/RequiredClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/SourceGeneration/src/Org.OpenAPITools/Model/RequiredClass.cs:2143">
P2: WriteProperties now emits JSON null for optional non-nullable date/datetime fields, but Read still deserializes them as non-nullable DateOnly/DateTime. JsonSerializer.Deserialize<DateOnly>/DateTime throws on null tokens, so the writer can now emit payloads the reader cannot parse, breaking round-trip consistency.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (mixedPropertiesAndAdditionalPropertiesClass.DateTimeOption.Value != null) | ||
| writer.WriteString("dateTime", mixedPropertiesAndAdditionalPropertiesClass.DateTimeOption.Value!.Value.ToString(DateTimeFormat)); | ||
| else | ||
| writer.WriteNull("dateTime"); |
There was a problem hiding this comment.
P2: Converter writes dateTime: null but still deserializes dateTime as non-nullable DateTime, causing round-trip JSON failures.
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/MixedPropertiesAndAdditionalPropertiesClass.cs, line 242:
<comment>Converter writes `dateTime: null` but still deserializes `dateTime` as non-nullable `DateTime`, causing round-trip JSON failures.</comment>
<file context>
@@ -236,18 +236,27 @@ public override void Write(Utf8JsonWriter writer, MixedPropertiesAndAdditionalPr
+ if (mixedPropertiesAndAdditionalPropertiesClass.DateTimeOption.Value != null)
+ writer.WriteString("dateTime", mixedPropertiesAndAdditionalPropertiesClass.DateTimeOption.Value!.Value.ToString(DateTimeFormat));
+ else
+ writer.WriteNull("dateTime");
if (mixedPropertiesAndAdditionalPropertiesClass.MapOption.IsSet)
</file context>
| if (formatTest.StringFormattedAsDecimalOption.Value != null) | ||
| writer.WriteString("string_formatted_as_decimal", formatTest.StringFormattedAsDecimal.ToString()); | ||
| else | ||
| writer.WriteNull("string_formatted_as_decimal"); |
There was a problem hiding this comment.
P2: Serialization now emits null for string_formatted_as_decimal, but deserialization still calls GetDecimal() without a null-token guard, which will throw on JsonTokenType.Null. This creates a runtime mismatch when a null value is written.
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/FormatTest.cs, line 991:
<comment>Serialization now emits null for string_formatted_as_decimal, but deserialization still calls GetDecimal() without a null-token guard, which will throw on JsonTokenType.Null. This creates a runtime mismatch when a null value is written.</comment>
<file context>
@@ -952,16 +985,28 @@ public void WriteProperties(Utf8JsonWriter writer, FormatTest formatTest, JsonSe
+ if (formatTest.StringFormattedAsDecimalOption.Value != null)
+ writer.WriteString("string_formatted_as_decimal", formatTest.StringFormattedAsDecimal.ToString());
+ else
+ writer.WriteNull("string_formatted_as_decimal");
if (formatTest.UnsignedIntegerOption.IsSet)
</file context>
| if (formatTest.DateTimeOption.Value != null) | ||
| writer.WriteString("dateTime", formatTest.DateTimeOption.Value!.Value.ToString(DateTimeFormat)); | ||
| else | ||
| writer.WriteNull("dateTime"); |
There was a problem hiding this comment.
P2: Writing a null dateTime now produces JSON that the reader cannot parse because it deserializes with non-nullable DateTime. This creates a write/read mismatch and will throw on round-trip.
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/FormatTest.cs, line 902:
<comment>Writing a null dateTime now produces JSON that the reader cannot parse because it deserializes with non-nullable DateTime. This creates a write/read mismatch and will throw on round-trip.</comment>
<file context>
@@ -896,15 +896,21 @@ public void WriteProperties(Utf8JsonWriter writer, FormatTest formatTest, JsonSe
+ if (formatTest.DateTimeOption.Value != null)
+ writer.WriteString("dateTime", formatTest.DateTimeOption.Value!.Value.ToString(DateTimeFormat));
+ else
+ writer.WriteNull("dateTime");
if (formatTest.DecimalOption.IsSet)
</file context>
| if (order.ShipDateOption.Value != null) | ||
| writer.WriteString("shipDate", order.ShipDateOption.Value!.Value.ToString(ShipDateFormat)); | ||
| else | ||
| writer.WriteNull("shipDate"); |
There was a problem hiding this comment.
P2: shipDate is now serialized as JSON null, but deserialized as non-nullable DateTime, causing converter round-trip failures.
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/NullReferenceTypes/src/Org.OpenAPITools/Model/Order.cs, line 384:
<comment>`shipDate` is now serialized as JSON null, but deserialized as non-nullable `DateTime`, causing converter round-trip failures.</comment>
<file context>
@@ -354,19 +354,34 @@ public override void Write(Utf8JsonWriter writer, Order order, JsonSerializerOpt
+ if (order.ShipDateOption.Value != null)
+ writer.WriteString("shipDate", order.ShipDateOption.Value!.Value.ToString(ShipDateFormat));
+ else
+ writer.WriteNull("shipDate");
var statusRawValue = Order.StatusEnumToJsonValue(order.StatusOption.Value!.Value);
</file context>
| if (dateOnlyClass.DateOnlyPropertyOption.Value != null) | ||
| writer.WriteString("dateOnlyProperty", dateOnlyClass.DateOnlyPropertyOption.Value!.Value.ToString(DateOnlyPropertyFormat)); | ||
| else | ||
| writer.WriteNull("dateOnlyProperty"); |
There was a problem hiding this comment.
P2: Write now emits JSON null for DateOnlyProperty, but read still deserializes as non-nullable DateOnly, which will throw on a null token. This breaks round-trip for explicitly-null optional values.
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/DateOnlyClass.cs, line 176:
<comment>Write now emits JSON null for DateOnlyProperty, but read still deserializes as non-nullable DateOnly, which will throw on a null token. This breaks round-trip for explicitly-null optional values.</comment>
<file context>
@@ -170,7 +170,10 @@ public override void Write(Utf8JsonWriter writer, DateOnlyClass dateOnlyClass, J
+ if (dateOnlyClass.DateOnlyPropertyOption.Value != null)
+ writer.WriteString("dateOnlyProperty", dateOnlyClass.DateOnlyPropertyOption.Value!.Value.ToString(DateOnlyPropertyFormat));
+ else
+ writer.WriteNull("dateOnlyProperty");
}
}
</file context>
| if (nowGet200Response.NowOption.Value != null) | ||
| writer.WriteString("now", nowGet200Response.NowOption.Value!.Value.ToString(NowFormat)); | ||
| else | ||
| writer.WriteNull("now"); |
There was a problem hiding this comment.
P2: Write now emits null for optional DateTime fields, but Read still deserializes them as non-nullable DateTime, so "now": null / "today": null will throw JsonException and break round-trip.
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/UseDateTimeForDate/src/Org.OpenAPITools/Model/NowGet200Response.cs, line 192:
<comment>Write now emits null for optional DateTime fields, but Read still deserializes them as non-nullable DateTime, so "now": null / "today": null will throw JsonException and break round-trip.</comment>
<file context>
@@ -186,10 +186,16 @@ public override void Write(Utf8JsonWriter writer, NowGet200Response nowGet200Res
+ if (nowGet200Response.NowOption.Value != null)
+ writer.WriteString("now", nowGet200Response.NowOption.Value!.Value.ToString(NowFormat));
+ else
+ writer.WriteNull("now");
if (nowGet200Response.TodayOption.IsSet)
</file context>
| if (dateOnlyClass.DateOnlyPropertyOption.Value != null) | ||
| writer.WriteString("dateOnlyProperty", dateOnlyClass.DateOnlyPropertyOption.Value!.Value.ToString(DateOnlyPropertyFormat)); | ||
| else | ||
| writer.WriteNull("dateOnlyProperty"); |
There was a problem hiding this comment.
P2: WriteProperties can now emit "dateOnlyProperty": null, but Read still deserializes with JsonSerializer.Deserialize, which throws on null for non-nullable value types. This breaks round‑tripping and will fail on null payloads produced by this converter.
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/DateOnlyClass.cs, line 177:
<comment>WriteProperties can now emit "dateOnlyProperty": null, but Read still deserializes with JsonSerializer.Deserialize<DateOnly>, which throws on null for non-nullable value types. This breaks round‑tripping and will fail on null payloads produced by this converter.</comment>
<file context>
@@ -171,7 +171,10 @@ public override void Write(Utf8JsonWriter writer, DateOnlyClass dateOnlyClass, J
+ if (dateOnlyClass.DateOnlyPropertyOption.Value != null)
+ writer.WriteString("dateOnlyProperty", dateOnlyClass.DateOnlyPropertyOption.Value!.Value.ToString(DateOnlyPropertyFormat));
+ else
+ writer.WriteNull("dateOnlyProperty");
}
}
</file context>
| if (formatTest.DateTimeOption.Value != null) | ||
| writer.WriteString("dateTime", formatTest.DateTimeOption.Value!.Value.ToString(DateTimeFormat)); | ||
| else | ||
| writer.WriteNull("dateTime"); |
There was a problem hiding this comment.
P2: WriteProperties now emits explicit null for optional dateTime/string_formatted_as_decimal, but Read still uses non-nullable parsing (Deserialize/GetDecimal) without null checks, so round-tripping an Option set to null will throw on deserialization.
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/NullReferenceTypes/src/Org.OpenAPITools/Model/FormatTest.cs, line 902:
<comment>WriteProperties now emits explicit null for optional dateTime/string_formatted_as_decimal, but Read still uses non-nullable parsing (Deserialize<DateTime>/GetDecimal) without null checks, so round-tripping an Option set to null will throw on deserialization.</comment>
<file context>
@@ -896,15 +896,21 @@ public void WriteProperties(Utf8JsonWriter writer, FormatTest formatTest, JsonSe
+ if (formatTest.DateTimeOption.Value != null)
+ writer.WriteString("dateTime", formatTest.DateTimeOption.Value!.Value.ToString(DateTimeFormat));
+ else
+ writer.WriteNull("dateTime");
if (formatTest.DecimalOption.IsSet)
</file context>
| if (enumTest.EnumNumberOption.IsSet) | ||
| writer.WriteNumber("enum_number", EnumTest.EnumNumberEnumToJsonValue(enumTest.EnumNumberOption.Value!.Value)); | ||
| if (enumTest.EnumNumberOption.Value != null) | ||
| writer.WriteNumber("enum_number", EnumTest.EnumNumberEnumToJsonValue(enumTest.EnumNumberOption.Value!.Value)); |
There was a problem hiding this comment.
P2: enum_number is serialized as a floating-point JSON number (1.1/-1.2) but deserialized with GetInt32, which throws on decimal numbers, breaking round-trip deserialization.
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 838:
<comment>enum_number is serialized as a floating-point JSON number (1.1/-1.2) but deserialized with GetInt32, which throws on decimal numbers, breaking round-trip deserialization.</comment>
<file context>
@@ -822,13 +822,22 @@ public void WriteProperties(Utf8JsonWriter writer, EnumTest enumTest, JsonSerial
if (enumTest.EnumNumberOption.IsSet)
- writer.WriteNumber("enum_number", EnumTest.EnumNumberEnumToJsonValue(enumTest.EnumNumberOption.Value!.Value));
+ if (enumTest.EnumNumberOption.Value != null)
+ writer.WriteNumber("enum_number", EnumTest.EnumNumberEnumToJsonValue(enumTest.EnumNumberOption.Value!.Value));
+ else
+ writer.WriteNull("enum_number");
</file context>
| if (requiredClass.NotRequiredNotnullableDatePropOption.Value != null) | ||
| writer.WriteString("not_required_notnullable_date_prop", requiredClass.NotRequiredNotnullableDatePropOption.Value!.Value.ToString(NotRequiredNotnullableDatePropFormat)); | ||
| else | ||
| writer.WriteNull("not_required_notnullable_date_prop"); |
There was a problem hiding this comment.
P2: WriteProperties now emits JSON null for optional non-nullable date/datetime fields, but Read still deserializes them as non-nullable DateOnly/DateTime. JsonSerializer.Deserialize/DateTime throws on null tokens, so the writer can now emit payloads the reader cannot parse, breaking round-trip consistency.
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/RequiredClass.cs, line 2143:
<comment>WriteProperties now emits JSON null for optional non-nullable date/datetime fields, but Read still deserializes them as non-nullable DateOnly/DateTime. JsonSerializer.Deserialize<DateOnly>/DateTime throws on null tokens, so the writer can now emit payloads the reader cannot parse, breaking round-trip consistency.</comment>
<file context>
@@ -2137,10 +2137,16 @@ public void WriteProperties(Utf8JsonWriter writer, RequiredClass requiredClass,
+ if (requiredClass.NotRequiredNotnullableDatePropOption.Value != null)
+ writer.WriteString("not_required_notnullable_date_prop", requiredClass.NotRequiredNotnullableDatePropOption.Value!.Value.ToString(NotRequiredNotnullableDatePropFormat));
+ else
+ writer.WriteNull("not_required_notnullable_date_prop");
if (requiredClass.NotRequiredNotnullableintegerPropOption.IsSet)
</file context>
|
thanks for the PR cc @devhl-labs |
|
I started a review, but last time I did that I was told others could not see it. Is it visible? Fixes #23190 |
|
Did you "submit" the review? (I don't see it) I think Github allows one to leave a few reviews before submitting them all together. |
|
|
||
| {{/required}} | ||
| {{^required}} | ||
| {{^nrt}} |
There was a problem hiding this comment.
I think you want to use vendorExtensions.x-is-nullable-type which is true for any value type or if nrt is enabled. Also, should ^ be # instead?
| {{/isNullable}} | ||
| {{^isNullable}} | ||
| {{^required}} | ||
| {{#nrt}} |
There was a problem hiding this comment.
Replace nrt and x-is-value-type with vendorExtensions.x-is-nullable-type
| {{#allVars}} | ||
| {{^isNullable}} | ||
| {{#required}} | ||
| if ({{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}.IsSet && {{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}.Value == null) |
There was a problem hiding this comment.
If its required, I don't think it is an option, so the IsSet can go away. Curious that the samples could build.
Fixes #23190
The generator was improperly generating code like this in Read/WriteProperties in the JsonConverter even if the property was marked as non-required and nullable reference types was enabled.
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Summary by cubic
Fix JSON (de)serialization for optional, non-nullable properties in
csharpgenerichostwhen NRT is enabled. Optional properties no longer throw, and writers safely emitnullwhen needed to match C# NRT semantics.JsonConverter.mustacheandWritePropertyHelper.mustacheto only throw for required, non-nullable properties; with NRT enabled, optional properties do not throw and writers useWriteNullfor value-type options whenOption.Valueisnull. When NRT is disabled, previous behavior is preserved.generichostC# samples to reflect the new behavior (removed unnecessary throws; addedWriteNullwhere applicable).Written for commit 9ae7669. Summary will update on new commits.