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)

Most upvoted comments

The analyzer would still be nice so you find out at design/build time before it throws at runtime.