Skip to content

Only use full namespace for API version enum when there are collisions#10108

Open
Copilot wants to merge 8 commits intomainfrom
copilot/update-api-version-enum-names
Open

Only use full namespace for API version enum when there are collisions#10108
Copilot wants to merge 8 commits intomainfrom
copilot/update-api-version-enum-names

Conversation

Copy link
Contributor

Copilot AI commented Mar 20, 2026

PR #10061 unconditionally used the full namespace for API version enum names in multi-service scenarios. This produces unnecessarily verbose names when services have distinct last namespace segments. This PR changes the logic to only use the full namespace when there's an actual collision, and restores the ClientHelper.BuildNameForService helper for proper prefix/suffix-aware naming. The collision-aware naming applies to all multi-service generated members — enum type names, version property names, and latest version field names all use BuildNameForService by default, falling back to full namespace only when last segments collide. The enum type naming follows the {X}ServiceVersion pattern (e.g., KeyVaultServiceVersion) rather than Service{X}Version.

Behavior

Namespaces Member Before (PR #10061) After (no collision) After (collision)
Sample.KeyVault, Sample.Storage Enum type SampleKeyVaultVersion KeyVaultServiceVersion
Sample.KeyVault, Sample.Storage Version property SampleKeyVaultApiVersion KeyVaultApiVersion
Sample.KeyVault, Sample.Storage Latest field LatestServiceKeyVaultVersion LatestKeyVaultVersion
Azure.One.Tests, Azure.Two.Tests Enum type AzureOneTestsVersion AzureOneTestsVersion (collision on Tests)
Azure.One.Tests, Azure.Two.Tests Version property AzureOneTestsApiVersion AzureOneTestsApiVersion (collision on Tests)

Changes

  • ClientHelper.cs — restored BuildNameForService helper (removed in PR Fix duplicate API version field names for multi-service clients with colliding namespaces #10061) that extracts the last namespace segment and ensures proper prefix/suffix without duplication; added shared GetLastNamespaceSegment and HasLastSegmentCollision static helpers
  • ClientHelperTests.cs — restored all 22 tests for BuildNameForService
  • ApiVersionEnumProvider.BuildName() — uses ClientHelper.BuildNameForService with empty prefix and "ServiceVersion" suffix for the non-collision case, producing {X}ServiceVersion names; falls back to full namespace only when last segments collide; delegates to shared ClientHelper methods instead of local helpers
  • ClientOptionsProvider.BuildVersionProperties() — uses ClientHelper.BuildNameForService for version property names in the non-collision case; falls back to full namespace only when last segments collide
  • ClientOptionsProvider.BuildLatestVersionsFields() — uses ClientHelper.BuildNameForService for latest version field names in the non-collision case; falls back to full namespace only when last segments collide
  • Tests — updated assertions and expected test data files (ClientOptionsProvider, ClientProvider, RestClientProvider) for member name changes in non-collision cases; added MultiServiceClient_WithCollidingLastSegments_UsesFullNamespace test and MultiServiceClient_SameLastSegment_ProducesUniqueVersionEnums test with TestData golden file validation
Original prompt

This section details on the original issue you should resolve

<issue_title>Only Use Full Namespace for Api Version Enum When There are Conflicts</issue_title>
<issue_description>Follow up to #10061.

This PR introduced a change to avoid collisions in the generated api version enums in the multi-service scenario by using the full namespace of the enum as the name. We should update this to only do this whenever there is a collision in the generated names.</issue_description>

Comments on the Issue (you are @copilot in this section)


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

…lisions

Use last namespace segment for api version enum names in multi-service
scenarios by default. Fall back to full namespace only when two or more
services share the same last namespace segment, avoiding unnecessary
prefix noise while still guaranteeing uniqueness.

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Mar 20, 2026
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copilot AI changed the title [WIP] Update enum naming for api version conflicts Only use full namespace for API version enum when there are collisions Mar 20, 2026
Copilot AI requested a review from jorgerangel-msft March 20, 2026 15:14
… naming

Re-adds ClientHelper.BuildNameForService and its tests that were removed
in PR #10061. Uses BuildNameForService for the non-collision case in both
ApiVersionEnumProvider.BuildName() and
ClientOptionsProvider.BuildVersionProperties(), falling back to the full
namespace only when last segments collide.

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@jorgerangel-msft
Copy link
Contributor

@copilot we also need to ensure we regenerate all the test projects.

…ct helpers, add TestData validation

- Revert ClientOptionsProvider.BuildVersionProperties() to use full namespace (field names unchanged)
- Revert ClientProviderTests assertions to original field names
- Move GetLastNamespaceSegment and HasLastSegmentCollision to ClientHelper as shared static helpers
- Update ApiVersionEnumProvider.BuildName() to use shared ClientHelper methods
- Add TestData golden file validation for collision test
- Update test data files: only enum type names change, property/field names stay the same

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 20, 2026

@copilot we also need to ensure we regenerate all the test projects.

Ran eng/scripts/Generate.ps1 to regenerate all test projects. No files changed — all existing test projects are single-service, so the multi-service naming logic doesn't affect them. All unit tests pass (ApiVersionEnumProviderTests: 8, ClientHelperTests: 22, ClientOptionsProviderTests: 31, ClientProviderTests: 238).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • telemetry.astro.build
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/typespec/typespec/website/node_modules/.bin/../astro/bin/astro.mjs build sh pec/�� ebsite/src/conte--llmstxt node /.bin/node --no-emit bash n/sh sh /nod�� rs &amp;&amp; pnpm build:src node /.bin/sh build&#39;);process.node l/reference dist/node-gyp-bidoc dirname (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copy link
Contributor

@jorgerangel-msft jorgerangel-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot lets refactor our approach here. Lets only use the naming changes from #10061 when the generated members will collide. Otherwise, lets default to the previous approach prior to the PR which was to use the client helper to create the member name.

…enum type, version properties, fields)

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/4bb660ba-ec46-42ad-8074-4e13314801b6
Copy link
Contributor Author

Copilot AI commented Mar 20, 2026

@copilot lets refactor our approach here. Lets only use the naming changes from #10061 when the generated members will collide. Otherwise, lets default to the...

Refactored to apply BuildNameForService for ALL multi-service members (enum type, version properties, field names) when there's no collision. Full namespace naming from PR #10061 is now only used as fallback when last segments collide. Updated 10 test data files and 2 test assertions, and regenerated all test projects (no changes since they're all single-service). See commit 10024bc.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • telemetry.astro.build
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/typespec/typespec/website/node_modules/.bin/../astro/bin/astro.mjs build node pec/�� tput-dir ../../w--llmstxt sh .2_tmp_5782_0/no../../website/src/content/docs/docs/emitters/openapi3/reference --no-emit ture /.bin/sh node /nod�� rs &amp;&amp; pnpm build:src tsconfig.build.json 0/x64/lib/node_modules/npm/node_--output-dir build&#39;);process.node l/reference /.bin/node node (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copy link
Contributor

