From cf4469d4b3d026bb4a9826ad4713b8ae73cba205 Mon Sep 17 00:00:00 2001 From: Michael Nash Date: Mon, 3 Aug 2026 14:41:15 -0700 Subject: [PATCH 1/5] Apply @@override options bags to protocol methods When `@@override` groups an operation's parameters into an options bag, the protocol method now adopts the same grouped shape instead of listing every parameter individually. The protocol method keeps taking raw request content when the body is declared outside the bag. If the body itself was folded into the bag, the protocol method stays flattened so a raw payload can still be sent. Fixes #11214 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 69b166be-7096-4882-9251-6e998d9288ae --- .../src/Providers/RestClientProvider.cs | 40 +++- .../Providers/ScmMethodProviderCollection.cs | 107 +++++++++- .../ScmMethodProviderCollectionTests.cs | 191 ++++++++++++++++++ ...mGroupedQueryParam_SerializesToProtocol.cs | 2 +- ...namedGroupedQueryParam_MapsByClientName.cs | 2 +- 5 files changed, 336 insertions(+), 6 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs index 8b2ff5859b3..172c8876111 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs @@ -1221,8 +1221,13 @@ internal static List GetMethodParameters( int optional = 400; var operation = serviceMethod.Operation; - // For convenience methods, use the service method parameters - var inputParameters = methodType is ScmMethodKind.Convenience ? serviceMethod.Parameters : operation.Parameters; + // Convenience methods use the service method parameters. The protocol method does too when + // @@override grouped the operation's parameters into an options bag, so both surfaces share + // the same shape (https://github.com/microsoft/typespec/issues/11214). + var inputParameters = methodType is ScmMethodKind.Convenience + || (methodType is ScmMethodKind.Protocol && ShouldGroupProtocolParameters(serviceMethod)) + ? serviceMethod.Parameters + : operation.Parameters; var pageSizeParameterName = GetPageSizeParameterName(serviceMethod as InputPagingServiceMethod); @@ -1307,7 +1312,7 @@ internal static List GetMethodParameters( if (methodType is ScmMethodKind.Protocol or ScmMethodKind.CreateRequest) { - if (inputParam is InputBodyParameter) + if (inputParam is InputBodyParameter || inputParam is InputMethodParameter { Location: InputRequestLocation.Body }) { if (methodType == ScmMethodKind.CreateRequest) { @@ -1393,6 +1398,35 @@ internal static List GetMethodParameters( return [.. sortedParams.Values]; } + /// + /// Determines whether the protocol method should adopt the grouped (options bag) parameter shape + /// produced by @@override. Grouping is skipped when the request body itself was folded into + /// the bag, because the protocol method must keep exposing the body as raw request content. + /// + internal static bool ShouldGroupProtocolParameters(InputServiceMethod serviceMethod) + { + bool hasGroupedParameter = false; + foreach (var parameter in serviceMethod.Operation.Parameters) + { + if (parameter.MethodParameterSegments is not { Count: > 1 } segments) + { + continue; + } + + // The bag is (or contains) the request body, so the protocol method has to stay flattened + // to keep accepting a raw payload. + if (parameter is InputBodyParameter + || segments[0] is InputMethodParameter { Location: InputRequestLocation.Body }) + { + return false; + } + + hasGroupedParameter = true; + } + + return hasGroupedParameter; + } + private static bool HasLiteralContentTypeHeader(InputOperation operation) { foreach (var p in operation.Parameters) 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 d039522a128..c2767092b22 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 @@ -1112,6 +1112,14 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod requestOptionsParameter = ScmKnownParameters.OptionalRequestOptions; } + // A grouped options bag adopted by the protocol method can carry the same name as the request + // options/context parameter, which would emit a duplicate parameter name. Rename the request + // options parameter when that happens. + requestOptionsParameter = ResolveRequestOptionsNameCollision( + requestOptionsParameter, + requiredParameters, + optionalParameters); + ParameterProvider[] parameters = [.. requiredParameters, .. optionalParameters, requestOptionsParameter]; var methodName = isAsync ? ServiceMethod.Name + "Async" : ServiceMethod.Name; @@ -1196,6 +1204,38 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod return protocolMethod; } + private static ParameterProvider ResolveRequestOptionsNameCollision( + ParameterProvider requestOptionsParameter, + IReadOnlyList requiredParameters, + IReadOnlyList optionalParameters) + { + var otherParameters = requiredParameters.Concat(optionalParameters).ToList(); + if (!otherParameters.Any(p => string.Equals(p.Name, requestOptionsParameter.Name, StringComparison.OrdinalIgnoreCase))) + { + return requestOptionsParameter; + } + + var baseName = "request" + + char.ToUpperInvariant(requestOptionsParameter.Name[0]) + + requestOptionsParameter.Name.Substring(1); + var uniqueName = baseName; + var suffix = 1; + while (otherParameters.Any(p => string.Equals(p.Name, uniqueName, StringComparison.OrdinalIgnoreCase))) + { + uniqueName = baseName + suffix++; + } + + return new ParameterProvider( + uniqueName, + requestOptionsParameter.Description, + requestOptionsParameter.Type, + requestOptionsParameter.DefaultValue, + location: requestOptionsParameter.Location, + wireInfo: requestOptionsParameter.WireInfo, + validation: requestOptionsParameter.Validation, + inputParameter: requestOptionsParameter.InputParameter); + } + // 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 @@ -1204,11 +1244,24 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod // 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( + private ValueExpression[] BuildCreateRequestArguments( MethodSignature createRequestSignature, IReadOnlyList bodyParameters) { var createRequestParameters = createRequestSignature.Parameters; + + // When the protocol method exposes an options bag, its parameters no longer line up with the + // CreateRequest method's flattened wire parameters. Expand each grouped parameter back out of + // the bag (e.g. `options.Top`) so CreateRequest still receives the individual values. + if (RestClientProvider.ShouldGroupProtocolParameters(ServiceMethod)) + { + var groupedArguments = BuildGroupedCreateRequestArguments(createRequestParameters, bodyParameters); + if (groupedArguments is not null) + { + return groupedArguments; + } + } + if (createRequestParameters.Count == bodyParameters.Count) { var arguments = new ValueExpression[createRequestParameters.Count]; @@ -1234,6 +1287,58 @@ private static ValueExpression[] BuildCreateRequestArguments( return [.. bodyParameters.Select(p => (ValueExpression)p)]; } + private static ValueExpression[]? BuildGroupedCreateRequestArguments( + IReadOnlyList createRequestParameters, + IReadOnlyList protocolParameters) + { + var arguments = new ValueExpression[createRequestParameters.Count]; + + for (int i = 0; i < createRequestParameters.Count; i++) + { + var createRequestParameter = createRequestParameters[i]; + var segments = createRequestParameter.InputParameter?.MethodParameterSegments; + + if (segments is not { Count: > 1 }) + { + var match = protocolParameters.FirstOrDefault( + p => string.Equals(p.Name, createRequestParameter.Name, StringComparison.OrdinalIgnoreCase)); + if (match is null) + { + return null; + } + + arguments[i] = match; + continue; + } + + var groupParameter = protocolParameters.FirstOrDefault( + p => string.Equals(p.Name, segments[0].Name, StringComparison.OrdinalIgnoreCase)); + if (groupParameter is null + || !ScmCodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(groupParameter.Type, out var typeProvider) + || typeProvider is not ModelProvider groupModel) + { + return null; + } + + var propertySegments = segments.Skip(1).Select(s => s.Name).ToList(); + var propertyExpression = groupModel.GetPropertyExpression(groupParameter, propertySegments, out var leafProperty); + + // CreateRequest takes the serialized (string/number) form of an enum, so convert before forwarding. + if (leafProperty.Type.IsEnum && !createRequestParameter.Type.IsEnum) + { + if (leafProperty.Type.IsNullable) + { + propertyExpression = propertyExpression.NullConditional(); + } + propertyExpression = leafProperty.Type.ToSerial(propertyExpression); + } + + arguments[i] = propertyExpression; + } + + return arguments; + } + private ParameterProvider ProcessOptionalParameters( List optionalParameters, List requiredParameters, 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 c26f86c02c4..fa5ddb8b1ae 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 @@ -1729,6 +1729,14 @@ public async Task MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientN // name ("bandIndex"). var methodBody = convenienceMethod!.BodyStatements!.ToDisplayString(); Assert.AreEqual(Helpers.GetExpectedFromFile(), methodBody); + + // The protocol method takes the same options bag, so the client-name lookup now happens + // when it expands the bag for the CreateRequest call. + var protocolMethod = methodCollection.FirstOrDefault(m => + m.Signature.Name == "GetPoint" && + m.Signature.Parameters.All(p => p.Type.Name != "CancellationToken")); + Assert.IsNotNull(protocolMethod); + Assert.That(protocolMethod!.BodyStatements!.ToDisplayString(), Does.Contain("options.BandIndex")); } [Test] @@ -1793,6 +1801,189 @@ public async Task MethodParameterSegments_EnumGroupedQueryParam_SerializesToProt var methodBody = convenienceMethod!.BodyStatements!.ToDisplayString(); Assert.AreEqual(Helpers.GetExpectedFromFile(), methodBody); + + // The protocol method takes the same options bag, so the enum serialization now happens + // when it expands the bag for the CreateRequest call, which still takes a string. + var protocolMethod = methodCollection.FirstOrDefault(m => + m.Signature.Name == "GetPoint" && + m.Signature.Parameters.All(p => p.Type.Name != "CancellationToken")); + Assert.IsNotNull(protocolMethod); + Assert.That(protocolMethod!.BodyStatements!.ToDisplayString(), Does.Contain("options.Resampling?.ToString()")); + } + + [Test] + public async Task OptionsBagOverride_AppliesToProtocolMethod() + { + // https://github.com/microsoft/typespec/issues/11214 + // When @@override groups an operation's parameters into an options bag, the protocol + // method should adopt the same grouped shape instead of listing every parameter. + var optionsModel = InputFactory.Model( + "GetWidgetOptions", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: true, isHttpMetadata: true, wireName: "id"), + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + InputFactory.Property("top", InputPrimitiveType.Int32, isRequired: false, isHttpMetadata: true, wireName: "top"), + ]); + + var optionsMethodParameter = InputFactory.MethodParameter( + "options", + optionsModel, + isRequired: true, + location: InputRequestLocation.Query); + + var idParam = InputFactory.PathParameter("id", InputPrimitiveType.String, isRequired: true); + idParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("id", InputPrimitiveType.String, isRequired: true), + ]); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false), + ]); + var topParam = InputFactory.QueryParameter("top", InputPrimitiveType.Int32, isRequired: false, serializedName: "top"); + topParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("top", InputPrimitiveType.Int32, isRequired: false), + ]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "GetWidget", + InputFactory.Operation( + "GetWidget", + parameters: [idParam, filterParam, topParam], + responses: [InputFactory.OperationResponse([200])]), + parameters: [optionsMethodParameter]); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync(clients: () => [inputClient], inputModels: () => [optionsModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + Assert.IsNotNull(client); + + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + + var parameters = protocolMethod!.Signature.Parameters; + var actual = string.Join(", ", parameters.Select(p => $"{p.Type.Name} {p.Name}")); + + // Expected shape: (GetWidgetOptions options, RequestOptions ) + Assert.AreEqual(2, parameters.Count, $"protocol method should take the options bag, but was: ({actual})"); + Assert.AreEqual("GetWidgetOptions", parameters[0].Type.Name, $"actual: ({actual})"); + + // The trailing request options parameter must not collide with the options bag name. + Assert.AreNotEqual(parameters[0].Name, parameters[1].Name, $"actual: ({actual})"); + + // The protocol method expands the bag when calling CreateRequest, which still takes the + // individual wire parameters. + var protocolBody = protocolMethod.BodyStatements!.ToDisplayString(); + Assert.That(protocolBody, Does.Contain("options.Id")); + Assert.That(protocolBody, Does.Contain("options.Filter")); + Assert.That(protocolBody, Does.Contain("options.Top")); + + // The convenience method forwards the bag straight through rather than unpacking it. + var convenienceMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Convenience && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(convenienceMethod); + Assert.That( + convenienceMethod!.BodyStatements!.ToDisplayString(), + Does.Contain("this.GetWidget(options, cancellationToken.ToRequestOptions())")); + } + + [Test] + public async Task OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent() + { + // https://github.com/microsoft/typespec/issues/11214 + // The bag only groups non-body parameters, so the protocol method can adopt it while still + // taking the raw request content. + var bodyModel = InputFactory.Model("Widget", properties: [InputFactory.Property("data", InputPrimitiveType.String, isRequired: true)]); + var optionsModel = InputFactory.Model( + "CreateWidgetOptions", + properties: + [ + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + InputFactory.Property("top", InputPrimitiveType.Int32, isRequired: false, isHttpMetadata: true, wireName: "top"), + ]); + + var optionsMethodParameter = InputFactory.MethodParameter("options", optionsModel, isRequired: true, location: InputRequestLocation.Query); + + var bodyParam = InputFactory.BodyParameter("body", bodyModel, isRequired: true); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: [optionsMethodParameter, InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false)]); + var topParam = InputFactory.QueryParameter("top", InputPrimitiveType.Int32, isRequired: false, serializedName: "top"); + topParam.Update(methodParameterSegments: [optionsMethodParameter, InputFactory.MethodParameter("top", InputPrimitiveType.Int32, isRequired: false)]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "CreateWidget", + InputFactory.Operation("CreateWidget", parameters: [bodyParam, filterParam, topParam], responses: [InputFactory.OperationResponse([200])]), + parameters: + [ + InputFactory.MethodParameter("body", bodyModel, isRequired: true, location: InputRequestLocation.Body), + optionsMethodParameter, + ]); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync(clients: () => [inputClient], inputModels: () => [bodyModel, optionsModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + + var parameters = protocolMethod!.Signature.Parameters; + var actual = string.Join(", ", parameters.Select(p => $"{p.Type.Name} {p.Name}")); + + Assert.AreEqual(3, parameters.Count, $"actual: ({actual})"); + Assert.IsTrue(parameters.Any(p => p.IsContentParameter), $"raw body must be preserved, but was: ({actual})"); + Assert.IsTrue(parameters.Any(p => p.Type.Name == "CreateWidgetOptions"), $"actual: ({actual})"); + } + + [Test] + public async Task OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened() + { + // https://github.com/microsoft/typespec/issues/11214 + // When the request body itself was folded into the bag, grouping the protocol method would + // remove its only way to send a raw payload, so it stays flattened. + var requestModel = InputFactory.Model( + "RequestModel", + properties: + [ + InputFactory.Property("data", InputPrimitiveType.String, isRequired: true, isHttpMetadata: false), + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + ]); + + var requestMethodParameter = InputFactory.MethodParameter("request", requestModel, isRequired: true, location: InputRequestLocation.Body); + + var bodyParam = InputFactory.BodyParameter("body", requestModel, isRequired: true); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: [requestMethodParameter, InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false)]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "CreateWidget", + InputFactory.Operation("CreateWidget", parameters: [bodyParam, filterParam], responses: [InputFactory.OperationResponse([200])]), + parameters: [requestMethodParameter]); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync(clients: () => [inputClient], inputModels: () => [requestModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + + var parameters = protocolMethod!.Signature.Parameters; + var actual = string.Join(", ", parameters.Select(p => $"{p.Type.Name} {p.Name}")); + + Assert.IsTrue(parameters.Any(p => p.IsContentParameter), $"raw body must be preserved, but was: ({actual})"); + Assert.IsFalse(parameters.Any(p => p.Type.Name == "RequestModel"), $"protocol must stay flattened, but was: ({actual})"); } [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_EnumGroupedQueryParam_SerializesToProtocol.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_EnumGroupedQueryParam_SerializesToProtocol.cs index 72441024051..e536733cf7b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_EnumGroupedQueryParam_SerializesToProtocol.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_EnumGroupedQueryParam_SerializesToProtocol.cs @@ -1,4 +1,4 @@ global::Sample.Argument.AssertNotNullOrEmpty(collectionId, nameof(collectionId)); global::Sample.Argument.AssertNotNull(options, nameof(options)); -return this.GetPoint(collectionId, options.Resampling?.ToString(), cancellationToken.ToRequestOptions()); +return this.GetPoint(collectionId, options, cancellationToken.ToRequestOptions()); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientName.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientName.cs index d6af6d4230b..e536733cf7b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientName.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientName.cs @@ -1,4 +1,4 @@ global::Sample.Argument.AssertNotNullOrEmpty(collectionId, nameof(collectionId)); global::Sample.Argument.AssertNotNull(options, nameof(options)); -return this.GetPoint(collectionId, options.BandIndex, cancellationToken.ToRequestOptions()); +return this.GetPoint(collectionId, options, cancellationToken.ToRequestOptions()); From cbc3a3422cda732f59edf9dea7db4dca150af2be Mon Sep 17 00:00:00 2001 From: Michael Nash Date: Mon, 3 Aug 2026 14:57:03 -0700 Subject: [PATCH 2/5] Keep protocol flattened when the bag cannot enforce a required parameter TCGC does not validate that a required wire parameter maps to a required property on the options bag, so the generated constructor may not force callers to supply it. Grouping the protocol method in that case would silently drop the compile-time guarantee the flattened signature provides, so grouping is skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 69b166be-7096-4882-9251-6e998d9288ae --- .../src/Providers/RestClientProvider.cs | 54 ++++++++++++++++++ .../ScmMethodProviderCollectionTests.cs | 57 +++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs index 172c8876111..747e51bcc2f 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs @@ -1421,12 +1421,66 @@ internal static bool ShouldGroupProtocolParameters(InputServiceMethod serviceMet return false; } + if (!SegmentsPreserveRequiredness(parameter, segments)) + { + return false; + } + hasGroupedParameter = true; } return hasGroupedParameter; } + /// + /// A required wire parameter must map to a required property so the bag's constructor forces callers + /// to supply it. TCGC does not validate this, and when it does not hold, grouping the protocol method + /// would silently drop the compile-time guarantee that the flattened signature provides. + /// + private static bool SegmentsPreserveRequiredness(InputParameter parameter, IReadOnlyList segments) + { + if (!parameter.IsRequired) + { + return true; + } + + var currentType = segments[0].Type; + for (int i = 1; i < segments.Count; i++) + { + if (currentType is not InputModelType model) + { + return false; + } + + var property = FindPropertyInHierarchy(model, segments[i].Name); + if (property is null || !property.IsRequired) + { + return false; + } + + currentType = property.Type; + } + + return true; + } + + private static InputModelProperty? FindPropertyInHierarchy(InputModelType model, string name) + { + for (var current = model; current != null; current = current.BaseModel) + { + foreach (var property in current.Properties) + { + if (property.SerializedName == name + || string.Equals(property.Name, name, StringComparison.OrdinalIgnoreCase)) + { + return property; + } + } + } + + return null; + } + private static bool HasLiteralContentTypeHeader(InputOperation operation) { foreach (var p in operation.Parameters) 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 fa5ddb8b1ae..f4cf78cd44b 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 @@ -1894,6 +1894,63 @@ public async Task OptionsBagOverride_AppliesToProtocolMethod() Assert.That( convenienceMethod!.BodyStatements!.ToDisplayString(), Does.Contain("this.GetWidget(options, cancellationToken.ToRequestOptions())")); + + // The bag now carries parameters that used to be required method parameters, so its public + // constructor must still force callers to supply the required ones. + var optionsProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(optionsModel); + Assert.IsNotNull(optionsProvider); + var publicCtor = optionsProvider!.Constructors.FirstOrDefault( + c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsNotNull(publicCtor, "options bag should expose a public constructor"); + var ctorParams = publicCtor!.Signature.Parameters.Select(p => p.Name).ToList(); + Assert.That(ctorParams, Does.Contain("id"), $"required property must be a required ctor arg, but ctor was: ({string.Join(", ", ctorParams)})"); + Assert.That(ctorParams, Does.Not.Contain("filter"), $"optional property must not be a ctor arg, but ctor was: ({string.Join(", ", ctorParams)})"); + Assert.That(ctorParams, Does.Not.Contain("top"), $"optional property must not be a ctor arg, but ctor was: ({string.Join(", ", ctorParams)})"); + } + + [Test] + public async Task OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened() + { + // https://github.com/microsoft/typespec/issues/11214 + // TCGC does not validate that a required wire parameter maps to a required bag property, so + // the bag's constructor would not force callers to supply it. Grouping the protocol method + // would drop the compile-time guarantee the flattened signature provides, so it stays flat. + var optionsModel = InputFactory.Model( + "GetWidgetOptions", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "id"), + ]); + + var optionsMethodParameter = InputFactory.MethodParameter("options", optionsModel, isRequired: true, location: InputRequestLocation.Query); + + var idParam = InputFactory.PathParameter("id", InputPrimitiveType.String, isRequired: true); + idParam.Update(methodParameterSegments: [optionsMethodParameter, InputFactory.MethodParameter("id", InputPrimitiveType.String, isRequired: false)]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "GetWidget", + InputFactory.Operation("GetWidget", parameters: [idParam], responses: [InputFactory.OperationResponse([200])]), + parameters: [optionsMethodParameter]); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync(clients: () => [inputClient], inputModels: () => [optionsModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + + var parameters = protocolMethod!.Signature.Parameters; + var actual = string.Join(", ", parameters.Select(p => $"{p.Type.Name} {p.Name}")); + + // The bag's constructor cannot force `id`, so the protocol method must keep requiring it directly. + var publicCtor = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(optionsModel)!.Constructors + .FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.That(publicCtor!.Signature.Parameters.Select(p => p.Name), Does.Not.Contain("id")); + + Assert.IsFalse(parameters.Any(p => p.Type.Name == "GetWidgetOptions"), $"protocol must stay flattened, but was: ({actual})"); + Assert.IsTrue(parameters.Any(p => p.Name == "id"), $"required parameter must stay on the signature, but was: ({actual})"); } [Test] From a425e3edf752e9bdfbd20f9d7d3dc510497923ff Mon Sep 17 00:00:00 2001 From: Michael Nash Date: Mon, 3 Aug 2026 15:54:10 -0700 Subject: [PATCH 3/5] Forward the request options parameter to CreateRequest by type An options bag can take the `options` name used by the request options parameter, which causes that parameter to be renamed. The CreateRequest call still bound its trailing argument by the original name, so it passed the bag instead and the generated client did not compile. Match ungrouped CreateRequest arguments on name and type, falling back to a unique type match when the bag has taken the name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 69b166be-7096-4882-9251-6e998d9288ae --- .../Providers/ScmMethodProviderCollection.cs | 41 ++++++++++++++++++- .../ScmMethodProviderCollectionTests.cs | 7 ++++ 2 files changed, 46 insertions(+), 2 deletions(-) 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 c2767092b22..92ecbec1a7a 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 @@ -1300,8 +1300,7 @@ private ValueExpression[] BuildCreateRequestArguments( if (segments is not { Count: > 1 }) { - var match = protocolParameters.FirstOrDefault( - p => string.Equals(p.Name, createRequestParameter.Name, StringComparison.OrdinalIgnoreCase)); + var match = FindUngroupedArgument(protocolParameters, createRequestParameter); if (match is null) { return null; @@ -1339,6 +1338,44 @@ private ValueExpression[] BuildCreateRequestArguments( return arguments; } + /// + /// Locates the protocol parameter to forward for a CreateRequest parameter that was not + /// folded into the options bag. The name alone is not reliable: a bag named after the request + /// options parameter causes that parameter to be renamed, so the original name now resolves to + /// the bag. The type disambiguates in that case. + /// + private static ParameterProvider? FindUngroupedArgument( + IReadOnlyList protocolParameters, + ParameterProvider createRequestParameter) + { + foreach (var parameter in protocolParameters) + { + if (string.Equals(parameter.Name, createRequestParameter.Name, StringComparison.OrdinalIgnoreCase) + && parameter.Type.Equals(createRequestParameter.Type)) + { + return parameter; + } + } + + ParameterProvider? typeMatch = null; + foreach (var parameter in protocolParameters) + { + if (!parameter.Type.Equals(createRequestParameter.Type)) + { + continue; + } + + if (typeMatch is not null) + { + return null; + } + + typeMatch = parameter; + } + + return typeMatch; + } + private ParameterProvider ProcessOptionalParameters( List optionalParameters, List requiredParameters, 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 f4cf78cd44b..15225efb95a 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 @@ -1887,6 +1887,13 @@ public async Task OptionsBagOverride_AppliesToProtocolMethod() Assert.That(protocolBody, Does.Contain("options.Filter")); Assert.That(protocolBody, Does.Contain("options.Top")); + // The renamed request options parameter must be the one forwarded to CreateRequest. Matching + // CreateRequest's trailing parameter by name alone resolves to the options bag, which does not compile. + Assert.That( + protocolBody, + Does.Contain($"options.Top, {parameters[1].Name})"), + $"CreateRequest must receive '{parameters[1].Name}', but body was: {protocolBody}"); + // The convenience method forwards the bag straight through rather than unpacking it. var convenienceMethod = methodCollection.FirstOrDefault( m => m.Kind == ScmMethodKind.Convenience && !m.Signature.Name.EndsWith("Async")); From 3a07763a04025c41240e70adc3381977d585345b Mon Sep 17 00:00:00 2001 From: jolov Date: Tue, 4 Aug 2026 20:02:11 -0700 Subject: [PATCH 4/5] test(http-client-csharp): add override method baselines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 12a20512-4c49-4d6b-83d9-4e6f6261c5bf --- .../ScmMethodProviderCollectionTests.cs | 26 +++++++++---------- ...de_AppliesToProtocolMethod(Convenience).cs | 3 +++ ...rride_AppliesToProtocolMethod(Protocol).cs | 4 +++ ...de_BodyInsideBag_ProtocolStaysFlattened.cs | 4 +++ ...yOutsideBag_ProtocolKeepsRequestContent.cs | 5 ++++ ...ramOptionalInBag_ProtocolStaysFlattened.cs | 4 +++ 6 files changed, 33 insertions(+), 13 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Convenience).cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Protocol).cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened.cs 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 15225efb95a..022211da0c6 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 @@ -1883,24 +1883,15 @@ public async Task OptionsBagOverride_AppliesToProtocolMethod() // The protocol method expands the bag when calling CreateRequest, which still takes the // individual wire parameters. var protocolBody = protocolMethod.BodyStatements!.ToDisplayString(); - Assert.That(protocolBody, Does.Contain("options.Id")); - Assert.That(protocolBody, Does.Contain("options.Filter")); - Assert.That(protocolBody, Does.Contain("options.Top")); - - // The renamed request options parameter must be the one forwarded to CreateRequest. Matching - // CreateRequest's trailing parameter by name alone resolves to the options bag, which does not compile. - Assert.That( - protocolBody, - Does.Contain($"options.Top, {parameters[1].Name})"), - $"CreateRequest must receive '{parameters[1].Name}', but body was: {protocolBody}"); + Assert.AreEqual(Helpers.GetExpectedFromFile("Protocol"), protocolBody); // The convenience method forwards the bag straight through rather than unpacking it. var convenienceMethod = methodCollection.FirstOrDefault( m => m.Kind == ScmMethodKind.Convenience && !m.Signature.Name.EndsWith("Async")); Assert.IsNotNull(convenienceMethod); - Assert.That( - convenienceMethod!.BodyStatements!.ToDisplayString(), - Does.Contain("this.GetWidget(options, cancellationToken.ToRequestOptions())")); + Assert.AreEqual( + Helpers.GetExpectedFromFile("Convenience"), + convenienceMethod!.BodyStatements!.ToDisplayString()); // The bag now carries parameters that used to be required method parameters, so its public // constructor must still force callers to supply the required ones. @@ -1958,6 +1949,9 @@ public async Task OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFla Assert.IsFalse(parameters.Any(p => p.Type.Name == "GetWidgetOptions"), $"protocol must stay flattened, but was: ({actual})"); Assert.IsTrue(parameters.Any(p => p.Name == "id"), $"required parameter must stay on the signature, but was: ({actual})"); + Assert.AreEqual( + Helpers.GetExpectedFromFile(), + protocolMethod.BodyStatements!.ToDisplayString()); } [Test] @@ -2007,6 +2001,9 @@ public async Task OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent( Assert.AreEqual(3, parameters.Count, $"actual: ({actual})"); Assert.IsTrue(parameters.Any(p => p.IsContentParameter), $"raw body must be preserved, but was: ({actual})"); Assert.IsTrue(parameters.Any(p => p.Type.Name == "CreateWidgetOptions"), $"actual: ({actual})"); + Assert.AreEqual( + Helpers.GetExpectedFromFile(), + protocolMethod.BodyStatements!.ToDisplayString()); } [Test] @@ -2048,6 +2045,9 @@ public async Task OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened() Assert.IsTrue(parameters.Any(p => p.IsContentParameter), $"raw body must be preserved, but was: ({actual})"); Assert.IsFalse(parameters.Any(p => p.Type.Name == "RequestModel"), $"protocol must stay flattened, but was: ({actual})"); + Assert.AreEqual( + Helpers.GetExpectedFromFile(), + protocolMethod.BodyStatements!.ToDisplayString()); } [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Convenience).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Convenience).cs new file mode 100644 index 00000000000..d55cae5b177 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Convenience).cs @@ -0,0 +1,3 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); + +return this.GetWidget(options, cancellationToken.ToRequestOptions()); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Protocol).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Protocol).cs new file mode 100644 index 00000000000..b3c32f359c2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Protocol).cs @@ -0,0 +1,4 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetWidgetRequest(options.Id, options.Filter, options.Top, requestOptions); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, requestOptions)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened.cs new file mode 100644 index 00000000000..1d8e176a583 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened.cs @@ -0,0 +1,4 @@ +global::Sample.Argument.AssertNotNull(content, nameof(content)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateCreateWidgetRequest(content, filter, 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/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent.cs new file mode 100644 index 00000000000..972a20ff897 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent.cs @@ -0,0 +1,5 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); +global::Sample.Argument.AssertNotNull(content, nameof(content)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateCreateWidgetRequest(content, options.Filter, options.Top, requestOptions); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, requestOptions)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened.cs new file mode 100644 index 00000000000..1c76f895f6c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened.cs @@ -0,0 +1,4 @@ +global::Sample.Argument.AssertNotNullOrEmpty(id, nameof(id)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetWidgetRequest(id, options); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); From 13bc727906f91adaabb43ed616dca97ff5be9915 Mon Sep 17 00:00:00 2001 From: jolov Date: Tue, 4 Aug 2026 21:02:07 -0700 Subject: [PATCH 5/5] fix(http-client-csharp): expand grouped paging arguments Preserve generator parameter metadata when protocol method customizations rename grouped options parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 12a20512-4c49-4d6b-83d9-4e6f6261c5bf --- .../Providers/ScmMethodProviderCollection.cs | 56 ++++++-- .../ScmMethodProviderCollectionTests.cs | 126 ++++++++++++++++++ ...colParameterNamesPreserveGroupedMapping.cs | 4 + .../TestClient.cs | 14 ++ ...andsBagForCollectionResult(Convenience).cs | 3 + ...ExpandsBagForCollectionResult(Protocol).cs | 3 + 6 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping/TestClient.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Convenience).cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Protocol).cs 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 92ecbec1a7a..2ccade53ab7 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 @@ -207,7 +207,16 @@ private ScmMethodProvider BuildConvenienceMethod(MethodProvider protocolMethod, if (_pagingServiceMethod != null) { collection = ScmCodeModelGenerator.Instance.TypeFactory.ClientResponseApi.CreateClientCollectionResultDefinition(Client, _pagingServiceMethod, responseBodyType, isAsync); - methodBody = [.. GetPagingMethodBody(collection, convenienceBodyParameters, true)]; + var createRequestSignature = Client.RestClient.GetCreateRequestMethod(ServiceMethod.Operation).Signature; + methodBody = + [ + .. GetPagingMethodBody( + collection, + createRequestSignature, + convenienceBodyParameters, + true, + [.. protocolMethod.Signature.Parameters]) + ]; } else if (responseBodyType is null) { @@ -1136,7 +1145,7 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod { // Partial methods cannot have optional parameters in the implementation. var requiredCustomParameters = PartialMethodCustomization.RenameAndCloneParameters( - customSignature.Parameters, + parameters, customSignature.Parameters, removeDefaults: true).ToArray(); @@ -1169,7 +1178,7 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod if (_pagingServiceMethod != null) { collection = ScmCodeModelGenerator.Instance.TypeFactory.ClientResponseApi.CreateClientCollectionResultDefinition(Client, _pagingServiceMethod, null, isAsync); - methodBody = [.. GetPagingMethodBody(collection, bodyParameters, false)]; + methodBody = [.. GetPagingMethodBody(collection, createRequestMethod.Signature, bodyParameters, false)]; } else { @@ -1289,8 +1298,10 @@ private ValueExpression[] BuildCreateRequestArguments( private static ValueExpression[]? BuildGroupedCreateRequestArguments( IReadOnlyList createRequestParameters, - IReadOnlyList protocolParameters) + IReadOnlyList protocolParameters, + Func? getArgument = null) { + getArgument ??= parameter => parameter; var arguments = new ValueExpression[createRequestParameters.Count]; for (int i = 0; i < createRequestParameters.Count; i++) @@ -1306,12 +1317,13 @@ private ValueExpression[] BuildCreateRequestArguments( return null; } - arguments[i] = match; + arguments[i] = getArgument(match); continue; } var groupParameter = protocolParameters.FirstOrDefault( - p => string.Equals(p.Name, segments[0].Name, StringComparison.OrdinalIgnoreCase)); + p => string.Equals(p.InputParameter?.Name, segments[0].Name, StringComparison.OrdinalIgnoreCase) + || string.Equals(p.Name, segments[0].Name, StringComparison.OrdinalIgnoreCase)); if (groupParameter is null || !ScmCodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(groupParameter.Type, out var typeProvider) || typeProvider is not ModelProvider groupModel) @@ -1320,7 +1332,7 @@ private ValueExpression[] BuildCreateRequestArguments( } var propertySegments = segments.Skip(1).Select(s => s.Name).ToList(); - var propertyExpression = groupModel.GetPropertyExpression(groupParameter, propertySegments, out var leafProperty); + var propertyExpression = groupModel.GetPropertyExpression(getArgument(groupParameter), propertySegments, out var leafProperty); // CreateRequest takes the serialized (string/number) form of an enum, so convert before forwarding. if (leafProperty.Type.IsEnum && !createRequestParameter.Type.IsEnum) @@ -1443,19 +1455,41 @@ private ParameterProvider ProcessOptionalParameters( private IEnumerable GetPagingMethodBody( TypeProvider collection, + MethodSignature createRequestSignature, IReadOnlyList parameters, - bool isConvenience) + bool isConvenience, + IReadOnlyList? pagingProtocolParameters = null) { if (isConvenience) { + var conversionStatements = GetStackVariablesForProtocolParamConversion(ConvenienceMethodParameters, out var declarations); + var protocolArguments = GetProtocolMethodArguments(declarations); + + IReadOnlyList constructorArguments = protocolArguments; + if (RestClientProvider.ShouldGroupProtocolParameters(ServiceMethod)) + { + var protocolParameters = pagingProtocolParameters + ?? throw new InvalidOperationException("Paging protocol parameters are required to expand grouped arguments."); + var argumentMap = new Dictionary(); + for (int i = 0; i < protocolParameters.Count; i++) + { + argumentMap[protocolParameters[i]] = protocolArguments[i]; + } + + constructorArguments = BuildGroupedCreateRequestArguments( + createRequestSignature.Parameters, + protocolParameters, + parameter => argumentMap[parameter]) ?? protocolArguments; + } + return [ - .. GetStackVariablesForProtocolParamConversion(ConvenienceMethodParameters, out var declarations), + .. conversionStatements, Return(New.Instance( collection.Type, [ This, - .. GetProtocolMethodArguments(declarations) + .. constructorArguments ])) ]; } @@ -1464,7 +1498,7 @@ .. GetProtocolMethodArguments(declarations) collection.Type, [ This, - .. parameters + .. BuildCreateRequestArguments(createRequestSignature, parameters) ])); } 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 022211da0c6..4ef1a0610f3 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 @@ -1906,6 +1906,132 @@ public async Task OptionsBagOverride_AppliesToProtocolMethod() Assert.That(ctorParams, Does.Not.Contain("top"), $"optional property must not be a ctor arg, but ctor was: ({string.Join(", ", ctorParams)})"); } + [Test] + public async Task OptionsBagOverride_PagingMethodExpandsBagForCollectionResult() + { + var optionsModel = InputFactory.Model( + "GetWidgetsOptions", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: true, isHttpMetadata: true, wireName: "id"), + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + ]); + var itemModel = InputFactory.Model( + "Widget", + properties: [InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)]); + var optionsMethodParameter = InputFactory.MethodParameter( + "options", + optionsModel, + isRequired: true, + location: InputRequestLocation.Query); + + var idParam = InputFactory.PathParameter("id", InputPrimitiveType.String, isRequired: true); + idParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("id", InputPrimitiveType.String, isRequired: true), + ]); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false), + ]); + + var response = InputFactory.OperationResponse( + [200], + InputFactory.Model( + "Page", + properties: [InputFactory.Property("items", InputFactory.Array(itemModel))])); + var serviceMethod = InputFactory.PagingServiceMethod( + "GetWidgets", + InputFactory.Operation("GetWidgets", parameters: [idParam, filterParam], responses: [response]), + parameters: [optionsMethodParameter], + pagingMetadata: InputFactory.PagingMetadata(["items"], null, null)); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync( + clients: () => [inputClient], + inputModels: () => [optionsModel, itemModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + Assert.IsNotNull(client); + + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + var convenienceMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Convenience && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + Assert.IsNotNull(convenienceMethod); + Assert.IsTrue(protocolMethod!.Signature.Parameters.Any(p => p.Type.Name == "GetWidgetsOptions")); + + Assert.AreEqual( + Helpers.GetExpectedFromFile("Protocol"), + protocolMethod.BodyStatements!.ToDisplayString()); + Assert.AreEqual( + Helpers.GetExpectedFromFile("Convenience"), + convenienceMethod!.BodyStatements!.ToDisplayString()); + } + + [Test] + public async Task OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping() + { + var optionsModel = InputFactory.Model( + "GetWidgetOptions", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: true, isHttpMetadata: true, wireName: "id"), + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + ]); + var optionsMethodParameter = InputFactory.MethodParameter( + "options", + optionsModel, + isRequired: true, + location: InputRequestLocation.Query); + + var idParam = InputFactory.PathParameter("id", InputPrimitiveType.String, isRequired: true); + idParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("id", InputPrimitiveType.String, isRequired: true), + ]); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false), + ]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "GetWidget", + InputFactory.Operation( + "GetWidget", + parameters: [idParam, filterParam], + responses: [InputFactory.OperationResponse([200])]), + parameters: [optionsMethodParameter]); + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync( + clients: () => [inputClient], + inputModels: () => [optionsModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + Assert.IsNotNull(client); + + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol + && m.IsPartialMethod + && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + Assert.AreEqual("renamedOptions", protocolMethod!.Signature.Parameters[0].Name); + Assert.AreEqual("renamedRequestOptions", protocolMethod.Signature.Parameters[1].Name); + Assert.AreEqual( + Helpers.GetExpectedFromFile(), + protocolMethod.BodyStatements!.ToDisplayString()); + } + [Test] public async Task OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping.cs new file mode 100644 index 00000000000..26a6d3b0b7c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping.cs @@ -0,0 +1,4 @@ +global::Sample.Argument.AssertNotNull(renamedOptions, nameof(renamedOptions)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetWidgetRequest(renamedOptions.Id, renamedOptions.Filter, renamedRequestOptions); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, renamedRequestOptions)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping/TestClient.cs new file mode 100644 index 00000000000..583443b391c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping/TestClient.cs @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.ClientModel; +using System.ClientModel.Primitives; +using Sample.Models; + +namespace Sample +{ + public partial class TestClient + { + public partial ClientResult GetWidget(GetWidgetOptions renamedOptions, RequestOptions renamedRequestOptions); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Convenience).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Convenience).cs new file mode 100644 index 00000000000..ae6b47c9f0a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Convenience).cs @@ -0,0 +1,3 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); + +return new global::Sample.TestClientGetWidgetsCollectionResultOfT(this, options.Id, options.Filter, cancellationToken.ToRequestOptions()); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Protocol).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Protocol).cs new file mode 100644 index 00000000000..632c2a8b25f --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Protocol).cs @@ -0,0 +1,3 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); + +return new global::Sample.TestClientGetWidgetsCollectionResult(this, options.Id, options.Filter, requestOptions);