aspnetcore: Create an analyzer that warns when a parameter on a route handler delegate has a BindAsync or TryParse method that doesn't comply with RequestDelegateFactory requirements
Background and Motivation
NOTE: This issue has been reworked, @DamianEdwards initial issue is included below the standard template.
When using the ASP.NET minimal APIs the delegate that is mapped to the route can contain various custom types defined by the developer. In order to create these objects prior to calling the delegate the framework relies on the types in question implementing either BindAsync
method or the TryParse
method. Unfortunately, developers often define these methods incorrectly.
In .NET 6 a runtime behavior was introduced that resulted in an exception being thrown if these types were listed in the delegate, but their types did not implement these methods. In .NET 8 we want to shift this error left and use an analyzer to tell developers that these methods are missing. An analyzer could help alert one to the fact that they have a TryParse
or BindAsync
method defined on a type used as a route handler parameter that doesn’t meet the requirements for discovery/invocation by RequestDelegateFactory
.
Proposed Analyzer
Analyzer Behavior and Message
The analyzer should highlight the type and name declarations of the parameter, i.e. the Todo todo
in the example below. If possible, it could also highlight the method name declaration on the type itself, i.e. the BindAsync
in the example below, when it’s determined that the type is used on a parameter of a route handler.
Category
- Design
- Documentation
- Globalization
- Interoperability
- Maintainability
- Naming
- Performance
- Reliability
- Security
- Style
- Usage
Severity Level
- Error
- Warning
- Info
- Hidden
Usage Scenarios
using System.Text.Json;
var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();
// ERROR! On both arguments because they don't implement TryParse and BindAsync respectively.
app.MapPost("/organizations/{organization}/contacts", (Organization organization, Contact contact) =>
{
return Results.Ok(contact);
});
app.Run();
// ERROR! Need so implement static bool TryParse(string value, IFormatProvider format, out Organization organization)
public class Organization
{
public static bool TryParse(string value, IFormatProvider format, out Organization organization)
{
organization = new Organization();
return true;
}
}
// ERROR! Need so implement static ValueTask<Contact> BindAsync(HttpContext context)
public class Contact
{
public async ValueTask<Contact> BindAsync(HttpContext context)
{
var contact = new Contact();
return contact;
}
}
Risks
Original Issue
It can be tricky to get the signature for TryParse
and BindAsync
correct as they’re not enforced with any language features (until abstract static members on interfaces are in non-preview).
An analyzer could help alert one to the fact that they have a TryParse
or BindAsync
method defined on a type used as a route handler parameter that doesn’t meet the requirements for discovery/invocation by RequestDelegateFactory
.
The analyzer should highlight the type and name declarations of the parameter, i.e. the Todo todo
in the example below. If possible it could also highlight the method name declaration on the type itself, i.e. the BindAsync
in the example below, when it’s determined that the type is used on a parameter of a route handler.
Example
app.MapPost("/todo", (Todo todo) =>
{
// Save the todo...
return Results.Created($"/todo/{todo.Id}", todo);
});
class Todo
{
public int? Id { get; set; }
public string? Title { get; set; }
public IList<string> Tags { get; set; } = new List<string>();
// This won't bind as it isn't static
public async ValueTask<Todo> BindAsync(HttpContext context)
{
if (int.TryParse(context.Request.Query["id"], out var id) Id = id;
Title = context.Request.Query["title"];
var tags = context.Request.Query["tags"];
return ValueTask.FromResult(this);
}
}
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 15 (15 by maintainers)
https://github.com/dotnet/aspnetcore/issues/42286 and https://github.com/dotnet/aspnetcore/issues/43546
The analyzer would still be nice so you find out at design/build time before it throws at runtime.