[Analyzer] RequestDelegate return value detection

Background and Motivation

RequestDelegate can be used with methods/lambdas that return a value that is then ignored.

Delegate signature:

aspnetcore/src/Http/Http.Abstractions/src/RequestDelegate.cs

Lines 6 to 11 in 86c7e01

    
           /// <summary> 
        
           /// A function that can process an HTTP request. 
        
           /// </summary> 
        
           /// <param name="context">The <see cref="HttpContext"/> for the request.</param> 
        
           /// <returns>A task that represents the completion of request processing.</returns> 
        
           public delegate Task RequestDelegate(HttpContext context);

Because Task is the base type of Task<T>, generic variance means it's possible to return Task<T> from a method that's used with RequestDelegate. The caller of the delegate never sees the value and it is ignored.

Example: #39956

Proposed API

An analyzer that detects using a method or lambda that returns Task<T> with RequestDelegate and warns the user.

Usage Examples

Task<string> HelloWorld(HttpContext c) => Task.FromResult("Hello " + c.Request.RouteValues["name"]);

endpoints.MapGet("/", HelloWorld); // <- detect bad method

endpoints.MapGet("/", (HttpContext c) => Task.FromResult("Hello " + c.Request.RouteValues["name"])); // <- detect bad lambda

Note that an async lambda can't get in this situation and doesn't need to be checked. The compiler detects a return value and prevents the lambda being converted to RequestDelegate:

RequestDelegate d = new RequestDelegate(async Task<string> (HttpContext context) =>
{
    await Task.Yield();
    return "hello world"; // <- compiler error
});

Alternative Designs

Risks

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

What does a "bad method" mean? We fixed this in .NET 7.

A RequestDelegate method that returns Task<T>.

That works now. This is why the issue is closed.

The change was reverted later - #42519

So is this about trimming or something else? The change wasn't reverted, just the trimming attributes.

API Review Notes:

This especially important since this isn't caught at runtime and is likely misusage.

Category: Usage
Severity: Warning

	/// <summary>
	/// A function that can process an HTTP request.
	/// </summary>
	/// <param name="context">The <see cref="HttpContext"/> for the request.</param>
	/// <returns>A task that represents the completion of request processing.</returns>
	public delegate Task RequestDelegate(HttpContext context);