Fix C# reference-map accessibility for hidden union types#11288
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
commit: |
|
No changes needing a change description found. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
Expected Azure SDK regeneration diffThe combined
There are no GA public API changes. The only accessibility change is the VoiceLive preview-only surface that was not present in GA 1.1.0. |
There was a problem hiding this comment.
Pull request overview
This PR updates the C# generator’s provider reference-map analysis to preserve established public API surface (including last GA contract/public nested types) while preventing BinaryData-erased unions from incorrectly “publicizing” internal/abstract union members, and it fixes request-header helper retention by deriving collection-header helper dependencies from the selected HttpRequestApi.
Changes:
- Extend reference-map graph construction to distinguish union item references that affect public surface vs metadata-only union items (erased to
BinaryData) and to root types that were public in the last contract. - Improve discriminator-derived reachability handling for “known variant” cases (including intermediate bases and nested scenarios).
- Make collection-header helper retention conditional on
HttpRequestApicapabilities, avoiding generation/retention of unusedPipelineRequestHeadersExtensionsfor non-pipeline request APIs; add positive/negative test coverage.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/ReferenceMap/ProviderReferenceMapAnalyzerTests.cs | Adds coverage for BinaryData/metadata-only unions, discriminator/nested accessibility, and last-contract rooting. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs | Verifies unverifiable additional-properties types are treated as metadata-only unions. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/ReferenceMap/ProviderReferenceMapAnalyzer.Helpers.cs | Adds last-contract public rooting helpers and threads union-item exclusions into custom-code rooting. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/ReferenceMap/ProviderReferenceMapAnalyzer.cs | Integrates union-item exclusions into public graph analysis and roots last-contract public types. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/ReferenceMap/ProviderReferenceMapAnalyzer.Candidates.cs | Roots last-contract public types during removal; refines discriminator reachability propagation. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/ReferenceMap/ProviderReferenceMapAnalyzer.BodyReferences.cs | Adds union-item exclusion handling and respects UnionItemTypeReferenceKind during reference traversal. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs | Marks unverifiable types as metadata-only unions (to avoid retaining them as real dependencies). |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Primitives/CSharpType.cs | Introduces UnionItemTypeReferenceKind and plumbs it through union construction/cloning. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/TestHelpers/MockHelpers.cs | Allows injecting a custom HttpRequestApi in ClientModel tests. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ReferenceMap/ClientBodyDependencyReferenceMapTests.cs | Adds negative test ensuring scalar headers don’t retain PipelineRequestHeadersExtensions. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs | Adds coverage for request-API-dependent collection-header helper retention. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs | Uses HttpRequestApi capability to decide which collection-header helper type (if any) to retain. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/PipelineRequestProvider.cs | Implements the request-API helper capability for the default pipeline request API. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/Abstractions/HttpRequestApi.cs | Adds GetCollectionHeaderHelperType() capability hook (default: none). |
Co-authored-by: jolov <jolov@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
|
Revalidated the combined Azure/unbranded regeneration from a fresh worktree at latest |
Preserve the reference-map follow-up changes while taking the latest main branch before restoring the reverted implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
Revert the main-branch rollback from microsoft#11282 while preserving the accessibility and SDK-compatibility fixes already on microsoft#11288. Keep the shared internal helper provider from microsoft#11285, but let provider-graph reachability determine whether helpers are emitted. This reverts commit 0540648. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
|
latest regen: Azure/azure-sdk-for-net#61111 |
jorgerangel-msft
left a comment
There was a problem hiding this comment.
It looks like there is at least 1 change in the regen preview that doesn't compile: details
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
|
Follow-up for the Azure SDK full-regeneration build failure in Azure/azure-sdk-for-net#61111:
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
|
Restored #11285’s intentional generic |
|
latest regen: Azure/azure-sdk-for-net#61150 |
JoshLove-msft
left a comment
There was a problem hiding this comment.
Reference-map correctness findings.
--generated by Copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4b68173-2f2b-467d-9a94-9be9d14b5e17
|
latest regen: Azure/azure-sdk-for-net#61203 |
Summary
BinaryDatafrom semantic public and implementation dependenciesHttpRequestApi, avoiding unusedPipelineRequestHeadersExtensionsoutputInternalHelperProvidermodifiers and lazy helper constructionFixes #11291.
Main-branch reconciliation
This branch merges current
mainand explicitly reverts squash commit05406488bfrom #11282. The existing #11288 commits remain intact as follow-up commits rather than being rebased or squashed away.Validation
RegenPreview.ps1: 46 passed, 0 failed, with no untracked files or unexpected request helpersmain; the resulting 72-file expected patch was byte-for-byte identical