Skip to content

Commit

Permalink
Handle Task/ValueTask.CompletedTask return in RDG (#47211)
Browse files Browse the repository at this point in the history 
* Handle Task/ValueTask.CompletedTask return in RDG

* Simplify awaitable check

* Address feedback from peer review
  • Loading branch information
@captainsafia
captainsafia committed Mar 21, 2023
1 parent f0267e0 commit f050a45
Show file tree
Hide file tree
Showing 30 changed files with 120 additions and 126 deletions.
2 changes: 1 addition & 1 deletion src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
codeWriter.EndBlockWithComma();
codeWriter.WriteLine("(del, options, inferredMetadataResult) =>");
codeWriter.StartBlock();
codeWriter.WriteLine($"var handler = ({endpoint!.EmitHandlerDelegateCast()})del;");
codeWriter.WriteLine($"var handler = ({endpoint!.EmitHandlerDelegateType(considerOptionality: true)})del;");
codeWriter.WriteLine("EndpointFilterDelegate? filteredInvocation = null;");
endpoint!.EmitRouteOrQueryResolver(codeWriter);
endpoint!.EmitJsonBodyOrServiceResolver(codeWriter);
Expand Down
56 changes: 14 additions & 42 deletions src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointResponse.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal class EndpointResponse
public string WrappedResponseType { get; set; }
public string? ContentType { get; set; }
public bool IsAwaitable { get; set; }
public bool IsVoid { get; set; }
public bool HasNoResponse { get; set; }
public bool IsIResult { get; set; }
public bool IsSerializable { get; set; }
public bool IsAnonymousType { get; set; }
Expand All @@ -27,18 +27,20 @@ internal class EndpointResponse
internal EndpointResponse(IMethodSymbol method, WellKnownTypes wellKnownTypes)
{
WellKnownTypes = wellKnownTypes;
ResponseType = UnwrapResponseType(method);
ResponseType = UnwrapResponseType(method, out bool isAwaitable, out bool awaitableIsVoid);
WrappedResponseType = method.ReturnType.ToDisplayString(EmitterConstants.DisplayFormat);
IsAwaitable = GetIsAwaitable(method);
IsVoid = method.ReturnsVoid;
IsAwaitable = isAwaitable;
HasNoResponse = method.ReturnsVoid || awaitableIsVoid;
IsIResult = GetIsIResult();
IsSerializable = GetIsSerializable();
ContentType = GetContentType(method);
IsAnonymousType = method.ReturnType.IsAnonymousType;
}

private ITypeSymbol? UnwrapResponseType(IMethodSymbol method)
private ITypeSymbol? UnwrapResponseType(IMethodSymbol method, out bool isAwaitable, out bool awaitableIsVoid)
{
isAwaitable = false;
awaitableIsVoid = false;
var returnType = method.ReturnType;
var task = WellKnownTypes.Get(WellKnownType.System_Threading_Tasks_Task);
var taskOfT = WellKnownTypes.Get(WellKnownType.System_Threading_Tasks_Task_T);
Expand All @@ -47,12 +49,16 @@ internal EndpointResponse(IMethodSymbol method, WellKnownTypes wellKnownTypes)
if (returnType.OriginalDefinition.Equals(taskOfT, SymbolEqualityComparer.Default) ||
returnType.OriginalDefinition.Equals(valueTaskOfT, SymbolEqualityComparer.Default))
{
isAwaitable = true;
awaitableIsVoid = false;
return ((INamedTypeSymbol)returnType).TypeArguments[0];
}

if (returnType.OriginalDefinition.Equals(task, SymbolEqualityComparer.Default) ||
returnType.OriginalDefinition.Equals(valueTask, SymbolEqualityComparer.Default))
{
isAwaitable = true;
awaitableIsVoid = true;
return null;
}

Expand All @@ -61,7 +67,7 @@ internal EndpointResponse(IMethodSymbol method, WellKnownTypes wellKnownTypes)

private bool GetIsSerializable() =>
!IsIResult &&
!IsVoid &&
!HasNoResponse &&
ResponseType != null &&
ResponseType.SpecialType != SpecialType.System_String &&
ResponseType.SpecialType != SpecialType.System_Object;
Expand All @@ -73,40 +79,6 @@ private bool GetIsIResult()
SymbolEqualityComparer.Default.Equals(ResponseType, resultType);
}

private static bool GetIsAwaitable(IMethodSymbol method)
{
var potentialGetAwaiters = method.ReturnType.OriginalDefinition.GetMembers(WellKnownMemberNames.GetAwaiter);
var getAwaiters = potentialGetAwaiters.OfType<IMethodSymbol>().Where(x => !x.Parameters.Any());
return getAwaiters.Any(symbol => symbol.Name == WellKnownMemberNames.GetAwaiter && VerifyGetAwaiter(symbol));

static bool VerifyGetAwaiter(IMethodSymbol getAwaiter)
{
var returnType = getAwaiter.ReturnType;

// bool IsCompleted { get }
if (!returnType.GetMembers()
.OfType<IPropertySymbol>()
.Any(p => p.Name == WellKnownMemberNames.IsCompleted &&
p.Type.SpecialType == SpecialType.System_Boolean && p.GetMethod != null))
{
return false;
}

var methods = returnType.GetMembers().OfType<IMethodSymbol>();

if (!methods.Any(x => x.Name == WellKnownMemberNames.OnCompleted &&
x.ReturnsVoid &&
x.Parameters.Length == 1 &&
x.Parameters.First().Type.TypeKind == TypeKind.Delegate))
{
return false;
}

// void GetResult() || T GetResult()
return methods.Any(m => m.Name == WellKnownMemberNames.GetResult && !m.Parameters.Any());
}
}

private string? GetContentType(IMethodSymbol method)
{
// `void` returning methods do not have a Content-Type.
Expand All @@ -127,11 +99,11 @@ public override bool Equals(object obj)
SymbolEqualityComparer.Default.Equals(otherEndpointResponse.ResponseType, ResponseType) &&
otherEndpointResponse.WrappedResponseType.Equals(WrappedResponseType, StringComparison.Ordinal) &&
otherEndpointResponse.IsAwaitable == IsAwaitable &&
otherEndpointResponse.IsVoid == IsVoid &&
otherEndpointResponse.HasNoResponse == HasNoResponse &&
otherEndpointResponse.IsIResult == IsIResult &&
string.Equals(otherEndpointResponse.ContentType, ContentType, StringComparison.OrdinalIgnoreCase);
}

public override int GetHashCode() =>
HashCode.Combine(SymbolEqualityComparer.Default.GetHashCode(ResponseType), WrappedResponseType, IsAwaitable, IsVoid, IsIResult, ContentType);
HashCode.Combine(SymbolEqualityComparer.Default.GetHashCode(ResponseType), WrappedResponseType, IsAwaitable, HasNoResponse, IsIResult, ContentType);
}
43 changes: 15 additions & 28 deletions src/Http/Http.Extensions/gen/StaticRouteHandlerModel/StaticRouteHandlerModel.Emitter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,38 +11,23 @@ namespace Microsoft.AspNetCore.Http.Generators.StaticRouteHandlerModel;