@jorgerangel-msft jorgerangel-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot lets update the MultiServiceClient_SameLastSegment_ProducesUniqueVersionEnums test in ClientOptionsProviderTests to add TestData validation. We should not remove the existing assertions in the test

…roducesUniqueVersionEnums

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/d68247ba-74e0-495e-9cd6-961dcb2cd21b
Copy link
Contributor Author

Copilot AI commented Mar 20, 2026

@copilot lets update the MultiServiceClient_SameLastSegment_ProducesUniqueVersionEnums test in ClientOptionsProviderTests to add TestData validation. We should not remove the existing assertions in t...

Added TestData golden file validation to MultiServiceClient_SameLastSegment_ProducesUniqueVersionEnums while keeping the existing assertions. See commit e81dc12.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 20, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10108

commit: 352b1da

@jorgerangel-msft jorgerangel-msft marked this pull request as draft March 20, 2026 20:17
Copy link
Contributor

@jorgerangel-msft jorgerangel-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot lets change the default naming of the ServiceVersion enum in the multiservice client scenario. Instead of ServiceXVersion lets do XServiceVersion. So services Foo and Bar in a client would have members FooServiceVersion and BarServiceVersion

…erviceVersion

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/1abc462d-6e01-4045-857d-007426a4d797
Copy link
Contributor Author

Copilot AI commented Mar 20, 2026

@copilot lets change the default naming of the ServiceVersion enum in the multiservice client scenario. Instead of ServiceXVersion lets do XServiceVersion. So services Foo and Bar in a client would ha...

Changed the enum naming from Service{X}Version to {X}ServiceVersion. The BuildName() call now uses BuildNameForService(serviceNamespace, string.Empty, ApiVersionEnumName) which produces KeyVaultServiceVersion, StorageServiceVersion, etc. Updated 4 golden test data files and 2 test assertions. All 299 tests pass. See commit 352b1da.

@jorgerangel-msft jorgerangel-msft marked this pull request as ready for review March 20, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only Use Full Namespace for Api Version Enum When There are Collisions

2 participants