From 66ddafded749735e3b3e30c24588b021ca9adeb8 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 25 Jun 2026 22:18:17 -0700 Subject: [PATCH] [http-client-csharp] Fix protocol method call-site argument order (#60160) When an optional parameter (e.g. an optional path parameter) appears before a required body parameter, the protocol method orders its parameters required-first, which differs from the CreateRequest builder's declaration order. The call site forwarded the protocol parameters positionally, binding them to the wrong CreateRequest parameters (swapping the request body with the optional parameter) and producing malformed request URLs. Map each CreateRequest argument to the protocol parameter with the same name so values are passed in the order the builder expects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...tocol-method-arg-order-2026-6-25-22-0-0.md | 7 ++ .../Providers/ScmMethodProviderCollection.cs | 40 ++++++++++- ...tipartClient_UploadMethods_OptionalBody.cs | 2 +- .../ScmMethodProviderCollectionTests.cs | 70 +++++++++++++++++++ 4 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 .chronus/changes/fix-csharp-protocol-method-arg-order-2026-6-25-22-0-0.md diff --git a/.chronus/changes/fix-csharp-protocol-method-arg-order-2026-6-25-22-0-0.md b/.chronus/changes/fix-csharp-protocol-method-arg-order-2026-6-25-22-0-0.md new file mode 100644 index 00000000000..ef6325c42a7 --- /dev/null +++ b/.chronus/changes/fix-csharp-protocol-method-arg-order-2026-6-25-22-0-0.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-client-csharp" +--- + +Fix protocol method call site passing arguments to the `CreateRequest` method in the wrong order. When an optional parameter (such as an optional path parameter) appeared before a required body parameter, the protocol method reordered its parameters required-first, which differs from the `CreateRequest` method's declaration order. The generated call site now maps each argument to the matching `CreateRequest` parameter by name so the values are passed in the order the request builder expects. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs index 14eb510800c..034d7231e6e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs @@ -1117,7 +1117,7 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod [ UsingDeclare("message", ScmCodeModelGenerator.Instance.TypeFactory.HttpMessageApi.HttpMessageType, This.Invoke(createRequestMethod.Signature, - [.. bodyParameters.Select(p => (ValueExpression)p)]), out var message), + BuildCreateRequestArguments(createRequestMethod.Signature, bodyParameters)), out var message), Return(ScmCodeModelGenerator.Instance.TypeFactory.ClientResponseApi.ToExpression().FromResponse(client .PipelineProperty.Invoke(processMessageName, [message, requestOptionsParameter], isAsync, true, extensionType: _clientPipelineExtensionsDefinition.Type))) ]; @@ -1143,6 +1143,44 @@ [.. bodyParameters.Select(p => (ValueExpression)p)]), out var message), return protocolMethod; } + // The protocol method orders its parameters required-first (optional parameters and the + // request options/context parameter are moved to the end so they can have default values). + // This order can differ from the CreateRequest method's parameter order, which follows the + // operation/declaration order. Passing the protocol parameters positionally would therefore + // bind them to the wrong CreateRequest parameters (for example, swapping an optional path + // parameter with the request body). Reorder the arguments to match the CreateRequest + // signature by mapping each CreateRequest parameter to the protocol parameter with the same + // name. If the names cannot be reconciled, fall back to the original positional behavior. + private static ValueExpression[] BuildCreateRequestArguments( + MethodSignature createRequestSignature, + IReadOnlyList bodyParameters) + { + var createRequestParameters = createRequestSignature.Parameters; + if (createRequestParameters.Count == bodyParameters.Count) + { + var arguments = new ValueExpression[createRequestParameters.Count]; + bool allMatched = true; + for (int i = 0; i < createRequestParameters.Count; i++) + { + var match = bodyParameters.FirstOrDefault(p => p.Name == createRequestParameters[i].Name); + if (match is null) + { + allMatched = false; + break; + } + + arguments[i] = match; + } + + if (allMatched) + { + return arguments; + } + } + + return [.. bodyParameters.Select(p => (ValueExpression)p)]; + } + private ParameterProvider ProcessOptionalParameters( List optionalParameters, List requiredParameters, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/TestMultipartClient_UploadMethods_OptionalBody.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/TestMultipartClient_UploadMethods_OptionalBody.cs index ab30f2c5f4f..2ead02254cf 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/TestMultipartClient_UploadMethods_OptionalBody.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/TestData/ClientProviderTests/TestMultipartClient_UploadMethods_OptionalBody.cs @@ -15,7 +15,7 @@ public partial class MultipartClient { public virtual global::System.ClientModel.ClientResult Upload(string contentType, global::System.ClientModel.BinaryContent content, global::System.ClientModel.Primitives.RequestOptions options = null) { - using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateUploadRequest(contentType, content, options); + using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateUploadRequest(content, contentType, options); return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs index 6bd6a7743c5..535a522f81c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs @@ -611,6 +611,76 @@ public void ProtocolMethodWithOptionalBodyParameter() } } + // Regression test for https://github.com/Azure/azure-sdk-for-net/issues/60160. + // When an optional parameter appears before a required body parameter in the operation + // (e.g. an optional path version followed by the required body), the protocol method + // reorders its parameters required-first, which differs from the CreateRequest method's + // declaration order. The call site must still pass the arguments in the CreateRequest + // method's parameter order, not the protocol method's order. + [Test] + public void ProtocolMethodCallsCreateRequestWithArgumentsInBuilderOrder() + { + MockHelpers.LoadMockGenerator(); + List parameters = + [ + InputFactory.PathParameter( + "name", + InputPrimitiveType.String, + isRequired: true), + InputFactory.PathParameter( + "version", + InputPrimitiveType.String, + isRequired: false), + InputFactory.BodyParameter( + "body", + InputPrimitiveType.String, + isRequired: true), + ]; + List methodParameters = + [ + InputFactory.MethodParameter( + "name", + InputPrimitiveType.String, + isRequired: true, + location: InputRequestLocation.Path), + InputFactory.MethodParameter( + "version", + InputPrimitiveType.String, + isRequired: false, + location: InputRequestLocation.Path), + InputFactory.MethodParameter( + "body", + InputPrimitiveType.String, + isRequired: true, + location: InputRequestLocation.Body), + ]; + var inputOperation = InputFactory.Operation( + "TestOperation", + parameters: parameters); + var inputServiceMethod = InputFactory.BasicServiceMethod("Test", inputOperation, parameters: methodParameters); + var inputClient = InputFactory.Client("TestClient", methods: [inputServiceMethod]); + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + var methodCollection = new ScmMethodProviderCollection(inputServiceMethod, client!); + + var protocolMethod = methodCollection.Single(m => + m.Signature.Parameters.Any(p => p.Name == "options") + && m.Signature.Name == "TestOperation"); + + // The protocol method orders parameters required-first: (name, content, version, options). + var protocolParameterOrder = string.Join(", ", protocolMethod.Signature.Parameters.Select(p => p.Name)); + Assert.AreEqual("name, content, version, options", protocolParameterOrder); + + // The CreateRequest builder keeps declaration order: (name, version, content, options). + var createRequestMethod = client!.RestClient.GetCreateRequestMethod(inputOperation); + var expectedArguments = string.Join(", ", createRequestMethod.Signature.Parameters.Select(p => p.Name)); + Assert.AreEqual("name, version, content, options", expectedArguments); + + // The call site must use the CreateRequest builder's order, not the protocol method's order. + var body = protocolMethod.BodyStatements!.ToDisplayString(); + StringAssert.Contains($"{createRequestMethod.Signature.Name}({expectedArguments})", body); + StringAssert.DoesNotContain($"{createRequestMethod.Signature.Name}(name, content, version, options)", body); + } + [Test] public void OperationWithOptionalEnum() {