internal static class StaticRouteHandlerModelEmitter
{
public static string EmitHandlerDelegateType(this Endpoint endpoint)
public static string EmitHandlerDelegateType(this Endpoint endpoint, bool considerOptionality = false)
{
if (endpoint.Parameters.Length == 0)
{
return endpoint.Response == null || endpoint.Response.IsVoid ? "System.Action" : $"System.Func<{endpoint.Response.WrappedResponseType}>";
return endpoint.Response == null || (endpoint.Response.HasNoResponse && !endpoint.Response.IsAwaitable) ? "System.Action" : $"System.Func<{endpoint.Response.WrappedResponseType}>";
}
var parameterTypeList = string.Join(", ", endpoint.Parameters.Select(p => p.Type.ToDisplayString(EmitterConstants.DisplayFormat)));
var parameterTypeList = string.Join(", ", endpoint.Parameters.Select(p => considerOptionality
? p.Type.ToDisplayString(p.IsOptional ? NullableFlowState.MaybeNull : NullableFlowState.NotNull, EmitterConstants.DisplayFormat)
: p.Type.ToDisplayString(EmitterConstants.DisplayFormat)));

if (endpoint.Response == null || endpoint.Response.IsVoid)
if (endpoint.Response == null || (endpoint.Response.HasNoResponse && !endpoint.Response.IsAwaitable))
{
return $"System.Action<{parameterTypeList}>";
}
return $"System.Func<{parameterTypeList}, {endpoint.Response.WrappedResponseType}>";
}

public static string EmitHandlerDelegateCast(this Endpoint endpoint)
{
if (endpoint.Parameters.Length == 0)
{
return endpoint.Response == null || endpoint.Response.IsVoid ? "Action" : $"Func<{endpoint.Response.WrappedResponseType}>";
}

var parameterTypeList = string.Join(", ", endpoint.Parameters.Select(
p => p.Type.ToDisplayString(p.IsOptional ? NullableFlowState.MaybeNull : NullableFlowState.NotNull, EmitterConstants.DisplayFormat)));

if (endpoint.Response == null || endpoint.Response.IsVoid)
{
return $"Action<{parameterTypeList}>";
}
return $"Func<{parameterTypeList}, {endpoint.Response.WrappedResponseType}>";
}

public static string EmitSourceKey(this Endpoint endpoint)
{
return $@"(@""{endpoint.Location.File}"", {endpoint.Location.LineNumber})";
Expand Down Expand Up @@ -87,11 +72,11 @@ public static void EmitRequestHandler(this Endpoint endpoint, CodeWriter codeWri
{
return;
}
if (!endpoint.Response.IsVoid && endpoint.Response is { ContentType: {} contentType})
if (!endpoint.Response.HasNoResponse && endpoint.Response is { ContentType: {} contentType})
{
codeWriter.WriteLine($@"httpContext.Response.ContentType ??= ""{contentType}"";");
}
if (!endpoint.Response.IsVoid)
if (!endpoint.Response.HasNoResponse)
{
codeWriter.Write("var result = ");
}
Expand All @@ -100,7 +85,7 @@ public static void EmitRequestHandler(this Endpoint endpoint, CodeWriter codeWri
codeWriter.Write("await ");
}
codeWriter.WriteLine($"handler({endpoint.EmitArgumentList()});");
if (!endpoint.Response.IsVoid)
if (!endpoint.Response.HasNoResponse)
{
codeWriter.WriteLine(endpoint.Response.EmitResponseWritingCall(endpoint.IsAwaitable));
}
Expand All @@ -127,11 +112,11 @@ private static string EmitResponseWritingCall(this EndpointResponse endpointResp
{
return $"{returnOrAwait} GeneratedRouteBuilderExtensionsCore.ExecuteObjectResult(result, httpContext);";
}
else if (!endpointResponse.IsVoid)
else if (!endpointResponse.HasNoResponse)
{
return $"{returnOrAwait} {endpointResponse.EmitJsonResponse()}";
}
else if (!endpointResponse.IsAwaitable && endpointResponse.IsVoid)
else if (!endpointResponse.IsAwaitable && endpointResponse.HasNoResponse)
{
return $"{returnOrAwait} Task.CompletedTask;";
}
Expand Down Expand Up @@ -178,9 +163,11 @@ public static void EmitFilteredRequestHandler(this Endpoint endpoint, CodeWriter

public static void EmitFilteredInvocation(this Endpoint endpoint, CodeWriter codeWriter)
{
if (endpoint.Response?.IsVoid == true)
if (endpoint.Response?.HasNoResponse == true)
{
codeWriter.WriteLine($"handler({endpoint.EmitFilteredArgumentList()});");
codeWriter.WriteLine(endpoint.Response?.IsAwaitable == true
? $"await handler({endpoint.EmitFilteredArgumentList()});"
: $"handler({endpoint.EmitFilteredArgumentList()});");
codeWriter.WriteLine("return ValueTask.FromResult<object?>(Results.Empty);");
}
else if (endpoint.Response?.IsAwaitable == true)
Expand Down
Loading

0 comments on commit f050a45

Please sign in to comment.