Skip to content

Add initial parameter support to RequestDelegateGenerator #46407

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.

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 6 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 12 additions & 0 deletions src/Http/Http.Extensions/gen/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,16 @@ internal static class DiagnosticDescriptors
DiagnosticSeverity.Warning,
isEnabledByDefault: true
);

// This is temporary. The plan is to be able to resolve all parameters to a known EndpointParameterSource.
public static DiagnosticDescriptor GetUnableToResolveParameterDescriptor(string parameterName)
{
return new(
"RDG073",
new LocalizableResourceString(nameof(Resources.UnableToResolveParameter_Title), Resources.ResourceManager, typeof(Resources)),
new LocalizableResourceString(nameof(Resources.FormatUnableToResolveParameter_Message), Resources.ResourceManager, typeof(Resources), parameterName),
"Usage",
DiagnosticSeverity.Hidden,
isEnabledByDefault: true);
}
}
14 changes: 2 additions & 12 deletions src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
{
context.ReportDiagnostic(Diagnostic.Create(diagnostic, endpoint.Operation.Syntax.GetLocation(), filePath));
}
foreach (var diagnostic in endpoint.Response.Diagnostics)
{
context.ReportDiagnostic(Diagnostic.Create(diagnostic, endpoint.Operation.Syntax.GetLocation(), filePath));
}
foreach (var diagnostic in endpoint.Route.Diagnostics)
{
context.ReportDiagnostic(Diagnostic.Create(diagnostic, endpoint.Operation.Syntax.GetLocation(), filePath));
}
});

var endpoints = endpointsWithDiagnostics
.Where(endpoint => endpoint.Diagnostics.Count == 0 &&
endpoint.Response.Diagnostics.Count == 0 &&
endpoint.Route.Diagnostics.Count == 0)
.Where(endpoint => endpoint.Diagnostics.Count == 0)
.WithTrackingName(GeneratorSteps.EndpointsWithoutDiagnosicsStep);

