Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1221,8 +1221,13 @@ internal static List<ParameterProvider> 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))
Comment thread
JoshLove-msft marked this conversation as resolved.
? serviceMethod.Parameters
: operation.Parameters;

var pageSizeParameterName = GetPageSizeParameterName(serviceMethod as InputPagingServiceMethod);

Expand Down Expand Up @@ -1307,7 +1312,7 @@ internal static List<ParameterProvider> 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)
{
Expand Down Expand Up @@ -1393,6 +1398,89 @@ internal static List<ParameterProvider> GetMethodParameters(
return [.. sortedParams.Values];
}

/// <summary>
/// Determines whether the protocol method should adopt the grouped (options bag) parameter shape
/// produced by <c>@@override</c>. 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.
/// </summary>
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;
}

if (!SegmentsPreserveRequiredness(parameter, segments))
{
return false;
}

hasGroupedParameter = true;
}

return hasGroupedParameter;
}

/// <summary>
/// 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.
/// </summary>
private static bool SegmentsPreserveRequiredness(InputParameter parameter, IReadOnlyList<InputMethodParameter> 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -1196,6 +1204,38 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod
return protocolMethod;
}

private static ParameterProvider ResolveRequestOptionsNameCollision(
Comment thread
JoshLove-msft marked this conversation as resolved.
ParameterProvider requestOptionsParameter,
IReadOnlyList<ParameterProvider> requiredParameters,
IReadOnlyList<ParameterProvider> 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
Expand All @@ -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<ParameterProvider> 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];
Expand All @@ -1234,6 +1287,58 @@ private static ValueExpression[] BuildCreateRequestArguments(
return [.. bodyParameters.Select(p => (ValueExpression)p)];
}

private static ValueExpression[]? BuildGroupedCreateRequestArguments(
IReadOnlyList<ParameterProvider> createRequestParameters,
IReadOnlyList<ParameterProvider> 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;
}

Comment thread
JoshLove-msft marked this conversation as resolved.
arguments[i] = match;
continue;
}
Comment thread
JoshLove-msft marked this conversation as resolved.

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<ParameterProvider> optionalParameters,
List<ParameterProvider> requiredParameters,
Expand Down
Loading
Loading