fix(http-client-csharp): preserve explicit null for optional nullable properties - #11971
JoshLove-msft wants to merge 2 commits into
Conversation
Track generated nullable property presence independently of its value and preserve absent/null collection states during JSON deserialization. Cover assignment, round trips, inheritance, customization, factory defaults, and dynamic patch precedence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
commit: |
|
No changes needing a change description found. |
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds presence tracking so C# models preserve omitted, explicit null, and non-null states during JSON and wire serialization.
Changes:
- Adds backing fields and presence flags for nullable scalar/model properties.
- Preserves nullable collection and dynamic-model states.
- Adds regenerated samples, regression tests, and customization documentation.
File summaries
| File | Description |
|---|---|
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/SampleTypeSpecModelFactory.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/Thing.Serialization.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/Thing.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/SampleTypeSpecContext.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableProperties.Serialization.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableProperties.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableDynamicProperties.Serialization.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableDynamicProperties.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableContainer.Serialization.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableContainer.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableChild.Serialization.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableChild.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableBase.Serialization.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/OptionalNullableBase.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/NullableDynamicModel.Serialization.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/NullableDynamicModel.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/DynamicModel.Serialization.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/Sample-TypeSpec.tsp | Updated as part of this pull request. |
| packages/http-client-csharp/generator/TestProjects/Local.Tests/ModelSerializationExtensionsTests.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/OptionalNullableCustomPropertyRetainsHandwrittenBehavior/Model.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanReplaceSerializationMethod.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanReplaceDeserializationMethod.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethod.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeDeserializationMethodWithoutOptions.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeDeserializationMethodWithOptions.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/MrwSerializationTypeDefinitionTests/SerializedNameIsUsed(False).cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Xml.cs | Updated as part of this pull request. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs | Updated as part of this pull request. |
| packages/http-client-csharp/.tspd/docs/customization.md | Updated as part of this pull request. |
Review details
Suppressed comments (2)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs:209
- This treats every redeclared inherited property as sharing the base presence flag, even when
ModelProvidermarks the derived membernewfor a domain change (ModelProvider.cs:704-719). The generatednewproperty keeps its field-based setter because this update only rewritesOverridesetters, so assigningnullor a value never sets the shared flag and serialization omits the property; share the flag only for overrides and letnewproperties allocate their own state.
if (property.BaseProperty != null)
{
var baseProperty = BaseModelProvider?.Properties.FirstOrDefault(p => p.Name == property.BaseProperty.Name);
if (baseProperty != null && GetNullablePropertyPresence(baseProperty) is { } basePresence)
{
_nullablePropertyPresence[property] = basePresence;
if (property.Modifiers.HasFlag(MethodSignatureModifiers.Override))
{
property.Update(body: new MethodPropertyBody(
Return(Base.Property(baseProperty.Name)),
property.Body.HasSetter ? Base.Property(baseProperty.Name).Assign(Value).Terminate() : null));
}
}
return;
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs:173
- This reservation only includes generated
m.Fields, but inherited handwritten fields are kept inm.CustomCodeView.Fieldsand merged separately byCanonicalTypeProvider(Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs:246-247,305-306). A base customization with an accessible field such as_textIsDefinedcan therefore receive the same name as a generated presence field in a derived model, producing a field-hiding warning; this generator treats warnings as errors (generator/Directory.Build.props:29). Include inheritedCustomCodeView.Fieldsin the reserved names and add an inheritance/custom-field regression test.
return ancestors.SelectMany(m => m.Fields.Select(f => f.Name));
- Files reviewed: 14/32 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Nullable collection properties lose explicit null presence and serialize as omitted.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 17/37 changed files
- Comments generated: 1
- Review effort level: Lite
| if (presence != null || propertyType is { IsCollection: true, IsReadOnlyMemory: false }) | ||
| { | ||
| if (presence != null) | ||
| { | ||
| isDefinedCondition = presence.As<bool>().Or(isDefinedCondition); |
Summary
Fixes #11970.
Generated model classes now distinguish an untouched optional nullable property from one explicitly assigned
null. Omitted properties remain omitted, explicit null writes JSON null, and non-null values keep their existing representation. JSON deserialization preserves these states for subsequent JSON and wire-format serialization.The generator uses private backing storage and nonpublic presence flags for scalar/model properties, and reuses change-tracking collection state for nullable lists and dictionaries. It preserves inherited properties, including dynamic models with a non-dynamic base, without changing public property types or constructor/factory signatures. Expected projects were regenerated with the supported scripts; generated files were not hand-edited.
Compatibility boundaries
Coverage and validation
Regression coverage includes omitted/null/value states, assignment transitions, model references, strings, integers, booleans, enums, dates, bytes, nullable collection elements, nested wire serialization, both JSON interfaces, round trips, duplicate-property last-value-wins, inheritance, name collisions, handwritten customization, factory defaults, read-only properties, and dynamic patches.
--maxWorkers=4avoids contention against the existing five-second timeout).Generate.ps1, focused final regeneration,npm run build, API extraction, repository/emitter formatting, and C# formatting passed.npm run lintstill references removed ESLint tooling, so the repository's current linter was used instead.No C# changelog/Chronus entry is included.