castleproject / Core

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

Proxy fail to create for create substitute that returns a derived class from abstract that satisfies interface implicitly #659

Closed siblount closed 1 year ago

siblount commented 1 year ago

Describe the bug Castle proxy fails to create a substitute for a concrete class that derives from an abstract class and virtual members.

To Reproduce Here's an example of what I am talking about: Setup classes

namespace TestNSubstituteBS
{
    public abstract class AbstractIOContext
    {
        public abstract IDirectoryInfo CreateDirectoryInfo();
    }
}
namespace TestNSubstituteBS
{
    public class ConcreteIOContext : AbstractIOContext
    {
        public override XDirectoryInfo CreateDirectoryInfo() => new XDirectoryInfo();
    }
}
namespace TestNSubstituteBS
{
    public class XDirectoryInfo : IDirectoryInfo { }
}

namespace TestNSubstituteBS
{
    public interface IDirectoryInfo { }
}

Test code:

namespace TestNSubstituteBS.Tests
{
    [TestClass()]
    public class AbstractFactoryTests
    {
        [TestMethod]
        public void CreateTest()
        {
            var a = Substitute.For<ConcreteIOContext>(); // <--------------- fails here
            a.CreateDirectoryInfo().Returns(new XDirectoryInfo());
        }
    }
}

Running the above returns this error message...

Test method TestNSubstituteBS.Tests.AbstractFactoryTests.CreateTest threw exception: 
System.TypeLoadException: Return type in method 'Castle.Proxies.ConcreteIOContextProxy.CreateDirectoryInfo()' on type 'Castle.Proxies.ConcreteIOContextProxy' from assembly 'DynamicProxyGenAssembly2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' is not compatible with base type method 'TestNSubstituteBS.ConcreteIOContext.CreateDirectoryInfo()'.

and stack trace...

  Stack Trace: 
TypeBuilder.CreateTypeNoLock()
TypeBuilder.CreateTypeInfo()
AbstractTypeEmitter.BuildType()
BaseClassProxyGenerator.GenerateType(String name, INamingScope namingScope)
<>c__DisplayClass13_0.<GetProxyType>b__0(CacheKey cacheKey)
SynchronizedDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
BaseProxyGenerator.GetProxyType()
DefaultProxyBuilder.CreateClassProxyType(Type classToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
ProxyGenerator.CreateClassProxyType(Type classToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
ProxyGenerator.CreateClassProxy(Type classToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, Object[] constructorArguments, IInterceptor[] interceptors)
CastleDynamicProxyFactory.CreateProxyUsingCastleProxyGenerator(Type typeToProxy, Type[] additionalInterfaces, Object[] constructorArguments, IInterceptor[] interceptors, ProxyGenerationOptions proxyGenerationOptions)
CastleDynamicProxyFactory.GenerateTypeProxy(ICallRouter callRouter, Type typeToProxy, Type[] additionalInterfaces, Object[] constructorArguments)
CastleDynamicProxyFactory.GenerateProxy(ICallRouter callRouter, Type typeToProxy, Type[] additionalInterfaces, Object[] constructorArguments)
SubstituteFactory.Create(Type[] typesToProxy, Object[] constructorArguments, Boolean callBaseByDefault)
SubstituteFactory.Create(Type[] typesToProxy, Object[] constructorArguments)
Substitute.For(Type[] typesToProxy, Object[] constructorArguments)
Substitute.For[T](Object[] constructorArguments)
AbstractFactoryTests.CreateTest() line 18

As it turns out, it seems that there is an issue with the proxy when the ConcreteIOContext has the return type XDirectoryInfo, which does implement IDirectoryInfo, but not stated explicitly. If stated explicitly, the issue goes away.

namespace TestNSubstituteBS
{
    public class ConcreteIOContext : AbstractIOContext
    {
        // public override XDirectoryInfo CreateDirectoryInfo() => new XDirectoryInfo(); <-- does not work
        public override IDirectoryInfo CreateDirectoryInfo() => new XDirectoryInfo(); // <--- this does work
    }
}

The code above now makes it work without any issue.

siblount commented 1 year ago

I don't know how any of this works, but there is also the possibility that whatever was not configured correctly, I also made an issue on NSub's repo: https://github.com/nsubstitute/NSubstitute/issues/730

stakx commented 1 year ago

This issue seems to be about C# 9 covariant return types, which DynamicProxy should support since version 5.1.0. (See #619.)

Can you please check which version of NSubstitute (and Castle.Core) you are using? Also, which version of .NET are you running your code on?

siblount commented 1 year ago

Ah...It would seem that the latest NSubstitute release is using Castle.Core version 5.0.0.* according to the csproj.

I am also using .NET 6.0 Windows.

stakx commented 1 year ago

OK, in that case there's not much to be done on DynamicProxy's side. NSubstitute should probably upgrade their Castle.Core dependency... I've seen in the linked issue that they've already done so.

I'm going to close this issue as it'll likely be resolved after the upgrade. If not, let us know again here and we'll reopen.

siblount commented 1 year ago

Yup, verified that the problem is solved by updating Castle.Core version. Thanks @stakx !