[http-client-csharp] Preserve last-contract model base types - #11818
[http-client-csharp] Preserve last-contract model base types#11818Wei Hu (live1206) wants to merge 20 commits into
Conversation
commit: |
|
No changes needing a change description found. |
|
Regen: Azure/azure-sdk-for-net#62636 |
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core model inheritance resolution and constructor-chaining constraints in the generator, which is high-impact even with good regression coverage.
Pull request overview
This PR extends the http-client-csharp generator’s last-contract back-compat logic to preserve a model’s previously shipped CLR base type when the current TypeSpec base hierarchy no longer contains it, while guarding against cases that would produce invalid C# (e.g., conflicting partial bases or inaccessible constructor chaining).
Changes:
- Update
ModelProviderbase-type selection to restore last-contract bases when safe, retain promoted bases when already derived, and resolve preserved bases against the current build. - Add constructor-accessibility checks for symbol-backed base types and emit a new diagnostic for incompatible custom-base conflicts.
- Add targeted regression tests and test data for generated, referenced, and framework base-type preservation scenarios.
File summaries
| File | Description |
|---|---|
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ReferencedLastContractBaseWithInternalParameterlessConstructorIsNotRestored(LastContract)/Models.cs | Adds last-contract baseline source for an external base with an internal parameterless ctor (should not be restored). |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored(LastContract)/Models.cs | Adds last-contract baseline source for a non-generated base lacking a parameterless ctor (should not be restored). |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored(Current)/ExternalBase.cs | Adds current-build external base type used to validate constructor-chaining constraints. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_LastContractBaseRestoresInheritedProperties/Models.cs | Adds baseline types to validate inherited properties remain inherited and are not duplicated. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase(LastContract)/Models.cs | Adds last-contract baseline for a previous base used in the custom-base precedence scenario. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase(Current)/Models.cs | Adds current-build custom base partial to validate partial-base conflict handling. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_BaseTypeChangePreservesNonGeneratedLastContractBaseType/DerivedModel.cs | Adds baseline derived model inheriting from a framework type (e.g., System.Exception). |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_BaseTypeChangePreservesLastContractBaseType/Models.cs | Adds baseline types to validate restoring a previously shipped generated base. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs | Adds regression tests covering base restoration, promoted bases, inherited properties, custom-base conflicts, and ctor-accessibility constraints. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/DiagnosticCodes.cs | Introduces incompatible-backcompat-base-type diagnostic code. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs | Adds HasAccessibleParameterlessConstructor to validate ctor accessibility across assembly boundaries. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs | Implements last-contract base-type preservation logic and resolves preserved bases against the current compilation/providers. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs | Adds display label for the new back-compat change category. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs | Adds ModelBaseTypePreserved category for back-compat reporting. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core back-compat inheritance selection logic with broad API-surface implications that warrants final human verification beyond the added regressions.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The base-restoration property-collision guard currently treats private base properties as collisions, which can incorrectly block restoring a last-contract base type.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs:391
- HasDirectPropertyNameCollision treats private properties on a symbol-backed/base type as collisions. In C#, private base members are not inherited, so they cannot cause the CS0108 hiding warning/error and shouldn’t block restoring the last-contract base; this can incorrectly skip base restoration and break assignability compatibility.
- Files reviewed: 21/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The change alters core model inheritance/back-compat behavior with many edge-case guards (resolution, constructors, collisions, discriminator hierarchies) that warrants final human review despite strong regression coverage.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
latest regen: Azure/azure-sdk-for-net#62687 |
There was a problem hiding this comment.
🟡 Changes recommended
There is a C# compile issue in ScmModelProvider where an override reduces method accessibility, which must be corrected before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces non-trivial base-type reconciliation logic that can affect broad model/serialization behaviors, so it warrants final human review despite strong regression coverage.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The back-compat base restoration logic is guarded for known invalid-code cases and is covered by targeted regressions (including compilation checks) plus documentation updates.
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core model-type construction/back-compat behavior in the generator (including symbol/metadata resolution and inheritance/collision rules) and warrants a final human review despite strong regression coverage.
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
This intentionally does not materialize properties introduced only by a displaced, newer TypeSpec base. That broader reconciliation can be added when an SDK use case requires it.
Fixes #11816
Validation