Fixer/Analyzer: Hoist explicit calls to HttpContext.RequestServices.GetService to route handler parameters

[davidfowl]

We should have a fixer that rewrites calls to GetRequiredService to injecting them directly into handler methods. Code should go from this:

app.MapGet("/", (HttpContext context) =>
{
    var service = context.RequestServices.GetRequiredService<ITodoService>();
    return service.GetTodos();
});

To this:

app.MapGet("/", (ITodoService service) =>
{
    return service.GetTodos();
});

[ghost]

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.

[ghost]

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]

Background and Motivation

Many developers are unaware that request services can be resolved as parameters from minimal route handlers. This includes route handlers that were previously just RequestDelegates. Using a parameter to resolve services is more declarative and saves some typing.

Proposed Analyzer

We should have a fixer that rewrites calls to GetRequiredService to injecting them directly into handler methods.

Analyzer Behavior and Message

Code should go from this:

app.MapGet("/", (HttpContext context) =>
{
    var service = context.RequestServices.GetRequiredService<ITodoService>();
    return service.GetTodos();
});

To this:

app.MapGet("/", (ITodoService service) =>
{
    return service.GetTodos();
});

Or to reduce the likelihood of changing behavior, we could use the [FromServices] parameter.

app.MapGet("/", ([FromServices] ITodoService service) =>
{
    return service.GetTodos();
});

The message could be.

Route handlers should use parameters instead of HttpContext.RequestServices to resolve services.

Category

• Design
• Documentation
• Globalization
• Interoperability
• Maintainability
• Naming
• Performance
• Reliability
• Security
• Style
• Usage

Severity Level

• Error
• Warning
• Info
• Hidden

Usage Scenarios

// RequestDelegate?
app.MapGet("/refresh", async (context) =>
{
    var service = context.RequestServices.GetRequiredService<ITodoService>();
    await service.RefreshAsync();
});

// Optional services?
app.MapGet("/", (HttpContext context) =>
{
    var service = context.RequestServices.GetService<ITodoService>();
    return service?.GetTodos() ?? Array.Empty<Todo>();
});

// typeof(T)
app.MapGet("/", (HttpContext context) =>
{
    var service = context.RequestServices.GetRequiredService(typeof(ITodoService));
    return service.GetTodos();
});

// Other MapXXX methods
app.MapPost("/", (HttpContext context, TodoDto todo) =>
{
    var service = context.RequestServices.GetRequiredService(typeof(ITodoService));
    return service.GetTodos();
});
app.MapDelete("/{id}", (HttpContext context, int id) =>
// ...

Risks

Some people might prefer plain-old RequestDelegates and manually calling GetRequiredService. If we're not careful, false positives could occur if the service was only conditionally resolved or resolved from a non RequestServices service provider.

When the result of GetRequiredService is not stored in a variable, it could be difficult to pick a good parameter name. Perhaps this could be based on the type.

If nullability annotations are not enabled for the project, it might not be possible to correctly annotate the GetService parameter that is not required.

[ghost]

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.