Skip to content

Add BindAsync support to RequestDelegateGenerator #47033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 6, 2023
Merged

Add BindAsync support to RequestDelegateGenerator #47033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
24427ae
Add BindAsync support to RequestDelegateGenerator
halter73 Mar 4, 2023
42f78c8
Support explicit interface implementations
halter73 Mar 4, 2023
b616548
Add MapAction_BindAsync_Snapshot
halter73 Mar 4, 2023
f08cd59
Route parameters do not support BindAsync
halter73 Mar 4, 2023
3f99307
Move test types to shared types
captainsafia Mar 6, 2023
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
4 changes: 2 additions & 2 deletions src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ internal static class DiagnosticDescriptors
isEnabledByDefault: true,
helpLinkUri: "https://aka.ms/aspnet/analyzers");

internal static readonly DiagnosticDescriptor RouteParameterComplexTypeIsNotParsableOrBindable = new(
internal static readonly DiagnosticDescriptor RouteParameterComplexTypeIsNotParsable = new(
"ASP0020",
"Complex types referenced by route parameters must be parsable",
"Parameter '{0}' of type {1} should define a bool TryParse(string, IFormatProvider, out {1}) method, or implement IParsable<{1}>, or define a ValueTask<{1}> BindAsync(HttpContext), or implement IBindableFromHttpContext<{1}>.",
"Parameter '{0}' of type {1} should define a bool TryParse(string, IFormatProvider, out {1}) method, or implement IParsable<{1}>.",
"Usage",
DiagnosticSeverity.Error,
isEnabledByDefault: true,
Expand Down
5 changes: 1 addition & 4 deletions src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@
<Compile Include="$(SharedSourceRoot)RoslynUtils\WellKnownTypes.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\WellKnownTypeData.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\SymbolExtensions.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\Bindability.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RoslynUtils\Parsability.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RoslynUtils\ParsabilityMethod.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RoslynUtils\ParsabilityHelper.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RoslynUtils\ParsabilityHelper.cs" LinkBase="Shared" />
</ItemGroup>

</Project>
22 changes: 3 additions & 19 deletions ...tCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,11 @@ private static void DisallowNonParsableComplexTypesOnParameters(
if (IsRouteParameter(routeUsage, handlerDelegateParameter))
{
var parsability = ParsabilityHelper.GetParsability(parameterTypeSymbol, wellKnownTypes);
var bindability = ParsabilityHelper.GetBindability(parameterTypeSymbol, wellKnownTypes);

if (!(parsability == Parsability.Parsable || bindability == Bindability.Bindable))
if (parsability != Parsability.Parsable)
{
var descriptor = SelectDescriptor(parsability, bindability);

context.ReportDiagnostic(Diagnostic.Create(
descriptor,
DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable,
location,
handlerDelegateParameter.Name,
parameterTypeSymbol.Name
Expand Down Expand Up @@ -103,10 +100,8 @@ static bool ReportFromAttributeDiagnostic(OperationAnalysisContext context, Well
var parsability = ParsabilityHelper.GetParsability(parameterTypeSymbol, wellKnownTypes);
if (parameter.HasAttributeImplementingInterface(fromMetadataInterfaceTypeSymbol) && parsability != Parsability.Parsable)
{
var descriptor = SelectDescriptor(parsability, Bindability.NotBindable);

context.ReportDiagnostic(Diagnostic.Create(
descriptor,
DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable,
location,
parameter.Name,
parameterTypeSymbol.Name
Expand Down Expand Up @@ -140,16 +135,5 @@ static bool ReportFromAttributeDiagnostic(OperationAnalysisContext context, Well

return parameterTypeSymbol;
}

static DiagnosticDescriptor SelectDescriptor(Parsability parsability, Bindability bindability)
Copy link
Member

@mitchdenny mitchdenny Mar 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you introducing a new analyzer here? I thought that we wanted to cover parsability and bindability detection in that analyzer?

{
// This abomination is used to take the parsability and bindability and together figure
// out what the most optimal diagnostic message is to give to our plucky user.
return (parsability, bindability) switch
{
{ parsability: Parsability.NotParsable, bindability: Bindability.InvalidReturnType } => DiagnosticDescriptors.BindAsyncSignatureMustReturnValueTaskOfT,
_ => DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsableOrBindable
};
}
}
}
2 changes: 1 addition & 1 deletion src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/RouteHandlerAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public partial class RouteHandlerAnalyzer : DiagnosticAnalyzer
DiagnosticDescriptors.DoNotReturnActionResultsFromRouteHandlers,
DiagnosticDescriptors.DetectMisplacedLambdaAttribute,
DiagnosticDescriptors.DetectMismatchedParameterOptionality,
DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsableOrBindable,
DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable,
DiagnosticDescriptors.BindAsyncSignatureMustReturnValueTaskOfT,
DiagnosticDescriptors.AmbiguousRouteHandlerRoute,
DiagnosticDescriptors.AtMostOneFromBodyAttribute
Expand Down
30 changes: 19 additions & 11 deletions ...AspNetCoreAnalyzers/test/RouteHandlers/DisallowNonParsableComplexTypesOnParametersTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public class Customer
}
""";

var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsableOrBindable)
var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable)
.WithArguments("customer", "Customer")
.WithLocation(0);

Expand All @@ -218,7 +218,7 @@ public class Customer
}
""";

var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsableOrBindable)
var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable)
.WithArguments("customer", "Customer")
.WithLocation(0);

Expand Down Expand Up @@ -289,7 +289,7 @@ public static bool TryParse(string s, IFormatProvider provider, out Customer res
}

[Fact]
public async Task Route_Parameter_withBindAsyncMethodThatReturnsTask_of_T_Fails()
public async Task Route_Parameter_withBindAsyncMethod_Fails()
{
// Arrange
var source = $$"""
Expand All @@ -301,14 +301,14 @@ public async Task Route_Parameter_withBindAsyncMethodThatReturnsTask_of_T_Fails(

public class Customer
{
public async static Task<Customer> BindAsync(HttpContext context)
public async static ValueTask<Customer> BindAsync(HttpContext context)
Copy link
Member

@mitchdenny mitchdenny Mar 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the intention for this test was to fail due to the return type of BindAsync being Task<T> and not ValueTask<T>

{
return new Customer();
}
}
""";

var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.BindAsyncSignatureMustReturnValueTaskOfT)
var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable)
.WithArguments("customer", "Customer")
.WithLocation(0);

Expand Down Expand Up @@ -422,7 +422,7 @@ public static bool TryParse(string s, IFormatProvider provider, out Customer res
}

[Fact]
public async Task Route_Parameter_withHttpContextBindableComplexType_viaImplicitIBindableFromHttp_Works()
public async Task Route_Parameter_withHttpContextBindableComplexType_viaImplicitIBindableFromHttp_Fails()
{
// Arrange
var source = $$"""
Expand All @@ -433,7 +433,7 @@ public async Task Route_Parameter_withHttpContextBindableComplexType_viaImplicit
using Microsoft.AspNetCore.Http;

var webApp = WebApplication.Create();
webApp.MapGet("/customers/{customer}", (Customer customer) => {});
webApp.MapGet("/customers/{customer}", ({|#0:Customer customer|}) => {});

public class Customer : IBindableFromHttpContext<Customer>
{
Expand All @@ -444,12 +444,16 @@ public class Customer : IBindableFromHttpContext<Customer>
}
""";

var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable)
.WithArguments("customer", "Customer")
.WithLocation(0);

// Act
await VerifyCS.VerifyAnalyzerAsync(source);
await VerifyCS.VerifyAnalyzerAsync(source, expectedDiagnostic);
}

[Fact]
public async Task Route_Parameter_withHttpContextBindableComplexType_viaExplicitIBindableFromHttp_Works()
public async Task Route_Parameter_withHttpContextBindableComplexType_viaExplicitIBindableFromHttp_Fails()
{
// Arrange
var source = $$"""
Expand All @@ -460,7 +464,7 @@ public async Task Route_Parameter_withHttpContextBindableComplexType_viaExplicit
using Microsoft.AspNetCore.Http;

var webApp = WebApplication.Create();
webApp.MapGet("/customers/{customer}", (Customer customer) => {});
webApp.MapGet("/customers/{customer}", ({|#0:Customer customer|}) => {});

public class Customer : IBindableFromHttpContext<Customer>
{
Expand All @@ -471,8 +475,12 @@ public class Customer : IBindableFromHttpContext<Customer>
}
""";

var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable)
.WithArguments("customer", "Customer")
.WithLocation(0);

// Act
await VerifyCS.VerifyAnalyzerAsync(source);
await VerifyCS.VerifyAnalyzerAsync(source, expectedDiagnostic);
}

[Fact]
Expand Down
5 changes: 1 addition & 4 deletions src/Http/Http.Extensions/gen/Microsoft.AspNetCore.Http.Generators.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@
<Compile Include="$(SharedSourceRoot)RoslynUtils\WellKnownTypeData.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\WellKnownTypes.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\SymbolExtensions.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\Bindability.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RoslynUtils\Parsability.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RoslynUtils\ParsabilityMethod.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RoslynUtils\ParsabilityHelper.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RoslynUtils\ParsabilityHelper.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\CodeWriter.cs" LinkBase="Shared" />
</ItemGroup>

Expand Down
4 changes: 4 additions & 0 deletions src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
codeWriter.WriteLine("EndpointFilterDelegate? filteredInvocation = null;");
endpoint.EmitRouteOrQueryResolver(codeWriter);
endpoint.EmitJsonPreparation(codeWriter);
if (endpoint.NeedsParameterArray)
{
codeWriter.WriteLine("var parameters = del.Method.GetParameters();");
}
codeWriter.WriteLineNoTabs(string.Empty);
codeWriter.WriteLine("if (options?.EndpointBuilder?.FilterFactories.Count > 0)");
codeWriter.StartBlock();
Expand Down
6 changes: 6 additions & 0 deletions src/Http/Http.Extensions/gen/RequestDelegateGeneratorSources.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ private static Task WriteToResponseAsync<T>(T? value, HttpContext httpContext, J
}
return (false, default);
}

private static ValueTask<T?> BindAsync<T>(HttpContext context, ParameterInfo parameter)
where T : class, IBindableFromHttpContext<T>
{
return T.BindAsync(context, parameter);
}
}
}
""";
Expand Down
22 changes: 12 additions & 10 deletions src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointEmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using System.Globalization;
using System.IO;
using System.Text;
using System.Linq;

namespace Microsoft.AspNetCore.Http.Generators.StaticRouteHandlerModel.Emitters;
internal static class EndpointEmitter
Expand All @@ -17,24 +15,28 @@ internal static string EmitParameterPreparation(this Endpoint endpoint, int base

foreach (var parameter in endpoint.Parameters)
{
switch (parameter)
switch (parameter.Source)
{
case { Source: EndpointParameterSource.SpecialType }:
case EndpointParameterSource.SpecialType:
parameter.EmitSpecialParameterPreparation(parameterPreparationBuilder);
break;
case { Source: EndpointParameterSource.Query or EndpointParameterSource.Header }:
case EndpointParameterSource.Query:
case EndpointParameterSource.Header:
parameter.EmitQueryOrHeaderParameterPreparation(parameterPreparationBuilder);
break;
case { Source: EndpointParameterSource.Route }:
case EndpointParameterSource.Route:
parameter.EmitRouteParameterPreparation(parameterPreparationBuilder);
break;
case { Source: EndpointParameterSource.RouteOrQuery }:
case EndpointParameterSource.RouteOrQuery:
parameter.EmitRouteOrQueryParameterPreparation(parameterPreparationBuilder);
break;
case { Source: EndpointParameterSource.JsonBody }:
case EndpointParameterSource.BindAsync:
parameter.EmitBindAsyncPreparation(parameterPreparationBuilder);
break;
case EndpointParameterSource.JsonBody:
parameter.EmitJsonBodyParameterPreparationString(parameterPreparationBuilder);
break;
case { Source: EndpointParameterSource.Service }:
case EndpointParameterSource.Service:
parameter.EmitServiceParameterPreparation(parameterPreparationBuilder);
break;
}
Expand Down
56 changes: 50 additions & 6 deletions src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointParameterEmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Text;
using Microsoft.AspNetCore.Analyzers.Infrastructure;
using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure;
using Microsoft.CodeAnalysis;

namespace Microsoft.AspNetCore.Http.Generators.StaticRouteHandlerModel.Emitters;

internal static class EndpointParameterEmitter
{
internal static void EmitSpecialParameterPreparation(this EndpointParameter endpointParameter, CodeWriter codeWriter)
Expand All @@ -26,15 +28,15 @@ internal static void EmitQueryOrHeaderParameterPreparation(this EndpointParamete
// compiler errors around null handling.
if (endpointParameter.IsOptional)
{
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = {endpointParameter.EmitAssigningCodeResult()}.Count > 0 ? {endpointParameter.EmitAssigningCodeResult()}.ToString() : null;");
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = {endpointParameter.EmitAssigningCodeResult()}.Count > 0 ? (string?){endpointParameter.EmitAssigningCodeResult()} : null;");
}
else
{
codeWriter.WriteLine($"if (StringValues.IsNullOrEmpty({endpointParameter.EmitAssigningCodeResult()}))");
codeWriter.StartBlock();
codeWriter.WriteLine("wasParamCheckFailure = true;");
codeWriter.EndBlock();
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = {endpointParameter.EmitAssigningCodeResult()}.ToString();");
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = (string?){endpointParameter.EmitAssigningCodeResult()};");
}

endpointParameter.EmitParsingBlock(codeWriter);
Expand Down Expand Up @@ -64,7 +66,7 @@ internal static void EmitRouteParameterPreparation(this EndpointParameter endpoi
codeWriter.WriteLine($$"""throw new InvalidOperationException($"'{{endpointParameter.Name}}' is not a route parameter.");""");
codeWriter.EndBlock();

var assigningCode = $"httpContext.Request.RouteValues[\"{endpointParameter.Name}\"]?.ToString()";
var assigningCode = $"(string?)httpContext.Request.RouteValues[\"{endpointParameter.Name}\"]";
codeWriter.WriteLine($"var {endpointParameter.EmitAssigningCodeResult()} = {assigningCode};");

if (!endpointParameter.IsOptional)
Expand All @@ -75,7 +77,7 @@ internal static void EmitRouteParameterPreparation(this EndpointParameter endpoi
codeWriter.EndBlock();
}

codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = {endpointParameter.EmitAssigningCodeResult()}?.ToString();");
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = (string?){endpointParameter.EmitAssigningCodeResult()};");
endpointParameter.EmitParsingBlock(codeWriter);
}

Expand All @@ -94,7 +96,8 @@ internal static void EmitRouteOrQueryParameterPreparation(this EndpointParameter
codeWriter.EndBlock();
}

codeWriter.WriteLine($"var {endpointParameter.EmitHandlerArgument()} = {endpointParameter.EmitAssigningCodeResult()};");
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = (string?){endpointParameter.EmitAssigningCodeResult()};");
endpointParameter.EmitParsingBlock(codeWriter);
}

internal static void EmitJsonBodyParameterPreparationString(this EndpointParameter endpointParameter, CodeWriter codeWriter)
Expand All @@ -116,6 +119,47 @@ internal static void EmitJsonBodyParameterPreparationString(this EndpointParamet
codeWriter.EndBlock();
}

internal static void EmitBindAsyncPreparation(this EndpointParameter endpointParameter, CodeWriter codeWriter)
{
var unwrappedType = endpointParameter.Type.UnwrapTypeSymbol();
var unwrappedTypeString = unwrappedType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);

switch (endpointParameter.BindMethod)
{
case BindabilityMethod.IBindableFromHttpContext:
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = await BindAsync<{unwrappedTypeString}>(httpContext, parameters[{endpointParameter.Ordinal}]);");
break;
case BindabilityMethod.BindAsyncWithParameter:
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = await {unwrappedTypeString}.BindAsync(httpContext, parameters[{endpointParameter.Ordinal}]);");
break;
case BindabilityMethod.BindAsync:
codeWriter.WriteLine($"var {endpointParameter.EmitTempArgument()} = await {unwrappedTypeString}.BindAsync(httpContext);");
break;
default:
throw new Exception("Unreachable!");
}

// TODO: Generate more compact code if the type is a reference type and/or the BindAsync return nullability matches the handler parameter type.
if (endpointParameter.IsOptional)
{
codeWriter.WriteLine($"var {endpointParameter.EmitHandlerArgument()} = ({unwrappedTypeString}?){endpointParameter.EmitTempArgument()};");
}
else
{
codeWriter.WriteLine($"{unwrappedTypeString} {endpointParameter.EmitHandlerArgument()};");

codeWriter.WriteLine($"if ((object?){endpointParameter.EmitTempArgument()} == null)");
codeWriter.StartBlock();
codeWriter.WriteLine("wasParamCheckFailure = true;");
codeWriter.WriteLine($"{endpointParameter.EmitHandlerArgument()} = default!;");
codeWriter.EndBlock();
codeWriter.WriteLine("else");
codeWriter.StartBlock();
codeWriter.WriteLine($"{endpointParameter.EmitHandlerArgument()} = ({unwrappedTypeString}){endpointParameter.EmitTempArgument()};");
codeWriter.EndBlock();
}
}

internal static void EmitServiceParameterPreparation(this EndpointParameter endpointParameter, CodeWriter codeWriter)
{
codeWriter.WriteLine(endpointParameter.EmitParameterDiagnosticComment());
Expand Down
Loading