Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add design for interceptors with nonzero arity #68218

Closed
wants to merge 1 commit into from
Closed

Add design for interceptors with nonzero arity #68218

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions docs/features/interceptors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,13 @@ Interception can only occur for calls to ordinary member methods--not constructo

### Arity

Interceptors cannot have type parameters or be declared in generic types at any level of nesting.
Interceptors cannot be declared in generic types at any level of nesting.

This limitation prevents interceptors from matching the signature of an interceptable call in cases where the interceptable call uses type parameters which are not in scope at the interceptor declaration. We can consider adjusting the rules to alleviate this limitation if compelling scenarios arise for it in the future.
An interceptor method must either have arity 0 or have arity equal to the interceptable method.

When an interceptor has nonzero arity, the method type arguments passed to the original method are passed to the interceptor. Type parameter constraints and [signature matching](#signature-matching) are enforced on the resulting constructed interceptor method.

One important scenario enabled by this is when the type arguments to the original method are type parameters which are not in scope at the interceptor declaration.

```cs
using System.Runtime.CompilerServices;
Expand All @@ -154,14 +158,34 @@ static class Program
{
public static void M<T2>(T2 t)
{
C.InterceptableMethod(t);
C.InterceptableMethod(t); // intercepts with 'Interceptor1<T2>(T2)'
}
}

static class D
{
[InterceptsLocation("Program.cs", 13, 11)]
public static void Interceptor1(object s) => throw null!;
public static void Interceptor1<T3>(T3 t3) => throw null!;
}
```

If an interceptable method signature uses type parameters from the containing type (in parameters and returns, including `this`), it won't be possible to declare an interceptor for it in another type. This is because there's no position where the original containing type's type parameters can be substituted in to the interceptor. We could revisit this limitation if needed, perhaps by reviewing the arity matching requirements proposed for *implicit and explicit extensions* as a starting point.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to resolve this issue to fix dotnet/aspnetcore#47338. The methods we're trying to intercept in ASP.NET Core were never generic to begin with. We want the interceptor method to capture the generic parameter from an outer scope that was never passed to the original intercepted method, so the arity will be different.

Take the examples I originally posted in the ASP.NET Core issue:

public static void TestGenericParam<TUser>(IEndpointRouteBuilder app) where TUser : class
{
    app.MapPost("/test-param", ([FromServices] UserManager<TUser> userManager) => { });
}

public static void TestGenericResponse<TUser>(IEndpointRouteBuilder app) where TUser : new()
{
    app.MapPost("/test-return", () => new TUser());
}

The original MapPost has a generic arity of zero. The goal was to be able to define a MapPost<TUser>(IEndpointRouteBuilder app, string pattern, Delegate handler) so we could then use TUser in the generated code to resolve the arguments and serialize the response correctly.

Of course, for TUser to be inferred at these call sites, MapPost<TUser> would need to take an Action<UserManager<TUser>> or Func<TUser> respectively, rather than a Delegate like it does in the current PR to make ASP.NET Core use this feature at dotnet/aspnetcore#48555.

Looking at the signature matching part of this document, it appears arguments have to match the original method arguments exactly rather than take a more specific type. It'd be nice if we could relax that if we could statically prove that the call site will always use a more specific type that the interceptor accepts.

Here's an example that demonstrates the need for the interceptor to change the parameter type from Delegate to Func<T2> for type inference to work.

class C
{
    [Interceptable]
    public static void InterceptableMethod(Delegate handler) => throw null!;
}

static class Program
{
    public static void M<T1>(T1 t)
    {
        C.InterceptableMethod(() => t); // intercepts with 'Interceptor<T1>(Func<T1>)'
    }
}

static class D
{
    [InterceptsLocation("Program.cs", 13, 11)]
    public static void Interceptor<T2>(Func<T2> handler) => throw null!;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is good information and it's a tricky scenario. Is it safe to assume that we can't know the full set of possible type arguments to TestGenericResponse<TUser> in advance? (letting us possibly test and branch on the type of the return value.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it's not safe to assume we can discover the full set of type arguments at compile time. I discovered this issue working on MapIdentityApi<TUser>() where the TUser is provided by the consumer of the library.


```cs
class C<T>
{
static void Interceptable(T t) { }
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved

void M(T t)
{
Interceptable(t);
}
}

class D
{
[InterceptsLocation("Program.cs", 7, 9)]
public static void Interceptor(T t) { } // error: we can't refer to 'T' in 'C<T>' here.
}
```

Expand Down