fix(http-client-csharp): correct return type namespace and parameter names in partial method customization#11057
Closed
JoshLove-msft wants to merge 2 commits into
Closed
Conversation
…names in partial method customization Fixes two bugs in the partial method customization feature: - Return types rendered with an empty namespace (global::.TypeName) because the partial implementation was built from the customer's parsed declaration, whose return type references not-yet-generated models (Roslyn error types with no namespace). - A '0' was appended to parameter names when the signature and body referenced different ParameterProvider instances with the same name. BuildPartialSignature now accepts the generator's resolved return type, and the ScmMethodProviderCollection call sites pass the generator return type and clone from the generator's parameters (resolved types), sharing instances between signature and body. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JoshLove-msft
requested review from
ShivangiReja,
jorgerangel-msft,
joseharriaga,
jsquire and
m-nash
as code owners
microsoft-github-policy-service
Bot
added
the
emitter:client:csharp
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
commit: |
Contributor
|
No changes needing a change description found. |
|
You can try these changes here
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two bugs in the C# emitter's partial method customization feature, observed in Azure/azure-sdk-for-net#59749:
0appended to parameter names — e.g.resourceGroupResource0. This happens when the method signature and body reference differentParameterProviderinstances that share the same name, triggering the writer's disambiguation.Fix
BuildPartialSignaturenow accepts an optionalCSharpType? returnTypeand usesreturnType ?? customSignature.ReturnType, so callers can supply the generator's resolved return type instead of the customer's parsed (unresolved) one.ScmMethodProviderCollection(convenience + protocol paths) now pass the generator's resolved return type (GetResponseType(...)) and clone parameters from the generator's resolved parameters while taking the customer's names. The protocol body shares the same renamed parameter instances as the signature, preventing the0suffix.Tests
PartialMethodCustomizationTests(3 tests) verifying the generator return type is used, the null fallback, and that cloned parameters keep generator types with custom names.ClientProviderCustomizationTests(18 tests) still pass.Note: the management generator (in azure-sdk-for-net) is the actual consumer producing the buggy output; these shared helpers now enable correct usage, and this repo's data-plane paths are correct.