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

Proxies using Records derived from a base generic record broken using .NET 6 compiler #632

Closed
CesarD opened this issue Aug 13, 2022 · 7 comments · Fixed by #643
Closed
Assignees
Labels
Milestone

Comments

@CesarD
Copy link

CesarD commented Aug 13, 2022

Similar to what occurred in #601, when trying to mock (with Moq) records derived from a base generic record, an exception is thrown.

Repro code:

var mock1 = new Mock<MyDerivedRecord>();
var mock2 = new Mock<MyDerivedGenericRecord>();

var a = mock1.Object;
var b = mock2.Object;

public abstract record MyBaseRecord
{
}

public abstract record MyBaseGenericRecord<T>
{
	public T Prop { get; set; }
}

public record MyDerivedRecord : MyBaseRecord
{
}

public record MyDerivedGenericRecord : MyBaseGenericRecord<int>
{
}

Variable a holds the mocked object as expected, but when initializing b, the following is thrown:

System.ArgumentException
  HResult=0x80070057
  Message=Type to mock (MyDerivedGenericRecord) must be an interface, a delegate, or a non-sealed, non-static class.
  Source=Moq
  StackTrace:
   at Moq.CastleProxyFactory.CreateProxy(Type mockType, IInterceptor interceptor, Type[] interfaces, Object[] arguments) in C:\projects\moq4\src\Moq\Interception\CastleProxyFactory.cs:line 66
   at Moq.Mock`1.InitializeInstance() in C:\projects\moq4\src\Moq\Mock`1.cs:line 307
   at Moq.Mock`1.OnGetObject() in C:\projects\moq4\src\Moq\Mock`1.cs:line 326
   at Moq.Mock`1.get_Object() in C:\projects\moq4\src\Moq\Mock`1.cs:line 281
   at Program.<Main>$(String[] args) in C:\Users\Cesar\source\repos\ConsoleApp1\Program.cs:line 11

  This exception was originally thrown at this call stack:
    System.Reflection.Emit.TypeBuilder.CreateTypeNoLock()
    System.Reflection.Emit.TypeBuilder.CreateTypeInfo()
    Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.BuildType()
    Castle.DynamicProxy.Generators.BaseClassProxyGenerator.GenerateType(string, Castle.DynamicProxy.Generators.INamingScope)
    Castle.Core.Internal.SynchronizedDictionary<TKey, TValue>.GetOrAdd(TKey, System.Func<TKey, TValue>)
    Castle.DynamicProxy.Generators.BaseProxyGenerator.GetProxyType()
    Castle.DynamicProxy.ProxyGenerator.CreateClassProxy(System.Type, System.Type[], Castle.DynamicProxy.ProxyGenerationOptions, object[], Castle.DynamicProxy.IInterceptor[])
    Moq.CastleProxyFactory.CreateProxy(System.Type, Moq.IInterceptor, System.Type[], object[]) in CastleProxyFactory.cs

Inner Exception 1:
TypeLoadException: Return type in method 'Castle.Proxies.MyDerivedGenericRecordProxy_2.<Clone>$()' on type 'Castle.Proxies.MyDerivedGenericRecordProxy_2' from assembly 'DynamicProxyGenAssembly2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' is not compatible with base type method 'MyDerivedGenericRecord.<Clone>$()'.

Originally posted by @CesarD in #601 (comment)

@CesarD CesarD changed the title Mocking Records derived from a base generic record broken using .NET 6 compiler Proxies using Records derived from a base generic record broken using .NET 6 compiler Aug 13, 2022
@CesarD
Copy link
Author

CesarD commented Aug 27, 2022

@stakx not sure if you were aware of this one.

@stakx
Copy link
Member

stakx commented Dec 11, 2022

@CesarD, sorry for not getting back to you sooner. I'll try looking into this in the next few days.

@CesarD
Copy link
Author

CesarD commented Dec 11, 2022

Thanks so much!! 🤩🤩

@stakx stakx self-assigned this Dec 28, 2022
@stakx
Copy link
Member

stakx commented Dec 28, 2022

Finally found some time to look into this today. As suspected, the root cause for this issue is the same as in #601 (comment). We solved that other issue by adding the following logic inside MethodSignatureComparer.EqualSignatureTypes:

// This enables covariant method returns for .NET 5 and newer.
// No need to check for runtime support, since such methods are marked with a custom attribute;
// see https://github.com/dotnet/runtime/blob/main/docs/design/features/covariant-return-methods.md.
if (preserveBaseOverridesAttribute != null)
{
return (xm != null && xm.IsDefined(preserveBaseOverridesAttribute, inherit: false) && y.IsAssignableFrom(x))
|| (ym != null && ym.IsDefined(preserveBaseOverridesAttribute, inherit: false) && x.IsAssignableFrom(y));
}

But before code execution gets there, it has to go through several other checks, amongst them this one here:

else if (x.IsGenericType != y.IsGenericType)
{
return false;
}

This is why your scenario with a generic record base class fails: DynamicProxy compares the signatures of the <Clone>$ methods from MyBaseGenericRecord<T> and MyDerivedGenericRecord; and because one of those types is generic while the other is not, the above condition is met and we never get to the check for covariant returns. DynamicProxy then thinks that there are two distinct <Clone>$ methods and tries to implement them both... which is what causes Reflection Emit to throw.

@CesarD
Copy link
Author

CesarD commented Dec 29, 2022

Thanks a lot for taking care of this one!! <3

@stakx stakx added the bug label Dec 30, 2022
@stakx
Copy link
Member

stakx commented Dec 30, 2022

@CesarD, FYI, I've just pushed Moq 4.18.4 to NuGet, which now uses Castle.Core 5.1.1, which includes a fix for your issue.

@CesarD
Copy link
Author

CesarD commented Dec 30, 2022

Thank you so much!!! 👏🏼🫶🏼

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

Successfully merging a pull request may close this issue.

2 participants