investigate interaction between Implements and CallsBaseMethods

[blairconrad]

public interface IInterface
{
    void DoSomething();
}

public interface IOtherInterface : IInterface
{
}

public abstract class AbstractClass : IInterface
{
    public void DoSomething()
    {
        Console.WriteLine("Got here");
    }
}

[Test]
public void Implements_and_CallsBaseMethods_should_get_along()
{
    var fake = (IOtherInterface)A.Fake<AbstractClass>(builder => builder
        .Implements(typeof(IOtherInterface))
        .CallsBaseMethods());

    fake.DoSomething();
}

fakeDoSomething does not call AbstractClass.DoSomething, no matter in which order the Implements and CallsBaseMethods are used.

However,

var fake = (IOtherInterface)A.Fake<AbstractClass>(builder => builder
    .Implements(typeof(IOtherInterface)));

A.CallTo(() => fake.DoSomething()).CallsBaseMethod();
fake.DoSomething();

does call AbstractClass.DoSomething.

Omitting the (IOtherInterface) cast also causes AbstractClass.DoSomething to be executed:

 var fake = A.Fake<AbstractClass>(builder => builder
     .Implements(typeof(IOtherInterface))
     .CallsBaseMethods());

 fake.DoSomething();

Found while investigating an answer to Having an interface fake inherit from abstract while both share same interface inheritance.

[adamralph]

Interesting. This seems like a bug.

[blairconrad]

Indeed. Probably something like CallsBaseMethods looks at the methods on the type and then only adds the "delegate" instruction if they're not abstract. They're abstract on the interface, but not on the type we're faking, so we accidentally miss out. Then the call comes in, using the interface's version of the method, and it sails right on past.

Note: I have not looked. I'm just guessing.

Of course, even if this is true, I wonder if we wouldn't need to investigate method equality on the matchers. Should a call to IInterface.Method match Class.Method, assuming the latter implements the former?
I should stop speculating until some actual research has been done.

[thomaslevesque]

Indeed. Probably something like CallsBaseMethods looks at the methods on the type and then only adds the "delegate" instruction if they're not abstract.

Exactly!

public static IFakeOptions<T> CallsBaseMethods<T>(this IFakeOptions<T> options)
{
    Guard.AgainstNull(options, nameof(options));

    return options.ConfigureFake(fake => A.CallTo(fake)
                                          .Where(call => !call.Method.IsAbstract)
                                          .CallsBaseMethod());
}

Not sure how to fix it, though.

[thomaslevesque]

Here's a similar scenario that is more worrying:

public interface IInterface
{
    void DoSomething();
}

public abstract class AbstractClass
{
    public void DoSomething()
    {
        Console.WriteLine("Got here");
    }
}

public void Test()
{
    var fake = (IInterface)A.Fake<AbstractClass>(builder => builder
        .Implements(typeof(IInterface))
        .CallsBaseMethods());

    A.CallTo(() => fake.DoSomething()).CallsBaseMethod();

    fake.DoSomething();
}

This code throws:

An unhandled exception of type 'System.NotImplementedException' occurred in fakeiteasy.dll

Additional information: This is a DynamicProxy2 error: The interceptor attempted to 'Proceed' for method 'Void DoSomething()' which has no target. When calling method without target there is no implementation to 'proceed' to and it is the responsibility of the interceptor to mimic the implementation (set return value, out arguments etc)

[thomaslevesque]

Apparently IInterface.DoSomething() isn't properly mapped to AbstractClass.DoSomething(). This happens only for additional interfaces (i.e. interfaces not implemented by the faked class), so it might be a bug in Castle.Core.

[blairconrad]

Am I oversimplifying, or does this whole issue boil down to:

1. Implements acts like explicit interface implementation, and
2. explicitly-implemented interface methods are distinct from implicitly-implemented ones, and that's why we have to configure them separately

?

[thomaslevesque]

I think your analysis is correct, but to be honest, I'm no longer sure of anything about this issue...

[blairconrad]

I'm coming to believe that it's not a bug. It's just the way Implements works: like explicit interface implementation. And I don't think we'd want to change the behaviour anyhow, mostly because we now have the flexibility of configuring the interface methods or the class methods.
I don't think we should pursue a change. We could potentially document that Implements acts like explicit implementation.

[thomaslevesque]

Sounds good to me. 👍 for not fixing, since it's not broken.