castleproject / Core

Castle Core, including Castle DynamicProxy, Logging Services and DictionaryAdapter
http://www.castleproject.org/
Other
2.2k stars 467 forks source link

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

Open stakx opened 2 weeks ago

stakx commented 2 weeks ago

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 commented 2 weeks ago

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

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

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