-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Add infrastructure for RequestDelegateGenerator #45924
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
Changes from all commits
182f02d
2d8a509
25dc43a
b0e147c
e5bf644
b9b8cec
f4af25d
96db4db
6665934
bb9243d
2d78789
5e88875
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,11 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Suppressions xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"> | ||
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema"> | ||
<Suppression> | ||
<DiagnosticId>PKV004</DiagnosticId> | ||
<Target>net7.0</Target> | ||
</Suppression> | ||
<Suppression> | ||
<DiagnosticId>PKV004</DiagnosticId> | ||
<Target>net8.0</Target> | ||
</Suppression> | ||
</Suppressions> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,11 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Suppressions xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"> | ||
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema"> | ||
<Suppression> | ||
<DiagnosticId>PKV0001</DiagnosticId> | ||
<Target>net7.0</Target> | ||
</Suppression> | ||
<Suppression> | ||
<DiagnosticId>PKV0001</DiagnosticId> | ||
<Target>net8.0</Target> | ||
</Suppression> | ||
</Suppressions> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>netstandard2.0</TargetFramework> | ||
<EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules> | ||
<IsPackable>false</IsPackable> | ||
<IsAnalyzersProject>true</IsAnalyzersProject> | ||
<AddPublicApiAnalyzers>false</AddPublicApiAnalyzers> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<Reference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="all" /> | ||
<Reference Include="Microsoft.CodeAnalysis.Common" PrivateAssets="all" /> | ||
Comment on lines
+12
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a reference to |
||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<None Include="$(OutputPath)\$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<InternalsVisibleTo Include="Microsoft.AspNetCore.Http.Extensions.Tests" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<Compile Include="$(SharedSourceRoot)IsExternalInit.cs" LinkBase="Shared" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Linq; | ||
using System.Text; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
using Microsoft.CodeAnalysis.Operations; | ||
using Microsoft.AspNetCore.Http.Generators.StaticRouteHandlerModel; | ||
|
||
namespace Microsoft.AspNetCore.Http.Generators; | ||
|
||
[Generator] | ||
public sealed class RequestDelegateGenerator : IIncrementalGenerator | ||
{ | ||
private static readonly string[] _knownMethods = | ||
{ | ||
"MapGet", | ||
"MapPost", | ||
"MapPut", | ||
"MapDelete", | ||
"MapPatch", | ||
"Map", | ||
}; | ||
|
||
public void Initialize(IncrementalGeneratorInitializationContext context) | ||
{ | ||
var endpoints = context.SyntaxProvider.CreateSyntaxProvider( | ||
predicate: (node, _) => node is InvocationExpressionSyntax | ||
{ | ||
Expression: MemberAccessExpressionSyntax | ||
{ | ||
Name: IdentifierNameSyntax | ||
{ | ||
Identifier: { ValueText: var method } | ||
} | ||
}, | ||
ArgumentList: { Arguments: { Count: 2 } args } | ||
} && _knownMethods.Contains(method), | ||
transform: (context, token) => | ||
{ | ||
var operation = context.SemanticModel.GetOperation(context.Node, token) as IInvocationOperation; | ||
return StaticRouteHandlerModelParser.GetEndpointFromOperation(operation); | ||
}) | ||
.Where(endpoint => endpoint.Response.ResponseType == "string") | ||
.WithTrackingName("EndpointModel"); | ||
|
||
var thunks = endpoints.Select((endpoint, _) => $$""" | ||
[{{StaticRouteHandlerModelEmitter.EmitSourceKey(endpoint)}}] = ( | ||
(del, builder) => | ||
{ | ||
builder.Metadata.Add(new SourceKey{{StaticRouteHandlerModelEmitter.EmitSourceKey(endpoint)}}); | ||
}, | ||
(del, builder) => | ||
{ | ||
var handler = ({{StaticRouteHandlerModelEmitter.EmitHandlerDelegateType(endpoint)}})del; | ||
EndpointFilterDelegate? filteredInvocation = null; | ||
|
||
if (builder.FilterFactories.Count > 0) | ||
{ | ||
filteredInvocation = BuildFilterDelegate(ic => | ||
{ | ||
if (ic.HttpContext.Response.StatusCode == 400) | ||
{ | ||
return System.Threading.Tasks.ValueTask.FromResult<object?>(Results.Empty); | ||
} | ||
{{StaticRouteHandlerModelEmitter.EmitFilteredInvocation()}} | ||
}, | ||
builder, | ||
handler.Method); | ||
} | ||
|
||
{{StaticRouteHandlerModelEmitter.EmitRequestHandler()}} | ||
{{StaticRouteHandlerModelEmitter.EmitFilteredRequestHandler()}} | ||
|
||
return filteredInvocation is null ? RequestHandler : RequestHandlerFiltered; | ||
}), | ||
"""); | ||
|
||
var stronglyTypedEndpointDefinitions = endpoints.Select((endpoint, _) => $$""" | ||
{{RequestDelegateGeneratorSources.GeneratedCodeAttribute}} | ||
internal static Microsoft.AspNetCore.Builder.RouteHandlerBuilder {{endpoint.HttpMethod}}( | ||
this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder endpoints, | ||
[System.Diagnostics.CodeAnalysis.StringSyntax("Route")] string pattern, | ||
{{StaticRouteHandlerModelEmitter.EmitHandlerDelegateType(endpoint)}} handler, | ||
[System.Runtime.CompilerServices.CallerFilePath] string filePath = "", | ||
[System.Runtime.CompilerServices.CallerLineNumber]int lineNumber = 0) | ||
{ | ||
return MapCore(endpoints, pattern, handler, GetVerb, filePath, lineNumber); | ||
} | ||
"""); | ||
|
||
var thunksAndEndpoints = thunks.Collect().Combine(stronglyTypedEndpointDefinitions.Collect()); | ||
|
||
context.RegisterSourceOutput(thunksAndEndpoints, (context, sources) => | ||
{ | ||
var (thunks, endpoints) = sources; | ||
|
||
var endpointsCode = new StringBuilder(); | ||
var thunksCode = new StringBuilder(); | ||
foreach (var endpoint in endpoints) | ||
{ | ||
endpointsCode.AppendLine(endpoint); | ||
} | ||
foreach (var thunk in thunks) | ||
{ | ||
thunksCode.AppendLine(thunk); | ||
} | ||
|
||
var code = RequestDelegateGeneratorSources.GetGeneratedRouteBuilderExtensionsSource( | ||
genericThunks: string.Empty, | ||
captainsafia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
thunks: thunksCode.ToString(), | ||
endpoints: endpointsCode.ToString()); | ||
context.AddSource("GeneratedRouteBuilderExtensions.g.cs", code); | ||
}); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my learning - what are these files for? And why do we need both
net7.0
andnet8.0
in them?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure of the reasons but I ran this delta past @wtgodbe since it was automatically introduced when I ran the
GenerateProjectList
task after adding the new source generator project to the repo.From my understanding, it's a requirement of the package validation SDK we use for validating changes across versions.
As to why we have both versions, I'm thinking we might be able to get away with just having
net8
in the main branch but I'll let @wtgodbe share.For now, I just settled with whatever the build produced here 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think net7.0 can be removed now that the source is targeting .NET 8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Roger that. It probably makes sense to do this in a separate PR. I assume there's no harm to having
net7.0
here since we've had it in the main branch for a while.We might also want to add removing the older framework target as part of our updating branding instructions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notice that these files keep changing because the order of the namespaces emitted into the file keep changing. Anyone know the reason for this non-determinism?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ViktorHofer and @smasher164 own the Package Validation SDK - any idea on why there's a net7.0 & and a net8.0 section in the generated file, or why there may nondeterminism in the ordering of the file? It's been somewhat noisy lately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smasher164 can you please take a look? We might have caused a non-deterministic behavior by ordering the compatibility differences in ApiCompat. Or maybe not and something else is wrong.