microsoft / microsoft/typespec

[http-client-csharp] Remove internal models referenced only by BinaryData union metadata

Open
#11,845 0 comments 1 reaction 0 assignees View on GitHub
emitter:client:csharp
Dominant language
Java
Stars
5.9k
Forks
394
Avg merge
1d 23h
Merged PRs (30d)
104

Description

## Description

The C# generator represents TypeSpec unions as `BinaryData` while retaining the original union item types as metadata on `CSharpType`. The provider reference map currently treats that metadata as a reachability edge even when a variant is explicitly internal.

As a result, an internal model whose only inbound reference is an erased `BinaryData` union item is still emitted, even though no generated CLR signature or implementation code uses the model. Public XML documentation may also mention that inaccessible model.

The current behavior is covered by `ProviderReferenceMapAnalyzerTests.BinaryDataUnionPropertyDoesNotPublicizeInternalUnionMembers`, introduced in #11288 during the provider-reference-map migration. The test ensures that the variant is not promoted to public, but also requires `ShouldWriteProvider` to return `true`. Investigation did not find a concrete runtime or product requirement for retaining the model; the assertion appears to preserve migration behavior.

This was discussed in #11839 while fixing loss of union metadata during back-compat type replacement. Metadata preservation and the policy for interpreting that metadata should remain separate concerns.

## Desired behavior

- An explicitly internal variant referenced only through erased `BinaryData` union metadata should not be considered reachable and should not be emitted.
- An internal variant with a genuine implementation dependency—such as generated serialization code, custom code, a discriminator relationship, or another concrete generated reference—should remain internal and be emitted.
- Existing API-contract/baseline retention should continue to apply independently.
- Public union variants and their public-surface reachability should remain unaffected.
- Public XML documentation should not reference internal or removed union variants.

## Test coverage

Replace or split `BinaryDataUnionPropertyDoesNotPublicizeInternalUnionMembers` so coverage independently verifies:

1. An internal variant referenced only by `BinaryData` union metadata is not written.
2. An internal variant with a real implementation reference remains internal and is written.
3. Public XML documentation excludes internal or removed union variants.

## Context

- #11839
- #11288

- by copilot

Contributor guide

Open the contributing guide

Research direction

Start with ProviderReferenceMapAnalyzerTests.BinaryDataUnionPropertyDoesNotPublicizeInternalUnionMembers and run its existing test. Trace how the provider reference map and ShouldWriteProvider interpret BinaryData union metadata, then split coverage for metadata-only, genuine implementation, and XML documentation cases. Done means internal metadata-only variants are not written while real dependencies and public variants remain covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.