dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.44k stars 10.02k forks source link

RDG does not support generic types from outer scope #47338

Open halter73 opened 1 year ago

halter73 commented 1 year ago

Given the code:

public static void TestGenericParam<TUser>(IEndpointRouteBuilder app) where TUser : class
{
    app.MapPost("/test-param", ([FromServices] UserManager<TUser> userManager) => { });
}

public static void TestGenericResponse<TUser>(IEndpointRouteBuilder app) where TUser : new()
{
    app.MapPost("/test-return", () => new TUser());
}

RDG will generate code that fails to compile because TUser is undefined. As a short-term mitigation, we can skip generating MapPost and similar methods when we see an open generic parameter in the delegate signature. Long term, we can look at flowing the generic parameters through the Map methods we generate which can then hopefully be inferred.

GeneratedRouteBuilderExtensions.g.cs(73,85): error CS0246: The type or namespace name 'TUser' could not be found (are you missing a using directive or an assembly reference?)
GeneratedRouteBuilderExtensions.g.cs(88,33): error CS0246: The type or namespace name 'TUser' could not be found (are you missing a using directive or an assembly reference?)
        internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder MapPost(
            this global::Microsoft.AspNetCore.Routing.IEndpointRouteBuilder endpoints,
            [global::System.Diagnostics.CodeAnalysis.StringSyntax("Route")] string pattern,
            global::System.Action<global::Microsoft.AspNetCore.Identity.UserManager<TUser>> handler, // <---- Error!
            [global::System.Runtime.CompilerServices.CallerFilePath] string filePath = "",
            [global::System.Runtime.CompilerServices.CallerLineNumber]int lineNumber = 0)
        {
// ...
                            handler(ic.GetArgument<global::Microsoft.AspNetCore.Identity.UserManager<TUser>>(0)!);
// ... 
                        var userManager_local = httpContext.RequestServices.GetRequiredService<Microsoft.AspNetCore.Identity.UserManager<TUser>>();
// ...
                    var handler = (Func<TUser>)del;
// ...
                    var jsonTypeInfo =  (JsonTypeInfo<TUser>)serializerOptions.GetTypeInfo(typeof(TUser));
ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

halter73 commented 1 year ago

FWIW, I worked around this in MapIdentityApi<TUser> by injecting IServiceProvider instead of UserManager<TUser> the and calling GetRequiredService<UserManager<TUser>>() inside the body of the route handler.

captainsafia commented 1 year ago

I investigated what it would look like to add support for this but ultimately believe that it should be done after https://github.com/dotnet/aspnetcore/issues/48289 and https://github.com/dotnet/roslyn/pull/68218 are resolved.

A current limitation in the interceptors feature is that it does not support intercepting methods with generic type arguments. This gives us three options with how to handle this moving forward:

My preference is the 2nd, 3rd, and 1st options in that order. I'd like to move us away from a world where we ship a generator that has two different interception strategies that we need to support for the lifetime of 8.0.

halter73 commented 1 year ago

Even if https://github.com/dotnet/roslyn/pull/68218 is resolved, I do not think it will address our issue. I explain why in https://github.com/dotnet/roslyn/pull/68218/files#r1220428975, but it mostly boils down to that proposal being about intercepting methods that are already generic rather than introducing generic parameters. And even if we could change the generic arity, we'd then also need to change the handler parameter type from Delegate to something like Action<UserManager<TUser>> for type inference to work. That part in particular feels like it might be a big ask.

In any case, it doesn't look like any sort of generic interceptor support is planned for .NET 8. Is the plan for preview6 to have RDG skip over delegates with generic types in their signature and emit a warning?

captainsafia commented 1 year ago

Moving this to the backlog. For preview7, we fixed this so that static codegen does not happen for these codepaths. We'll need to fix the underlying issues in the interceptors feature for .NET 9.

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.