Remove RequiresUnreferencedCode from low-level endpoint map methods

[JamesNK]

• Remove RequiresUnreferencedCode from low-level endpoint map methods. These methods should be reflection-free. No longer support checking the request delegate method at runtime, inspecting its return type, and generating a wrapper.
• slnf updated because of missing projects required by MinimalSample.csproj

@halter73 You made this change in #42195. Are you ok with reverting it? Does it still make sense to keep the test that I had to modify after reverting the change?

[halter73]

This makes sense to me.

[JamesNK]

This is the bug the change originally fixed: #39956

app.Map("/Fails1", Fails1);
app.Run();
static async Task<string> Fails1(HttpContext context)
{
    var response = await Task.FromResult("response");
    return response;
}

So HttpContext argument + Task<T> prefers the RequestDelegate overload over Delegate.

The only non-runtime fix I can imagine is adding an explicit overload for it, i.e. MapGet<T>(this IEndpointRouteBuilder, Func<HttpContext, Task<T>> delegate), which then calls the untyped delegate logic. The compiler would then prefer it over the non-reflective RequestDelegate overload.

Alternatively, we leave the runtime reflection as it is, but suppress it. The downside is someone who happens to use HttpContext argument + Task<T> in their app won't get a warning when trimming is enabled.

[halter73]

Alternatively, we leave the runtime reflection as it is, but suppress it. The downside is someone who happens to use HttpContext argument + Task in their app won't get a warning when trimming is enabled.

What happens if someone unknowingly because of the suppression thinks it's safe to trim their app when in fact they're relying on RDFs return value serialization? Could fields on the returned values be stripped? That seems worse than just ignoring the return value altogether which is why I said this change to just to remove this functionality from the RequestDelegate overload made sense to me.

It's certainly unfortunate that trimming stops us from just making return value serialization work for people who do not trim and accidentally use the RequestDelegate overload, but I still think that's not as bad as suppressing a warning that really shouldn't be suppressed. It's easy enough to manually cast to Delegate if you need to.

[halter73]

Alternatively, we leave the runtime reflection as it is, but suppress it. The downside is someone who happens to use HttpContext argument + Task in their app won't get a warning when trimming is enabled.

What happens if someone unknowingly because of the suppression thinks it's safe to trim their app when in fact they're relying on RDFs return value serialization? Could fields on the returned values be stripped? That seems worse than just ignoring the return value altogether which is why I said this change to just to remove this functionality from the RequestDelegate overload made sense to me.

It's certainly unfortunate that trimming stops us from just making return value serialization work for people who do not trim and accidentally use the RequestDelegate overload, but I still think that's not as bad as suppressing a warning that really shouldn't be suppressed. It's easy enough to manually cast to Delegate if you need to.

[JamesNK]

Here is an idea: write an analyzer that detects using a method that returns Task<T> with RequestDelegate.

Issue: #42523

[davidfowl]

This is a good change