var thunks = endpoints.Select((endpoint, _) => $$"""
Expand Down Expand Up @@ -97,7 +87,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
}

{{endpoint.EmitRequestHandler()}}
{{StaticRouteHandlerModelEmitter.EmitFilteredRequestHandler()}}
{{endpoint.EmitFilteredRequestHandler()}}

RequestDelegate targetDelegate = filteredInvocation is null ? RequestHandler : RequestHandlerFiltered;
var metadata = inferredMetadataResult?.EndpointMetadata ?? ReadOnlyCollection<object>.Empty;
Expand Down
62 changes: 34 additions & 28 deletions src/Http/Http.Extensions/gen/Resources.resx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema

<!--
Microsoft ResX Schema
Version 2.0

The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.

Example:

... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>

There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.

Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.

The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:

Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.

mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -129,4 +129,10 @@
<data name="UnableToResolveMethod_Message" xml:space="preserve">
<value>Unable to statically resolve endpoint handler method. Only method groups, lambda expressions or readonly fields/variables are allowed. Compile-time endpoint generation will skip this endpoint.</value>
</data>
</root>
<data name="UnableToResolveParameter_Message" xml:space="preserve">
<value>Unable to statically resolve parameter '{0}' for endpoint. Compile-time endpoint generation will skip this endpoint.</value>
</data>
<data name="UnableToResolveParameter_Title" xml:space="preserve">
<value>Unable to resolve parameter</value>
</data>
</root>
136 changes: 97 additions & 39 deletions src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Endpoint.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// 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.Collections.Generic;
using System.Linq;
Expand All @@ -12,61 +13,118 @@ namespace Microsoft.AspNetCore.Http.Generators.StaticRouteHandlerModel;

internal class Endpoint
{
public string HttpMethod { get; }
public EndpointRoute Route { get; }
public EndpointResponse Response { get; }
public List<DiagnosticDescriptor> Diagnostics { get; } = new List<DiagnosticDescriptor>();
public (string, int) Location { get; }
public IInvocationOperation Operation { get; }

private WellKnownTypes WellKnownTypes { get; }
private string? _argumentListCache;

public Endpoint(IInvocationOperation operation, WellKnownTypes wellKnownTypes)
{
Operation = operation;
WellKnownTypes = wellKnownTypes;
Location = GetLocation();
HttpMethod = GetHttpMethod();
Response = new EndpointResponse(Operation, wellKnownTypes);
Route = new EndpointRoute(Operation);
}
Location = GetLocation(operation);
HttpMethod = GetHttpMethod(operation);

private (string, int) GetLocation()
{
var filePath = Operation.Syntax.SyntaxTree.FilePath;
var span = Operation.Syntax.SyntaxTree.GetLineSpan(Operation.Syntax.Span);
var lineNumber = span.EndLinePosition.Line + 1;
return (filePath, lineNumber);
}
if (!operation.TryGetRouteHandlerPattern(out var routeToken))
{
Diagnostics.Add(DiagnosticDescriptors.UnableToResolveRoutePattern);
Copy link
Member

Choose a reason for hiding this comment

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

I'm inclined to have each component of the model maintain its own diagnostics. As we build up the diagnostics we emit, it'll make it easier to warn for specific scenarios that might not be immediately accessible at the top-level.

For example, let's say that we find a TryParse method on a parameter that doesn't have the correct signature. It would be easier to read and track if we were to capture that diagnostic when we're constructing the parameter instead of bubbling it up and capturing them all at the top.

The downside is it does make the code to aggregate them all when we emit a little honky but that seems worth it for the benefits of having diagnostics localized to where they would be thrown (similar to an exception).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be better to give EndpointParameter a backreference to Endpoints.Diagnostics if we want to capture the diagnostic while constructing the parameter. I just didn't like greedily creating a list for each small component even though it's not that expensive. The aggregation and repeated declarations were kind of verbose too as you say.

return;
}

private string GetHttpMethod()
{
var syntax = (InvocationExpressionSyntax)Operation.Syntax;
var expression = (MemberAccessExpressionSyntax)syntax.Expression;
var name = (IdentifierNameSyntax)expression.Name;
var identifier = name.Identifier;
return identifier.ValueText;
RoutePattern = routeToken.ValueText;

if (!operation.TryGetRouteHandlerMethod(out var method))
{
Diagnostics.Add(DiagnosticDescriptors.UnableToResolveMethod);
return;
}

Response = new EndpointResponse(method, wellKnownTypes);

if (method.Parameters.Length == 0)
{
return;
}

var parameters = new EndpointParameter[method.Parameters.Length];

for (var i = 0; i < method.Parameters.Length; i++)
{
var parameter = new EndpointParameter(method.Parameters[i], wellKnownTypes);

if (parameter.Source == EndpointParameterSource.Unknown)
{
Diagnostics.Add(DiagnosticDescriptors.GetUnableToResolveParameterDescriptor(parameter.Name));
return;
}

parameters[i] = parameter;
}

Parameters = parameters;
}

public override bool Equals(object o)
public string HttpMethod { get; }
public string? RoutePattern { get; }
public EndpointResponse? Response { get; }
public EndpointParameter[] Parameters { get; } = Array.Empty<EndpointParameter>();
public string EmitArgumentList() => _argumentListCache ??= string.Join(", ", Parameters.Select(p => p.EmitArgument()));

public List<DiagnosticDescriptor> Diagnostics { get; } = new List<DiagnosticDescriptor>();

public (string File, int LineNumber) Location { get; }
public IInvocationOperation Operation { get; }

public override bool Equals(object o) =>
o is Endpoint other && Location == other.Location && SignatureEquals(this, other);

public override int GetHashCode() =>
HashCode.Combine(Location, GetSignatureHashCode(this));

public static bool SignatureEquals(Endpoint a, Endpoint b)
{
if (o is null)
if (!a.Response.WrappedResponseType.Equals(b.Response.WrappedResponseType, StringComparison.Ordinal) ||
!a.HttpMethod.Equals(b.HttpMethod, StringComparison.Ordinal) ||
a.Parameters.Length != b.Parameters.Length)
{
return false;
}

if (o is Endpoint endpoint)
for (var i = 0; i < a.Parameters.Length; i++)
{
return endpoint.HttpMethod.Equals(HttpMethod, StringComparison.OrdinalIgnoreCase) &&
endpoint.Location.Item1.Equals(Location.Item1, StringComparison.OrdinalIgnoreCase) &&
endpoint.Location.Item2.Equals(Location.Item2) &&
endpoint.Response.Equals(Response) &&
endpoint.Diagnostics.SequenceEqual(Diagnostics);
if (a.Parameters[i].Equals(b.Parameters[i]))
{
return false;
}
}

return false;
return true;
}

public override int GetHashCode() =>
HashCode.Combine(HttpMethod, Route, Location, Response, Diagnostics);
public static int GetSignatureHashCode(Endpoint endpoint)
{
var hashCode = new HashCode();
hashCode.Add(endpoint.Response.WrappedResponseType);
hashCode.Add(endpoint.HttpMethod);

foreach (var parameter in endpoint.Parameters)
{
hashCode.Add(parameter);
}

return hashCode.ToHashCode();
}

private static (string, int) GetLocation(IInvocationOperation operation)
{
var filePath = operation.Syntax.SyntaxTree.FilePath;
var span = operation.Syntax.SyntaxTree.GetLineSpan(operation.Syntax.Span);
var lineNumber = span.StartLinePosition.Line + 1;
return (filePath, lineNumber);
}

private static string GetHttpMethod(IInvocationOperation operation)
{
var syntax = (InvocationExpressionSyntax)operation.Syntax;
var expression = (MemberAccessExpressionSyntax)syntax.Expression;
var name = (IdentifierNameSyntax)expression.Name;
var identifier = name.Identifier;
return identifier.ValueText;
}
}
Original file line number Diff line number Diff line change
@@ -1,31 +1,14 @@
// 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.Collections;

using System.Collections.Generic;

namespace Microsoft.AspNetCore.Http.Generators.StaticRouteHandlerModel;

internal sealed class EndpointDelegateComparer : IEqualityComparer<Endpoint>, IComparer<Endpoint>
internal sealed class EndpointDelegateComparer : IEqualityComparer<Endpoint>
{
public static readonly EndpointDelegateComparer Instance = new EndpointDelegateComparer();

public bool Equals(Endpoint a, Endpoint b) => Compare(a, b) == 0;

public int GetHashCode(Endpoint endpoint) => HashCode.Combine(
endpoint.Response.WrappedResponseType,
endpoint.Response.IsVoid,
endpoint.Response.IsAwaitable,
endpoint.HttpMethod);

public int Compare(Endpoint a, Endpoint b)
{
if (a.Response.IsAwaitable == b.Response.IsAwaitable &&
a.Response.IsVoid == b.Response.IsVoid &&
a.Response.WrappedResponseType.Equals(b.Response.WrappedResponseType, StringComparison.Ordinal) &&
a.HttpMethod.Equals(b.HttpMethod, StringComparison.Ordinal))
{
return 0;
}
return -1;
}
public bool Equals(Endpoint a, Endpoint b) => Endpoint.SignatureEquals(a, b);
public int GetHashCode(Endpoint endpoint) => Endpoint.GetSignatureHashCode(endpoint);
}
Loading