Skip to content

[http-client-csharp] Prevent List/Get request-builder collisions - #12183

Draft
Jorge Rangel (jorgerangel-msft) with Copilot wants to merge 19 commits into
mainfrom
copilot/http-client-csharp-fix-duplicate-request-builders
Draft

Jorge Rangel (jorgerangel-msft) with Copilot wants to merge 19 commits into
mainfrom
copilot/http-client-csharp-fix-duplicate-request-builders

Conversation

Copilot AI commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

List-to-Get normalization collapsed singleton GET and pageable LIST operations onto the same REST request-builder signature, causing CS0111 despite their distinct request paths.

  • Collision handling: Check stable input names and projected C# signatures before applying List-to-Get normalization. Retain the original List name when REST or public declarations would collide, including generated Async variants.
  • Compatibility: Preserve non-colliding overloads and existing explicit, published-API, acronym, and URL naming. Compare generic arguments recursively so IEnumerable<int> and IEnumerable<int?> remain distinct signatures.
  • Regression coverage: Cover operation order, explicit-name peers, List/GetAll and async-name collisions, valid overloads, unique declarations, and URI/lookup/initial-page/next-page binding.
GetByResourceGroup  → CreateGetByResourceGroupRequest  → …/singletonTrackedResources/default
ListByResourceGroup → CreateListByResourceGroupRequest → …/singletonTrackedResources

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

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 Oct 14, 2026
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix duplicate REST request builders for getByResourceGroup and listByResourceGroup [http-client-csharp] Prevent List/Get request-builder collisions Oct 14, 2026
@pkg-pr-new

pkg-pr-new Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 7beecf7

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

False collision checks can unnecessarily rename existing APIs for exact-name peers and async-only streaming operations.

2 open findings
What changed in this PR

Prevents C# List-to-Get normalization from creating duplicate request builders for distinct operations, addressing #12175.

Changes:

  • Checks projected names and parameter signatures before normalization.
  • Compares generic parameter types recursively.
  • Adds regression coverage for collisions, valid overloads, and request binding.
File Description
packages/​http-client-csharp/​generator/​Microsoft.TypeSpec.Generator/​src/​Shared/​MethodSignatureHelper.cs Recursively compares generic arguments.
packages/​http-client-csharp/​generator/​Microsoft.TypeSpec.Generator.ClientModel/​test/​Providers/​ClientProviders/​ClientProviderTests.cs Tests naming collisions, overloads, and request binding.
packages/​http-client-csharp/​generator/​Microsoft.TypeSpec.Generator.ClientModel/​src/​Providers/​ClientProvider.cs Adds collision-aware List normalization.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Public sync/async comparisons and initial-versus-next-page builder checks still miss collisions that produce duplicate C# declarations.

2 open issues
2 resolved since last review

Review effort: Balanced

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Compatibility-preserved parameter ordering can still produce duplicate REST request builders despite the new collision checks.

1 open issue
2 resolved since last review

Review effort: Balanced

…request-builder collisions

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Compatibility ordering still uses the pre-normalized name during public signature checks, allowing duplicate async declarations after renaming.

1 open issue
1 resolved since last review

Review effort: Balanced

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Compatibility-sensitive naming changes warrant human validation of downstream API stability, and the signature-projection performance concern remains unresolved.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Skip disjoint peers before rebuilding operation parameters

packages/​http-client-csharp/​generator/​Microsoft.TypeSpec.Generator.ClientModel/​src/​Providers/​ClientProvider.cs:429

Skip peers with disjoint emitted names before projecting their parameters: CleanOperationNames invokes this path twice per List operation (ClientProvider.cs:220–224), but both protocol variants and convenience parameters are rebuilt for every unrelated peer. This adds quadratic parameter construction and repeated compatibility lookups to client creation even when no names can collide; intersect GetEmittedMethodNames first and return early.

1 resolved since last review

Review effort: Balanced

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot overall, the changes are way too complicated. Ideally, we simplify the PR and fix the problem we are trying to solve. If we can target the changes to simply avoid collisions based on method names, that may reduce a lot of the complexity we are adding.

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

Copilot AI commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Copilot overall, the changes are way too complicated. Ideally, we simplify the PR and fix the problem we are trying to solve. If we can target the changes to simply avoid collisions based on method n...

Simplified the collision handling to use emitted method names rather than parameter signatures, removed the projection machinery, and restored the existing REST/protocol parameter-building paths. Commit: 7beecf78.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Name-only collision checks rename valid existing overloads, breaking generated client APIs despite distinct parameter signatures.

1 open issue

Review effort: Balanced

This branch has not been deployed

No deployments
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.

[http-client-csharp] List/Get normalization creates duplicate REST request builders

3 participants