`invocation.MethodInvocationTarget` throws `ArgumentNullException` for default interface method

[stakx]

Repro code

var generator = new ProxyGenerator();
var foo = generator.CreateInterfaceProxyWithoutTarget<IFoo>(new InspectInvocation());
foo.Bar();

public interface IFoo
{
    void Bar() { }
}

public class InspectInvocation : IInterceptor
{
    public void Intercept(IInvocation invocation)
    {
        Debug.Assert(invocation.InvocationTarget == null || invocation.MethodInvocationTarget != null);  // will throw
    }
}

Expected outcome

Unless I misunderstand the meaning of IInvocation.InvocationTarget and IInvocation.MethodInvocationTarget, I would expect the latter to point to a method of the former's type, if the former (IInvocation.InvocationTarget) is non-null.

Actual outcome

MethodInvocationTarget will throw an ArgumentNullException due to the invocation's targetType not being set.

Affected version(s)

Castle.Core starting at tag v5.2.0

/cc @dtchepak @zvirja @alexandrnikitin – this will likely matter for NSubstitute (in https://github.com/nsubstitute/NSubstitute/blob/4d258a28aba054ea18785d36b4bdd83da023aefb/src/NSubstitute/Proxies/CastleDynamicProxy/CastleInvocationMapper.cs#L11-L16) once we've published version 5.2.0 and you upgrade to that version.

[stakx]

If I understand things correctly, then generally speaking the "target" is whatever one can .Proceed() to:

• For composition-based proxies, a target object can be supplied explicitly.
• For inheritance-based proxies, the target can be either missing (i. e. null) or the intercepted method's implementation inherited from the proxied type.

If the above assumptions are correct, then for an IInvocation representing an intercepted default interface method...:

• invocation.TargetType should return the interface type where the default implementation resides.
• invocation.InvocationTarget should return the same as invocation.Proxy.
• invocation.MethodInvocationTarget should return the MethodInfo of the method with the default implementation... so possibly the same as invocation.Method?

@jonorossi, sorry to bother you again, but are my assumptions and conclusions correct, or am I missing some finer distinctions?

[304NotModified]

FYI I tested this with NSubstitute 5.3.0 and Castle.Core 5.1.1 and 5.2.1

Tested with the following code:

    [Fact]
    public void TestDefaultInterfaceImpl1()
    {
        // Arrange
        var for1 = Substitute.For<IMyInterface>();

        // Act
        var message = for1.GetText();

        // Assert
        message.Should().Be("abc");
    }

    [Fact]
    public void TestDefaultInterfaceImpl2()
    {
        // Arrange
        var for1 = Substitute.For<IMyInterface>();
        for1.GetText().Returns("abcd");

        // Act
        var message = for1.GetText();

        // Assert
        message.Should().Be("abcd");
    }

    public interface IMyInterface
    {
        string GetText()
        {
            return "abc";
        }
    }

Castle.Core 5.1.1:

• Test TestDefaultInterfaceImpl1 assert fails: Expected message to be "abc" with a length of 3, but "" has a length of 0, differs near "" (index 0).
• Test TestDefaultInterfaceImpl2 succeeds

Castle.Core 5.2.1

Both tests fail with:

System.ArgumentNullException
Value cannot be null. (Parameter 'type')
at Castle.DynamicProxy.Internal.InvocationHelper.GetMethodOnType(Type type, MethodInfo proxiedMethod)
at Castle.DynamicProxy.Internal.InheritanceInvocation.get_MethodInvocationTarget()
at NSubstitute.Proxies.CastleDynamicProxy.CastleInvocationMapper.Map(IInvocation castleInvocation)
at NSubstitute.Proxies.CastleDynamicProxy.CastleForwardingInterceptor.Intercept(IInvocation invocation)
at Castle.DynamicProxy.AbstractInvocation.Proceed()
at NSubstitute.Proxies.CastleDynamicProxy.ProxyIdInterceptor.Intercept(IInvocation invocation)
at Castle.DynamicProxy.AbstractInvocation.Proceed()
at Castle.Proxies.ObjectProxy.GetText()

Remarks

If we could do something in NSubstitute to fix this, please let me know.

If the above assumptions are correct, then for an IInvocation representing an intercepted default interface method...:

• invocation.TargetType should return the interface type where the default implementation resides.
• invocation.InvocationTarget should return the same as invocation.Proxy.
• invocation.MethodInvocationTarget should return the MethodInfo of the method with the default implementation... so possibly the same as invocation.Method?

I'm not sure as I don't know the code base that well, but it sounds correct to me

[stakx]

@304NotModified, thanks for adding some more test cases to this issue, plus thinking about the meaning of those invocation properties. I'll see what I can do... but it might take a while (hopefully not another 6 months though...).

[BecaLParker]

I'm watching for a resolution to this issue as it's blocking us from taking the latest update - thanks :)