Skip to content

Commit 1546f61

Browse files
m-nashCopilotJoshLove-msft
authored
Fix partial method customization parameter renaming (#11058)
## Problem When a hand-written `partial` method declaration customizes a generated method (the [documented "Customize a generated client method's signature"](https://github.com/microsoft/typespec/blob/main/packages/http-client-csharp/.tspd/docs/customization.md) feature), the generated partial implementation comes out with parameter names that don't compile. `TypeProvider.CreatePartialMethodFromCustomSignature` passed `customSignature.Parameters` as **both** arguments to `RenameAndCloneParameters`. The intended contract is `(generatorParameters, customParameters)`: the first carries the generator metadata and the parameter declarations referenced by the method **body** and **XML docs**; the second only supplies the names. Because the custom parameters were used for the signature, the signature ended up referencing different `ParameterProvider`/`CodeWriterDeclaration` instances than the body and XML docs. The writer's name de-duplication then appended a numeric suffix, e.g.: ```csharp private static partial void MyMethod(string input) { input0.ToString(); // signature says `input`, body says `input0` } ``` ## Fix Pass `generatedMethod.Signature.Parameters` as the generator parameters so the signature, body, and XML docs all share the same parameter declarations. This matches the contract documented on `RenameAndCloneParameters` and the existing correct usage in `ScmMethodProviderCollection.BuildConvenienceMethod`. ## Tests - New regression test `CustomPartialMethodImplementationKeepsParameterNames` (fails before the fix showing `input0`, passes after). - Full suites green: 1543 `Microsoft.TypeSpec.Generator` tests + 1436 `Microsoft.TypeSpec.Generator.ClientModel` tests. ## Docs Added a note to `customization.md`: types generated into the same assembly must be fully qualified with `global::` in a partial method declaration, since they are unresolved symbols when the declaration is read. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: jolov <jolov@microsoft.com>
1 parent f904b5f commit 1546f61

5 files changed

Lines changed: 121 additions & 4 deletions

File tree

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,18 @@ public static IReadOnlyList<ParameterProvider> RenameAndCloneParameters(
166166
/// <param name="customSignature">The customer's partial declaration signature.</param>
167167
/// <param name="implementationParameters">The parameters to use for the implementation.
168168
/// Must all be required (no default values) per the C# partial method rules.</param>
169+
/// <param name="returnType">The return type the implementation should use. Pass the
170+
/// generator's own return type rather than relying on the customer's parsed declaration:
171+
/// the customer's declaration may reference types generated into the same assembly, which
172+
/// are unresolved when the declaration is read (Roslyn surfaces them as error types with no
173+
/// namespace), producing malformed <c>global::.TypeName</c> output. C# requires a partial
174+
/// method's declaration and implementation to share the same return type, so the generator's
175+
/// resolved return type is necessarily the correct one. When <c>null</c>, the customer's
176+
/// parsed return type is used as a fallback.</param>
169177
public static MethodSignature BuildPartialSignature(
170178
MethodSignature customSignature,
171-
IReadOnlyList<ParameterProvider> implementationParameters)
179+
IReadOnlyList<ParameterProvider> implementationParameters,
180+
CSharpType? returnType = null)
172181
{
173182
if (customSignature is null)
174183
{
@@ -184,7 +193,7 @@ public static MethodSignature BuildPartialSignature(
184193
customSignature.Name,
185194
customSignature.Description,
186195
customSignature.Modifiers | MethodSignatureModifiers.Partial,
187-
customSignature.ReturnType,
196+
returnType ?? customSignature.ReturnType,
188197
customSignature.ReturnDescription,
189198
implementationParameters,
190199
customSignature.Attributes,

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,12 +413,17 @@ internal MethodProvider[] FilterCustomizedMethods(IEnumerable<MethodProvider> sp
413413
private static MethodProvider CreatePartialMethodFromCustomSignature(MethodSignature customSignature, MethodProvider generatedMethod)
414414
{
415415
// Partial method implementations require all parameters to be required (no default values).
416+
// The generator's parameters carry the metadata and the declarations referenced by the
417+
// method body and XML docs; the custom signature only supplies the parameter names.
416418
var requiredParameters = PartialMethodCustomization.RenameAndCloneParameters(
417-
customSignature.Parameters,
419+
generatedMethod.Signature.Parameters,
418420
customSignature.Parameters,
419421
removeDefaults: true);
420422

421-
var partialSignature = PartialMethodCustomization.BuildPartialSignature(customSignature, requiredParameters);
423+
var partialSignature = PartialMethodCustomization.BuildPartialSignature(
424+
customSignature,
425+
requiredParameters,
426+
generatedMethod.Signature.ReturnType);
422427

423428
MethodProvider partialMethod = generatedMethod.BodyExpression != null
424429
? new MethodProvider(partialSignature, generatedMethod.BodyExpression, generatedMethod.EnclosingType, generatedMethod.XmlDocs, generatedMethod.Suppressions)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#nullable disable
2+
3+
namespace Test;
4+
5+
public partial class CustomPartialMethodType
6+
{
7+
static partial void MyMethod(string input);
8+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#nullable disable
2+
3+
namespace Test;
4+
5+
public partial class CustomPartialReturnType
6+
{
7+
// CustomReturnModel is a type generated into the same assembly, so it is unresolved when this
8+
// declaration is read. The generated implementation must still use the generator's resolved
9+
// return type (with its namespace) rather than this unresolved one.
10+
static partial CustomReturnModel MyMethod(string input);
11+
}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,90 @@ public async Task TestSpecViewReturnsAllMethodsEvenWhenCustomized()
433433
Assert.AreEqual("Method3", specView.Methods[2].Signature.Name);
434434
}
435435

436+
[Test]
437+
public async Task CustomPartialMethodImplementationKeepsParameterNames()
438+
{
439+
await MockHelpers.LoadMockGeneratorAsync(compilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
440+
441+
var inputParam = new ParameterProvider("input", $"The input value.", typeof(string));
442+
var generatedMethod = new MethodProvider(
443+
new MethodSignature(
444+
"MyMethod",
445+
$"Does something.",
446+
MethodSignatureModifiers.Static,
447+
null,
448+
null,
449+
[inputParam]),
450+
inputParam.Invoke("ToString").Terminate(),
451+
new TestTypeProvider());
452+
453+
var typeProvider = new TestTypeProvider(name: "CustomPartialMethodType", methods: [generatedMethod]);
454+
455+
var method = typeProvider.Methods.Single();
456+
Assert.IsTrue(method.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Partial));
457+
458+
using var codeWriter = new CodeWriter();
459+
codeWriter.WriteMethod(method);
460+
var result = codeWriter.ToString(false);
461+
462+
// The partial implementation's signature parameter must keep the same name the body uses.
463+
// Regression guard: the parameter must not be renamed with a numeric suffix (e.g. "input0").
464+
Assert.IsFalse(
465+
result.Contains("input0"),
466+
$"Partial implementation renamed the parameter with a numeric suffix:\n{result}");
467+
Assert.AreEqual("input", method.Signature.Parameters.Single().Name);
468+
}
469+
470+
[Test]
471+
public async Task CustomPartialMethodImplementationUsesGeneratorReturnType()
472+
{
473+
await MockHelpers.LoadMockGeneratorAsync(compilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
474+
475+
// The generator's resolved return type carries a namespace. The custom partial
476+
// declaration references the same type by name, but it is unresolved when read (a
477+
// type generated into the same assembly), so its parsed CSharpType has no namespace.
478+
var generatorReturnType = new CSharpType(
479+
"CustomReturnModel",
480+
"Sample.Models",
481+
isValueType: false,
482+
isNullable: false,
483+
declaringType: null,
484+
args: [],
485+
isPublic: true,
486+
isStruct: false);
487+
488+
var inputParam = new ParameterProvider("input", $"The input value.", typeof(string));
489+
var generatedMethod = new MethodProvider(
490+
new MethodSignature(
491+
"MyMethod",
492+
$"Does something.",
493+
MethodSignatureModifiers.Static,
494+
generatorReturnType,
495+
$"The result.",
496+
[inputParam]),
497+
inputParam.Invoke("ToString").Terminate(),
498+
new TestTypeProvider());
499+
500+
var typeProvider = new TestTypeProvider(name: "CustomPartialReturnType", methods: [generatedMethod]);
501+
502+
var method = typeProvider.Methods.Single();
503+
Assert.IsTrue(method.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Partial));
504+
505+
// The implementation must use the generator's resolved return type (with namespace),
506+
// not the customer's unresolved parsed return type (empty namespace).
507+
Assert.AreEqual("Sample.Models", method.Signature.ReturnType!.Namespace);
508+
Assert.AreEqual("CustomReturnModel", method.Signature.ReturnType.Name);
509+
510+
using var codeWriter = new CodeWriter();
511+
codeWriter.WriteMethod(method);
512+
var result = codeWriter.ToString(false);
513+
514+
// Regression guard: an empty-namespace return type renders as the malformed `global::.`.
515+
Assert.IsFalse(
516+
result.Contains("global::."),
517+
$"Partial implementation wrote a return type with no namespace:\n{result}");
518+
}
519+
436520
[Test]
437521
public async Task TestSpecViewReturnsAllPropertiesEvenWhenSuppressed()
438522
{

0 commit comments

Comments
 (0)