Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
a8eeca4
remove cancellationtoken on nonazure convenience methods
ArcturusZhang Apr 3, 2024
1eaea61
update code to remove some unused methods
ArcturusZhang Apr 3, 2024
fc4d4b8
update the document on protocol methods
ArcturusZhang Apr 3, 2024
fdce61a
fix regenerate issue
ArcturusZhang Apr 3, 2024
a633b43
Merge branch 'feature/v3' into enable-new-cancellationtoken-pattern
ArcturusZhang Apr 3, 2024
10187fe
Merge branch 'feature/v3' into enable-new-cancellationtoken-pattern
ArcturusZhang Apr 7, 2024
3048edd
fix the link and regen
ArcturusZhang Apr 7, 2024
3f4cea7
Merge branch 'feature/v3' into enable-new-cancellationtoken-pattern
ArcturusZhang Apr 9, 2024
8747988
regen
ArcturusZhang Apr 9, 2024
d1b8e19
Merge branch 'feature/v3' into enable-new-cancellationtoken-pattern
ArcturusZhang Apr 9, 2024
c17da1a
Merge remote-tracking branch 'origin/feature/v3' into enable-new-canc…
ArcturusZhang Apr 10, 2024
9a272d6
Merge remote-tracking branch 'origin/feature/v3' into enable-new-canc…
ArcturusZhang Apr 11, 2024
0436099
make the code change
ArcturusZhang Apr 12, 2024
5776e75
regen
ArcturusZhang Apr 12, 2024
52b2ddd
fix the requiredness of requestcontext parameter
ArcturusZhang Apr 12, 2024
ee69dd1
regen, but it has some issues, need to investigate why it becomes thi…
ArcturusZhang Apr 12, 2024
7a6253e
fix the issue
ArcturusZhang Apr 12, 2024
18e048e
add a comment to explain the changes
ArcturusZhang Apr 12, 2024
5db5c6e
Merge remote-tracking branch 'origin/feature/v3' into enable-new-canc…
ArcturusZhang Apr 12, 2024
08afee6
Merge remote-tracking branch 'origin/feature/v3' into enable-new-canc…
ArcturusZhang Apr 12, 2024
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 @@ -219,7 +219,7 @@ private static bool ContainsRequestContext(IReadOnlyCollection<Parameter>? param
continue;
}

var convenienceParameter = ProtocolToConvenienceParameterConverters.FirstOrDefault(convert => convert.Convenience.Name == parameter.Name)?.Convenience;
var convenienceParameter = ProtocolToConvenienceParameterConverters.FirstOrDefault(convert => convert.Convenience?.Name == parameter.Name)?.Convenience;
if (convenienceParameter == null)
{
throw new InvalidOperationException($"Cannot find corresponding convenience parameter for create request method parameter {parameter.Name}.");
Expand Down
14 changes: 8 additions & 6 deletions src/AutoRest.CSharp/Common/Output/Expressions/Snippets.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ internal static partial class Snippets
public static ValueExpression Dash { get; } = new KeywordExpression("_", null);
public static ValueExpression Default { get; } = new KeywordExpression("default", null);
public static ValueExpression Null { get; } = new KeywordExpression("null", null);

public static ValueExpression DefaultOf(CSharpType type) => type is { IsValueType: true, IsNullable: false } ? Default.CastTo(type) : Null.CastTo(type);
public static ValueExpression This { get; } = new KeywordExpression("this", null);
public static BoolExpression True { get; } = new(new KeywordExpression("true", null));
public static BoolExpression False { get; } = new(new KeywordExpression("false", null));
Expand All @@ -35,7 +37,7 @@ internal static partial class Snippets
public static ValueExpression Float(float value) => new FormattableStringToExpression($"{value}f");
public static ValueExpression Double(double value) => new FormattableStringToExpression($"{value}d");

public static ValueExpression Nameof(ValueExpression expression) => new InvokeInstanceMethodExpression(null, "nameof", new[]{expression}, null, false);
public static ValueExpression Nameof(ValueExpression expression) => new InvokeInstanceMethodExpression(null, "nameof", new[] { expression }, null, false);
public static ValueExpression ThrowExpression(ValueExpression expression) => new KeywordExpression("throw", expression);

public static ValueExpression NullCoalescing(ValueExpression left, ValueExpression right) => new BinaryOperatorExpression("??", left, right);
Expand All @@ -46,8 +48,8 @@ public static ValueExpression RemoveAllNullConditional(ValueExpression expressio
=> expression switch
{
NullConditionalExpression nullConditional => RemoveAllNullConditional(nullConditional.Inner),
MemberExpression { Inner: {} inner } member => member with {Inner = RemoveAllNullConditional(inner)},
TypedValueExpression typed => typed with { Untyped = RemoveAllNullConditional(typed.Untyped)},
MemberExpression { Inner: { } inner } member => member with { Inner = RemoveAllNullConditional(inner) },
TypedValueExpression typed => typed with { Untyped = RemoveAllNullConditional(typed.Untyped) },
_ => expression
};

Expand Down Expand Up @@ -86,9 +88,9 @@ public static EnumerableExpression InvokeArrayEmpty(CSharpType arrayItemType)
=> new(arrayItemType, new InvokeStaticMethodExpression(typeof(Array), nameof(Array.Empty), Array.Empty<ValueExpression>(), new[] { arrayItemType }));

public static StreamExpression InvokeFileOpenRead(string filePath)
=> new(new InvokeStaticMethodExpression(typeof(System.IO.File), nameof(System.IO.File.OpenRead), new[]{Literal(filePath)}));
=> new(new InvokeStaticMethodExpression(typeof(System.IO.File), nameof(System.IO.File.OpenRead), new[] { Literal(filePath) }));
public static StreamExpression InvokeFileOpenWrite(string filePath)
=> new(new InvokeStaticMethodExpression(typeof(System.IO.File), nameof(System.IO.File.OpenWrite), new[]{Literal(filePath)}));
=> new(new InvokeStaticMethodExpression(typeof(System.IO.File), nameof(System.IO.File.OpenWrite), new[] { Literal(filePath) }));

// Expected signature: MethodName(Utf8JsonWriter writer);
public static MethodBodyStatement InvokeCustomSerializationMethod(string methodName, Utf8JsonWriterExpression utf8JsonWriter)
Expand All @@ -100,7 +102,7 @@ public static MethodBodyStatement InvokeCustomBicepSerializationMethod(string me

// Expected signature: MethodName(JsonProperty property, ref T optional)
public static MethodBodyStatement InvokeCustomDeserializationMethod(string methodName, JsonPropertyExpression jsonProperty, CodeWriterDeclaration variable)
=> new InvokeStaticMethodStatement(null, methodName, new ValueExpression[]{jsonProperty, new FormattableStringToExpression($"ref {variable}")});
=> new InvokeStaticMethodStatement(null, methodName, new ValueExpression[] { jsonProperty, new FormattableStringToExpression($"ref {variable}") });

public static AssignValueIfNullStatement AssignIfNull(ValueExpression variable, ValueExpression expression) => new(variable, expression);
public static AssignValueStatement Assign(ValueExpression variable, ValueExpression expression) => new(variable, expression);
Expand Down
3 changes: 2 additions & 1 deletion src/AutoRest.CSharp/LowLevel/Generation/DpgClientWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ public void WriteClient()
WriteRequestCreationMethod(_writer, method, _client.Fields);
}

if (_client.ClientMethods.Any(cm => cm.ConvenienceMethod is not null))
// since the non-azure libraries do not have cancellationToken parameters on their convenience methods, we do not need to add the method to convert the cancellationToken to RequestContext
if (Configuration.IsBranded && _client.ClientMethods.Any(cm => cm.ConvenienceMethod is not null))
{
WriteCancellationTokenToRequestContextMethod();
}
Expand Down
30 changes: 17 additions & 13 deletions src/AutoRest.CSharp/LowLevel/Output/ConvenienceMethod.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace AutoRest.CSharp.Output.Models
{
internal record ConvenienceMethod(MethodSignature Signature, IReadOnlyList<ProtocolToConvenienceParameterConverter> ProtocolToConvenienceParameterConverters, CSharpType? ResponseType, IReadOnlyList<string>? RequestMediaTypes, IReadOnlyList<string>? ResponseMediaTypes, Diagnostic? Diagnostic, bool IsPageable, bool IsLongRunning, string? Deprecated)
{
private record ConvenienceParameterInfo(Parameter Convenience, ConvenienceParameterSpread? ConvenienceSpread);
private record ConvenienceParameterInfo(Parameter? Convenience, ConvenienceParameterSpread? ConvenienceSpread);

public IEnumerable<MethodBodyStatement> GetConvertStatements(LowLevelClientMethod clientMethod, bool async, FieldDeclaration clientDiagnostics)
{
Expand All @@ -33,7 +33,7 @@ public IEnumerable<MethodBodyStatement> GetConvertStatements(LowLevelClientMetho
var protocolInvocationExpressions = new List<ValueExpression>();
foreach (var protocol in protocolMethod.Parameters)
{
var (convenience, convenienceSpread) = parametersDict[protocol.Name]; // TODO -- maybe change it to name as keys?
var (convenience, convenienceSpread) = parametersDict[protocol.Name];
if (protocol == KnownParameters.RequestContext || protocol == KnownParameters.RequestContextRequired)
{
// convert cancellationToken to request context
Expand All @@ -46,18 +46,15 @@ public IEnumerable<MethodBodyStatement> GetConvertStatements(LowLevelClientMetho
yield return Declare(context, new InvokeStaticMethodExpression(null, "FromCancellationToken", new ValueExpression[] { convenience }));
protocolInvocationExpressions.Add(context);
}
else if (!protocol.IsOptionalInSignature)
{
// when we do not have a cancellationToken, and the requestContext parameter is required in protocol signature, we pass a null
protocolInvocationExpressions.Add(Null.CastTo(protocol.Type));
}
else
{
// do nothing
// if convenience parameter is null here, we just use the default value of the protocol parameter as value (we always do this even if the parameter is optional just in case there is an ordering issue)
protocolInvocationExpressions.Add(DefaultOf(protocol.Type));
}
}
else if (protocol == KnownParameters.RequestContent || protocol == KnownParameters.RequestContentNullable)
{
Debug.Assert(convenience is not null);
// convert body to request content
if (convenienceSpread == null)
{
Expand All @@ -75,10 +72,17 @@ public IEnumerable<MethodBodyStatement> GetConvertStatements(LowLevelClientMetho
else
{
// process any other parameter
// in this case, convenience parameter should never be null
Debug.Assert(convenience is not null);
var expression = convenience.GetConversionToProtocol(protocol.Type, RequestMediaTypes?.FirstOrDefault());
protocolInvocationExpressions.Add(expression);
if (convenience != null)
{
// if convenience parameter is not null here, we convert it to protocol parameter value properly
var expression = convenience.GetConversionToProtocol(protocol.Type, RequestMediaTypes?.FirstOrDefault());
protocolInvocationExpressions.Add(expression);
}
else
{
// if convenience parameter is null here, we just use the default value of the protocol parameter as value (we always do this even if the parameter is optional just in case there is an ordering issue)
protocolInvocationExpressions.Add(DefaultOf(protocol.Type));
}
}
}

Expand Down Expand Up @@ -210,7 +214,7 @@ private MethodBodyStatement GetSpreadConversion(ConvenienceParameterSpread conve
}
}

internal record ProtocolToConvenienceParameterConverter(Parameter Protocol, Parameter Convenience, ConvenienceParameterSpread? ConvenienceSpread);
internal record ProtocolToConvenienceParameterConverter(Parameter Protocol, Parameter? Convenience, ConvenienceParameterSpread? ConvenienceSpread);

internal record ConvenienceParameterSpread(ModelTypeProvider BackingModel, IReadOnlyList<Parameter> SpreadedParameters);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ private ConvenienceMethodOmittingMessage(string message)

public string Message { get; }

public static ConvenienceMethodOmittingMessage AnonymousModel = new("The convenience method of this operation is omitted because it is using at least one anonymous model");

public static ConvenienceMethodOmittingMessage NotConfident = new("The convenience method of this operation is made internal because this operation directly or indirectly uses a low confident type, for instance, unions, literal types with number values, etc.");

public static ConvenienceMethodOmittingMessage NotMeaningful = new("The convenience method is omitted here because it has exactly the same parameter list as the corresponding protocol method");
}
}
18 changes: 13 additions & 5 deletions src/AutoRest.CSharp/LowLevel/Output/OperationMethodChainBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,15 @@ public LowLevelClientMethod BuildOperationMethodChain()
? new[] { new CSharpAttribute(typeof(ObsoleteAttribute), Literal(deprecated)) }
: Array.Empty<CSharpAttribute>();

var shouldRequestContextOptional = ShouldRequestContextOptional();
var protocolMethodParameters = _orderedParameters.Select(p => p.Protocol).WhereNotNull().Select(p => p != KnownParameters.RequestContentNullable && !shouldRequestContextOptional ? p.ToRequired() : p).ToArray();
var isRequestContextOptional = _orderedParameters.Any(p => p.Protocol == KnownParameters.RequestContext);
// because request context is always the last parameter, when request context is changed to required by some reason, all parameters before it (which means all parameters) should be required
var protocolMethodParameters = isRequestContextOptional
? _orderedParameters.Select(p => p.Protocol).WhereNotNull().ToArray()
: _orderedParameters.Select(p => p.Protocol?.ToRequired()).WhereNotNull().ToArray();
var protocolMethodModifiers = (Operation.GenerateProtocolMethod ? _restClientMethod.Accessibility : Internal) | Virtual;
var protocolMethodSignature = new MethodSignature(_restClientMethod.Name, FormattableStringHelpers.FromString(_restClientMethod.Summary), FormattableStringHelpers.FromString(_restClientMethod.Description), protocolMethodModifiers, _returnType.Protocol, null, protocolMethodParameters, protocolMethodAttributes);
var convenienceMethodInfo = ShouldGenerateConvenienceMethod();
var convenienceMethod = BuildConvenienceMethod(shouldRequestContextOptional, convenienceMethodInfo);
var convenienceMethod = BuildConvenienceMethod(isRequestContextOptional, convenienceMethodInfo);

var diagnostic = new Diagnostic($"{_clientName}.{_restClientMethod.Name}");

Expand Down Expand Up @@ -202,7 +205,7 @@ private ConvenienceMethodGenerationInfo ShouldGenerateConvenienceMethod()
// If all the corresponding parameters and return types of convenience method and protocol method have the same type, it does not make sense to generate the convenience method.
private bool IsConvenienceMethodMeaningful()
{
return _orderedParameters.Where(parameter => parameter.Convenience != KnownParameters.CancellationTokenParameter).Any(parameter => !IsParameterTypeSame(parameter.Convenience, parameter.Protocol))
return _orderedParameters.Where(parameter => parameter.Protocol != KnownParameters.RequestContext && parameter.Protocol != KnownParameters.RequestContextRequired).Any(parameter => !IsParameterTypeSame(parameter.Convenience, parameter.Protocol))
|| !_returnType.Convenience.Equals(_returnType.Protocol);
}

Expand Down Expand Up @@ -382,6 +385,11 @@ private ReturnTypeChain BuildReturnTypes()
protocolToConvenience.Add(new ProtocolToConvenienceParameterConverter(protocolParameter, convenienceParameter, null));
}
}
else if (protocolParameter != null)
{
// convenience parameter is null - such as CancellationToken in non-azure library
protocolToConvenience.Add(new ProtocolToConvenienceParameterConverter(protocolParameter, convenienceParameter, null));
}
}
var accessibility = _restClientMethod.Accessibility | Virtual;
if (generationInfo.IsConvenienceMethodInternal)
Expand Down Expand Up @@ -590,7 +598,7 @@ public void AddRequestConditionHeaders(RequestConditionHeaders conditionHeaderFl
public void AddRequestContext()
{
_orderedParameters.Add(new ParameterChain(
KnownParameters.CancellationTokenParameter,
Configuration.IsBranded ? KnownParameters.CancellationTokenParameter : null, // in non-azure libraries, convenience method no longer takes a `CancellationToken` parameter.
ShouldRequestContextOptional() ? KnownParameters.RequestContext : KnownParameters.RequestContextRequired,
KnownParameters.RequestContext));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ public static bool ShouldGenerateShortVersion(LowLevelClient client, LowLevelCli
if (method.ConvenienceMethod is not null)
{
if (method.ConvenienceMethod.Signature.Parameters.Count == method.ProtocolMethodSignature.Parameters.Count - 1 &&
method.ConvenienceMethod.Signature.Parameters.Count > 0 &&
!method.ConvenienceMethod.Signature.Parameters.Last().Type.Equals(typeof(CancellationToken)))
{
bool allEqual = true;
Expand Down
Loading