Repository navigation
Keep REST request names stable across wrapper contracts - #11753
Jorge Rangel (jorgerangel-msft) merged 16 commits into
Conversation
b097b21 to
5962a20
Compare
|
No changes needing a change description found. |
commit: |
There was a problem hiding this comment.
Pull request overview
This PR stabilizes generated REST request-builder names when wrapper contracts preserve legacy Url methods.
Changes:
- Separates wrapper naming from REST request naming.
- Adds regression coverage for stable request generation.
- Updates REST request generation to use stable operation names.
Two critical issues remain in RestClientProvider.cs: an unused local causes a build failure, and normalized names can produce duplicate request method signatures.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Summary |
|---|---|
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/TestBackCompatProviderDoesNotRenameRestRequestMethod/MockableTestResource.cs |
Adds the wrapper back-compatibility fixture. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs |
Adds regression coverage for stable request names. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs |
Uses stable operation names; requires dead-code removal and collision handling. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs |
Centralizes operation-name normalization. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs:250
- For exact-name operations this returns the raw spelling, but REST helper names previously always applied
ToIdentifierName()to the operation name. The existingsnake_case_opexact-name case therefore regresses fromCreateSnakeCaseOpRequesttoCreateSnake_case_opRequest(and similar exact names can produce inconsistent helper identifiers). Keep using the currentOperation.Namefor exact-name/back-compat correctness, but apply the normal identifier conversion when constructing and comparing REST helper names.
if (serviceMethod.IsExactName)
{
return operationName;
…t-rename' into test/repro-wrapper-rest-request-rename
Jorge Rangel (jorgerangel-msft)
left a comment
There was a problem hiding this comment.
can we get a regen preview? Or If CI is not working, a pr from a local regen run?
I did a local regen in #11753 (comment), will get the PR for it. |
|
Refreshed the draft SDK regen PR from the latest
The date/time diffs in the earlier preview have now landed in Azure SDK - by copilot |
Summary
GetUrland consistently call the emittedCreateGetUrlRequestGetUrl, a later wrapper selectsGetUri, and the final rendered REST provider still contains the request captured by the earlier wrapperThis addresses the SDK generation failure described in #11708 where a generated wrapper called a REST request method that was absent from the final emitted provider. Retaining original operation identity also keeps distinct
GetUrlandGetUrioperations mapped to distinct REST request names without set-dependent collision handling.Validation
Microsoft.TypeSpec.Generator.ClientModel.Tests: 1,595 passed