Repository navigation
[csharp][generichost] Refactor custom DateTime formats (dateFormat and dateTimeFormat options are deprecated) - #25213
Merged
wing328 merged 1 commit intoOct 12, 2026
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
12 issues found across 451 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. 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/MixedPropertiesAndAdditionalPropertiesClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/NullReferenceTypes/src/Org.OpenAPITools/Model/MixedPropertiesAndAdditionalPropertiesClass.cs:266">
P3: This block is misindented relative to the surrounding property writers. Align its braces and statements with the neighboring `map` block.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net4.8/OneOf/src/Org.OpenAPITools/Client/DateTimeFormatHelper.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net4.8/OneOf/src/Org.OpenAPITools/Client/DateTimeFormatHelper.cs:24">
P3: `ReadDateTime` and `ReadDateTimeOffset` duplicate the token check and error handling. Consider a shared private helper that takes the parsed result. Also, the 'date-time' wording is inaccurate for `x-csharp-date-format` (date-only) properties, so the message should be generic, for example 'date or date-time'.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/HostConfiguration.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/HostConfiguration.mustache:43">
P1: `System.Text.Json` does not support `DateOnly` natively on .NET 6, so removing this registration leaves standard `DateOnly` properties unsupported in generated .NET 6 clients. Keep a fallback converter for targets before built-in support.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/UseDateTimeForDate/src/Org.OpenAPITools/Model/NowGet200Response.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/UseDateTimeForDate/src/Org.OpenAPITools/Model/NowGet200Response.cs:200">
P3: The new `{ ... }` blocks wrapping the `WritePropertyName`/`JsonSerializer.Serialize` pair are emitted with broken indentation: the opening brace sits at 16 spaces, `writer.WritePropertyName("now")` at 28, `JsonSerializer.Serialize(...)` at 32, and the closing brace at 28. The same defect appears in all five regenerated models (DateOnlyClass, MixedPropertiesAndAdditionalPropertiesClass, NullableClass, Order, and this file). Root cause is in `JsonConverter.mustache` (changed in this PR): the `{{^vendorExtensions.x-csharp-date-format}}` branch nests `{{#lambda.indent1}}` inside `{{#lambda.copyText}}`, and the levels compound when the copied text is re-indented at the write site, so statements inside one block land at different depths and the closing `}` misaligns with its opening `{`. In NullableClass this also leaves the dangling `else` (which correctly binds to the inner `if`) visually aligned with the outer `if`, making the branch structure misleading. Emit the block with a single consistent indent level (e.g. `{`/`}` at the guard's indent, both statements one level deeper).</violation>
<violation number="2" location="samples/client/petstore/csharp/generichost/net8/UseDateTimeForDate/src/Org.OpenAPITools/Model/NowGet200Response.cs:207">
P1: This serializes the `today` date as a date-time value (for example, `2024-01-02T00:00:00`) instead of the required `yyyy-MM-dd` date string. Preserve the date-only format when writing `Today`.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/DateOnlyClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/DateOnlyClass.cs:180">
P3: The block added for `dateOnlyProperty` is misindented: the braces and statements do not line up with the enclosing `if`. Format it like the rest of the generated code. The source is likely the converter template, so fix it there too or the next regeneration will restore the bad formatting.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net4.8/Petstore/src/Org.OpenAPITools/Model/Order.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net4.8/Petstore/src/Org.OpenAPITools/Model/Order.cs:411">
P3: The new shipDate block has broken brace and indentation layout: the `{` and `}` are misaligned and the statements are indented 28–32 spaces. Match the other properties in `WriteProperties`, and fix the generator template so regenerated output is clean.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net4.7/FormModels/src/Org.OpenAPITools/Model/DateOnlyClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net4.7/FormModels/src/Org.OpenAPITools/Model/DateOnlyClass.cs:182">
P1: This serializes the `DateTime` as an ISO timestamp instead of the date-only `yyyy-MM-dd` value this property previously emitted. Preserve date-only formatting here so generated requests keep the OpenAPI `date` wire format.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net4.7/Petstore/src/Org.OpenAPITools/Model/FormatTest.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net4.7/Petstore/src/Org.OpenAPITools/Model/FormatTest.cs:1015">
P1: This serializes the `date` property as a full DateTime timestamp instead of the required date-only value. Keep writing this property in `yyyy-MM-dd` form so generated clients do not emit a value that violates the date contract.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/NullableClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/NullableClass.cs:433">
P3: The braces and indentation in the new `date_prop` and `datetime_prop` write blocks are misaligned with the rest of this method. Align the `{` and `}` with the enclosing `if` and indent the body one level, matching the array and object blocks. Since this file is generated, check the template that emits these blocks so regeneration does not reintroduce the misalignment.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net4.8/FormModels/src/Org.OpenAPITools/Model/NullableClass.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net4.8/FormModels/src/Org.OpenAPITools/Model/NullableClass.cs:434">
P1: `DateProp` is an OpenAPI date, but this serializes its `DateTime` as a full timestamp instead of the previous date-only value. Preserve date-only output for this net4.8 target so generated requests remain schema-compatible.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/DateTimeFormatHelper.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/DateTimeFormatHelper.mustache:22">
P2: `ReadDateTime` passes `AdjustToUniversal`, so custom formats that include an offset specifier (`zzz`, `z`, `K`) cannot round-trip: the offset is normalized to UTC on read and written back as `+00:00`/`Z`, changing the original string. This is also asymmetric with `ReadDateTimeOffset`, which preserves offsets. For offset-bearing formats, route through the `DateTimeOffset` reader; otherwise document the UTC normalization as the intended contract.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
View guided diff | Re-trigger cubic
| _jsonOptions.Converters.Add(new DateTimeJsonConverter()); | ||
| _jsonOptions.Converters.Add(new DateTimeNullableJsonConverter()); | ||
| {{#supportsDateOnly}} | ||
| _jsonOptions.Converters.Add(new DateOnlyJsonConverter()); |
Contributor
There was a problem hiding this comment.
P1: System.Text.Json does not support DateOnly natively on .NET 6, so removing this registration leaves standard DateOnly properties unsupported in generated .NET 6 clients. Keep a fallback converter for targets before built-in support.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/openapi-generator/src/main/resources/csharp/libraries/generichost/HostConfiguration.mustache, line 43:
<comment>`System.Text.Json` does not support `DateOnly` natively on .NET 6, so removing this registration leaves standard `DateOnly` properties unsupported in generated .NET 6 clients. Keep a fallback converter for targets before built-in support.</comment>
<file context>
@@ -37,12 +37,6 @@ namespace {{packageName}}.{{clientPackage}}
{
_services = services;
_jsonOptions.Converters.Add(new JsonStringEnumConverter());
- _jsonOptions.Converters.Add(new DateTimeJsonConverter());
- _jsonOptions.Converters.Add(new DateTimeNullableJsonConverter());
- {{#supportsDateOnly}}
- _jsonOptions.Converters.Add(new DateOnlyJsonConverter());
- _jsonOptions.Converters.Add(new DateOnlyNullableJsonConverter());
- {{/supportsDateOnly}}
</file context>
| writer.WriteString("today", nowGet200Response.TodayOption.Value!.Value.ToString(TodayFormat)); | ||
| { | ||
| writer.WritePropertyName("today"); | ||
| JsonSerializer.Serialize(writer, nowGet200Response.TodayOption.Value!.Value, jsonSerializerOptions); |
Contributor
There was a problem hiding this comment.
P1: This serializes the today date as a date-time value (for example, 2024-01-02T00:00:00) instead of the required yyyy-MM-dd date string. Preserve the date-only format when writing Today.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net8/UseDateTimeForDate/src/Org.OpenAPITools/Model/NowGet200Response.cs, line 207:
<comment>This serializes the `today` date as a date-time value (for example, `2024-01-02T00:00:00`) instead of the required `yyyy-MM-dd` date string. Preserve the date-only format when writing `Today`.</comment>
<file context>
@@ -206,10 +196,16 @@ public void WriteProperties(Utf8JsonWriter writer, NowGet200Response nowGet200Re
- writer.WriteString("today", nowGet200Response.TodayOption.Value!.Value.ToString(TodayFormat));
+ {
+ writer.WritePropertyName("today");
+ JsonSerializer.Serialize(writer, nowGet200Response.TodayOption.Value!.Value, jsonSerializerOptions);
+ }
}
</file context>
Suggested change
| JsonSerializer.Serialize(writer, nowGet200Response.TodayOption.Value!.Value, jsonSerializerOptions); | |
| writer.WriteStringValue(nowGet200Response.TodayOption.Value!.Value.ToString("yyyy'-'MM'-'dd", System.Globalization.CultureInfo.InvariantCulture)); |
| writer.WriteString("dateOnlyProperty", dateOnlyClass.DateOnlyPropertyOption.Value.Value.ToString(DateOnlyPropertyFormat)); | ||
| { | ||
| writer.WritePropertyName("dateOnlyProperty"); | ||
| JsonSerializer.Serialize(writer, dateOnlyClass.DateOnlyPropertyOption.Value.Value, jsonSerializerOptions); |
Contributor
There was a problem hiding this comment.
P1: This serializes the DateTime as an ISO timestamp instead of the date-only yyyy-MM-dd value this property previously emitted. Preserve date-only formatting here so generated requests keep the OpenAPI date wire format.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net4.7/FormModels/src/Org.OpenAPITools/Model/DateOnlyClass.cs, line 182:
<comment>This serializes the `DateTime` as an ISO timestamp instead of the date-only `yyyy-MM-dd` value this property previously emitted. Preserve date-only formatting here so generated requests keep the OpenAPI `date` wire format.</comment>
<file context>
@@ -182,7 +177,10 @@ public void WriteProperties(Utf8JsonWriter writer, DateOnlyClass dateOnlyClass,
- writer.WriteString("dateOnlyProperty", dateOnlyClass.DateOnlyPropertyOption.Value.Value.ToString(DateOnlyPropertyFormat));
+ {
+ writer.WritePropertyName("dateOnlyProperty");
+ JsonSerializer.Serialize(writer, dateOnlyClass.DateOnlyPropertyOption.Value.Value, jsonSerializerOptions);
+ }
}
</file context>
Suggested change
| JsonSerializer.Serialize(writer, dateOnlyClass.DateOnlyPropertyOption.Value.Value, jsonSerializerOptions); | |
| writer.WriteStringValue(dateOnlyClass.DateOnlyPropertyOption.Value.Value.ToString("yyyy'-'MM'-'dd")); |
| writer.WriteString("date", formatTest.Date.ToString(DateFormat)); | ||
| { | ||
| writer.WritePropertyName("date"); | ||
| JsonSerializer.Serialize(writer, formatTest.Date, jsonSerializerOptions); |
Contributor
There was a problem hiding this comment.
P1: This serializes the date property as a full DateTime timestamp instead of the required date-only value. Keep writing this property in yyyy-MM-dd form so generated clients do not emit a value that violates the date contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net4.7/Petstore/src/Org.OpenAPITools/Model/FormatTest.cs, line 1015:
<comment>This serializes the `date` property as a full DateTime timestamp instead of the required date-only value. Keep writing this property in `yyyy-MM-dd` form so generated clients do not emit a value that violates the date contract.</comment>
<file context>
@@ -1020,7 +1010,10 @@ public void WriteProperties(Utf8JsonWriter writer, FormatTest formatTest, JsonSe
- writer.WriteString("date", formatTest.Date.ToString(DateFormat));
+ {
+ writer.WritePropertyName("date");
+ JsonSerializer.Serialize(writer, formatTest.Date, jsonSerializerOptions);
+ }
</file context>
Suggested change
| JsonSerializer.Serialize(writer, formatTest.Date, jsonSerializerOptions); | |
| writer.WriteStringValue(formatTest.Date.ToString("yyyy'-'MM'-'dd", System.Globalization.CultureInfo.InvariantCulture)); |
Comment on lines
+434
to
+435
| writer.WritePropertyName("date_prop"); | ||
| JsonSerializer.Serialize(writer, nullableClass.DatePropOption.Value.Value, jsonSerializerOptions); |
Contributor
There was a problem hiding this comment.
P1: DateProp is an OpenAPI date, but this serializes its DateTime as a full timestamp instead of the previous date-only value. Preserve date-only output for this net4.8 target so generated requests remain schema-compatible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net4.8/FormModels/src/Org.OpenAPITools/Model/NullableClass.cs, line 434:
<comment>`DateProp` is an OpenAPI date, but this serializes its `DateTime` as a full timestamp instead of the previous date-only value. Preserve date-only output for this net4.8 target so generated requests remain schema-compatible.</comment>
<file context>
@@ -440,13 +430,19 @@ public void WriteProperties(Utf8JsonWriter writer, NullableClass nullableClass,
if (nullableClass.DatePropOption.Value != null)
- writer.WriteString("date_prop", nullableClass.DatePropOption.Value.Value.ToString(DatePropFormat));
+ {
+ writer.WritePropertyName("date_prop");
+ JsonSerializer.Serialize(writer, nullableClass.DatePropOption.Value.Value, jsonSerializerOptions);
+ }
</file context>
Suggested change
| writer.WritePropertyName("date_prop"); | |
| JsonSerializer.Serialize(writer, nullableClass.DatePropOption.Value.Value, jsonSerializerOptions); | |
| writer.WriteString("date_prop", nullableClass.DatePropOption.Value.Value.ToString("yyyy'-'MM'-'dd", System.Globalization.CultureInfo.InvariantCulture)); |
| /// <summary> | ||
| /// Deserializes a DateTime using an exact format. | ||
| /// </summary> | ||
| public static DateTime ReadDateTime(ref Utf8JsonReader reader, string format) |
Contributor
There was a problem hiding this comment.
P3: ReadDateTime and ReadDateTimeOffset duplicate the token check and error handling. Consider a shared private helper that takes the parsed result. Also, the 'date-time' wording is inaccurate for x-csharp-date-format (date-only) properties, so the message should be generic, for example 'date or date-time'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net4.8/OneOf/src/Org.OpenAPITools/Client/DateTimeFormatHelper.cs, line 24:
<comment>`ReadDateTime` and `ReadDateTimeOffset` duplicate the token check and error handling. Consider a shared private helper that takes the parsed result. Also, the 'date-time' wording is inaccurate for `x-csharp-date-format` (date-only) properties, so the message should be generic, for example 'date or date-time'.</comment>
<file context>
@@ -0,0 +1,51 @@
+ /// <summary>
+ /// Deserializes a DateTime using an exact format.
+ /// </summary>
+ public static DateTime ReadDateTime(ref Utf8JsonReader reader, string format)
+ {
+ if (reader.TokenType != JsonTokenType.String)
</file context>
Comment on lines
+180
to
+183
| { | ||
| writer.WritePropertyName("dateOnlyProperty"); | ||
| JsonSerializer.Serialize(writer, dateOnlyClass.DateOnlyPropertyOption.Value.Value, jsonSerializerOptions); | ||
| } |
Contributor
There was a problem hiding this comment.
P3: The block added for dateOnlyProperty is misindented: the braces and statements do not line up with the enclosing if. Format it like the rest of the generated code. The source is likely the converter template, so fix it there too or the next regeneration will restore the bad formatting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/DateOnlyClass.cs, line 180:
<comment>The block added for `dateOnlyProperty` is misindented: the braces and statements do not line up with the enclosing `if`. Format it like the rest of the generated code. The source is likely the converter template, so fix it there too or the next regeneration will restore the bad formatting.</comment>
<file context>
@@ -182,7 +177,10 @@ public void WriteProperties(Utf8JsonWriter writer, DateOnlyClass dateOnlyClass,
if (dateOnlyClass.DateOnlyPropertyOption.IsSet)
- writer.WriteString("dateOnlyProperty", dateOnlyClass.DateOnlyPropertyOption.Value.Value.ToString(DateOnlyPropertyFormat));
+ {
+ writer.WritePropertyName("dateOnlyProperty");
+ JsonSerializer.Serialize(writer, dateOnlyClass.DateOnlyPropertyOption.Value.Value, jsonSerializerOptions);
</file context>
Suggested change
| { | |
| writer.WritePropertyName("dateOnlyProperty"); | |
| JsonSerializer.Serialize(writer, dateOnlyClass.DateOnlyPropertyOption.Value.Value, jsonSerializerOptions); | |
| } | |
| { | |
| writer.WritePropertyName("dateOnlyProperty"); | |
| JsonSerializer.Serialize(writer, dateOnlyClass.DateOnlyPropertyOption.Value.Value, jsonSerializerOptions); | |
| } |
Comment on lines
+411
to
+414
| { | ||
| writer.WritePropertyName("shipDate"); | ||
| JsonSerializer.Serialize(writer, order.ShipDateOption.Value.Value, jsonSerializerOptions); | ||
| } |
Contributor
There was a problem hiding this comment.
P3: The new shipDate block has broken brace and indentation layout: the { and } are misaligned and the statements are indented 28–32 spaces. Match the other properties in WriteProperties, and fix the generator template so regenerated output is clean.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net4.8/Petstore/src/Org.OpenAPITools/Model/Order.cs, line 411:
<comment>The new shipDate block has broken brace and indentation layout: the `{` and `}` are misaligned and the statements are indented 28–32 spaces. Match the other properties in `WriteProperties`, and fix the generator template so regenerated output is clean.</comment>
<file context>
@@ -413,7 +408,10 @@ public void WriteProperties(Utf8JsonWriter writer, Order order, JsonSerializerOp
if (order.ShipDateOption.IsSet)
- writer.WriteString("shipDate", order.ShipDateOption.Value.Value.ToString(ShipDateFormat));
+ {
+ writer.WritePropertyName("shipDate");
+ JsonSerializer.Serialize(writer, order.ShipDateOption.Value.Value, jsonSerializerOptions);
</file context>
Suggested change
| { | |
| writer.WritePropertyName("shipDate"); | |
| JsonSerializer.Serialize(writer, order.ShipDateOption.Value.Value, jsonSerializerOptions); | |
| } | |
| { | |
| writer.WritePropertyName("shipDate"); | |
| JsonSerializer.Serialize(writer, order.ShipDateOption.Value.Value, jsonSerializerOptions); | |
| } |
Comment on lines
+433
to
+436
| { | ||
| writer.WritePropertyName("date_prop"); | ||
| JsonSerializer.Serialize(writer, nullableClass.DatePropOption.Value.Value, jsonSerializerOptions); | ||
| } |
Contributor
There was a problem hiding this comment.
P3: The braces and indentation in the new date_prop and datetime_prop write blocks are misaligned with the rest of this method. Align the { and } with the enclosing if and indent the body one level, matching the array and object blocks. Since this file is generated, check the template that emits these blocks so regeneration does not reintroduce the misalignment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/NullableClass.cs, line 433:
<comment>The braces and indentation in the new `date_prop` and `datetime_prop` write blocks are misaligned with the rest of this method. Align the `{` and `}` with the enclosing `if` and indent the body one level, matching the array and object blocks. Since this file is generated, check the template that emits these blocks so regeneration does not reintroduce the misalignment.</comment>
<file context>
@@ -440,13 +430,19 @@ public void WriteProperties(Utf8JsonWriter writer, NullableClass nullableClass,
if (nullableClass.DatePropOption.IsSet)
if (nullableClass.DatePropOption.Value != null)
- writer.WriteString("date_prop", nullableClass.DatePropOption.Value.Value.ToString(DatePropFormat));
+ {
+ writer.WritePropertyName("date_prop");
+ JsonSerializer.Serialize(writer, nullableClass.DatePropOption.Value.Value, jsonSerializerOptions);
</file context>
Suggested change
| { | |
| writer.WritePropertyName("date_prop"); | |
| JsonSerializer.Serialize(writer, nullableClass.DatePropOption.Value.Value, jsonSerializerOptions); | |
| } | |
| { | |
| writer.WritePropertyName("date_prop"); | |
| JsonSerializer.Serialize(writer, nullableClass.DatePropOption.Value.Value, jsonSerializerOptions); | |
| } |
| @@ -206,10 +196,16 @@ public void WriteProperties(Utf8JsonWriter writer, NowGet200Response nowGet200Re | |||
| throw new JsonException("Cannot write null property NowGet200Response.Today to non-nullable JSON property 'today'."); | |||
|
|
|||
| if (nowGet200Response.NowOption.IsSet) | |||
| writer.WriteString("now", nowGet200Response.NowOption.Value!.Value.ToString(NowFormat)); | |||
| { | |||
| writer.WritePropertyName("now"); | |||
Contributor
There was a problem hiding this comment.
P3: The new { ... } blocks wrapping the WritePropertyName/JsonSerializer.Serialize pair are emitted with broken indentation: the opening brace sits at 16 spaces, writer.WritePropertyName("now") at 28, JsonSerializer.Serialize(...) at 32, and the closing brace at 28. The same defect appears in all five regenerated models (DateOnlyClass, MixedPropertiesAndAdditionalPropertiesClass, NullableClass, Order, and this file). Root cause is in JsonConverter.mustache (changed in this PR): the {{^vendorExtensions.x-csharp-date-format}} branch nests {{#lambda.indent1}} inside {{#lambda.copyText}}, and the levels compound when the copied text is re-indented at the write site, so statements inside one block land at different depths and the closing } misaligns with its opening {. In NullableClass this also leaves the dangling else (which correctly binds to the inner if) visually aligned with the outer if, making the branch structure misleading. Emit the block with a single consistent indent level (e.g. {/} at the guard's indent, both statements one level deeper).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/client/petstore/csharp/generichost/net8/UseDateTimeForDate/src/Org.OpenAPITools/Model/NowGet200Response.cs, line 200:
<comment>The new `{ ... }` blocks wrapping the `WritePropertyName`/`JsonSerializer.Serialize` pair are emitted with broken indentation: the opening brace sits at 16 spaces, `writer.WritePropertyName("now")` at 28, `JsonSerializer.Serialize(...)` at 32, and the closing brace at 28. The same defect appears in all five regenerated models (DateOnlyClass, MixedPropertiesAndAdditionalPropertiesClass, NullableClass, Order, and this file). Root cause is in `JsonConverter.mustache` (changed in this PR): the `{{^vendorExtensions.x-csharp-date-format}}` branch nests `{{#lambda.indent1}}` inside `{{#lambda.copyText}}`, and the levels compound when the copied text is re-indented at the write site, so statements inside one block land at different depths and the closing `}` misaligns with its opening `{`. In NullableClass this also leaves the dangling `else` (which correctly binds to the inner `if`) visually aligned with the outer `if`, making the branch structure misleading. Emit the block with a single consistent indent level (e.g. `{`/`}` at the guard's indent, both statements one level deeper).</comment>
<file context>
@@ -206,10 +196,16 @@ public void WriteProperties(Utf8JsonWriter writer, NowGet200Response nowGet200Re
if (nowGet200Response.NowOption.IsSet)
- writer.WriteString("now", nowGet200Response.NowOption.Value!.Value.ToString(NowFormat));
+ {
+ writer.WritePropertyName("now");
+ JsonSerializer.Serialize(writer, nowGet200Response.NowOption.Value!.Value, jsonSerializerOptions);
+ }
</file context>
wing328
added
Breaking change (with fallback)
Client: C-Sharp
Enhancement: Code Cleanup
wing328
changed the title
Member
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
2 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.
Summary
This refactors date and date-time serialization for the C# Generic Host library.
Previously, the generated converters could deserialize several custom formats but always serialized values using the single global
dateFormatordateTimeFormat. A JSON round trip therefore lost the property's original format and could even produce a value that the same configuration could not deserialize.System.Text.Jsonalready has built-in support for the standard JSON representations ofDateTime,DateTimeOffset,DateOnlyon supported target frameworks, and their nullable forms. Standard temporal values now use that native support, so the generated global date, date-time, and nullable converters have been removed.Property-specific nonstandard formats can now be declared with:
x-csharp-date-formatx-csharp-datetime-formatGenerated model converters use the specified format for both deserialization and serialization, preserving the custom format across JSON round trips and providing a symmetric contract. Exact custom-format parsing is implemented by the generated
DateTimeFormatHelper, using invariant culture.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.
Summary by cubic
Refactors date and date-time serialization in the C# Generic Host client so a JSON round trip no longer loses a property's original custom format.
Previously, generated converters deserialized several custom formats but always serialized using the single global
dateFormatordateTimeFormat, so output could not be read back by the same configuration. Standard temporal values now rely on the built-inSystem.Text.Jsonsupport forDateTime,DateTimeOffset, andDateOnly(including nullable forms), and the generated global date, date-time, and nullable converters have been removed. Nonstandard formats are declared per property withx-csharp-date-formatorx-csharp-datetime-format, and generated model converters use the declared format symmetrically for both deserialization and serialization. Exact parsing is implemented by the new generatedDateTimeFormatHelperusing invariant culture.Migration
dateFormatanddateTimeFormatgenerator options are removed; declare formats per property with the new vendor extensions instead.Written for commit 3c10708. Summary will update on new commits.