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.

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

NSubstitute/Castle proxy fail to create substitute that returns a derived class from abstract that satisfies interface implicitly #730

Closed
siblount opened this issue Aug 27, 2023 · 5 comments

Comments

@siblount
Copy link

Describe the bug
NSubstitute/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

Expected behaviour
Preferibly, that it works. Otherwise, an error message about how much of an idiot I am.
Environment:

  • NSubstitute version: 5.0.0
  • NSubstitute.Analyzers version: 1.0.16
  • Platform: .NET 6.0 Windows

Additional context

This occurs for Substitute.For<>() and Substitute.ForPartsOf<>().

@siblount siblount changed the title NSubstitute/Castle fail to create substitute for concrete virtual class with derived abstract class NSubstitute/Castle fail to create substitute for concrete class derived abstract class Aug 27, 2023
@siblount siblount changed the title NSubstitute/Castle fail to create substitute for concrete class derived abstract class NSubstitute/Castle proxy fail to create substitute for concrete class derived abstract class Aug 27, 2023
@siblount
Copy link
Author

Also note, using Moq, there is no issue with this:

public void CreateTest()
{
    var a = new Mock<ConcreteIOContext>();
    a.Setup(x => x.CreateDirectoryInfo()).Returns(new XDirectoryInfo());
}

@siblount
Copy link
Author

siblount commented Aug 27, 2023

I lied. It seems to be an issue with ConcreteIOContext.CreateDirectoryInfo() with the return type of XDirectoryInfo instead of IDirectoryInfo, which is what the AbstractIOContext implies. In other words, replacing the code above with this:

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 siblount changed the title NSubstitute/Castle proxy fail to create substitute for concrete class derived abstract class NSubstitute/Castle proxy fail to create substitute that returns a derived class from abstract that satisfies interface implicitly Aug 27, 2023
@siblount
Copy link
Author

It seems that from castleproject/Core#659 (comment) that the issue has been resolved with a later version of Castle.Core supporting C# 9 covariants (which would hopefully be my issue).

So maybe update Castle.Core to a version greater than 5.1.0?

@dtchepak
Copy link
Member

Thanks @siblount . I've updated Castle.Core in the MR readying next release: #731

There's an appveyor build if you want to try it out: https://ci.appveyor.com/project/NSubstitute/nsubstitute/builds/47889349/artifacts

@siblount
Copy link
Author

Tried out the CI build and I'm glad to say that the issue no longer persists. Thanks @dtchepak, closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants