Regression in 4.2.0 related to MethodSignatureComparer

[stakx]

@cbruun just reported a regression in Moq (devlooped/moq#469) that looks like it's a regression in Castle Core 4.2.0, caused by #303 (commit 0fe6129).

Repro code:

public interface IBreak
{
	void DoStuff<T>(Implementation1<T> a);
	void DoStuff<T>(Implementation2<T> a);
}

public abstract class BaseType<T> { }
public class Implementation1<T> : BaseType<T> { }
public class Implementation2<T> : BaseType<T> { }

// The following call to `CreateClassProxy` worked in 4.1.1, but throws in 4.2.0:
var generator = new ProxyGenerator();
generator.CreateClassProxy(typeof(object), new[] { typeof(IBreak) } /* , someInterceptor */);

System.TypeLoadException: Method 'DoStuff' in type 'Castle.Proxies.ObjectProxy' from assembly 'DynamicProxyGenAssembly2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' does not have an implementation.
   at System.Reflection.Emit.TypeBuilder.TermCreateClass(RuntimeModule module, Int32 tk, ObjectHandleOnStack type)
   at System.Reflection.Emit.TypeBuilder.CreateTypeNoLock()
   at System.Reflection.Emit.TypeBuilder.CreateTypeInfo()
   at Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.CreateType(TypeBuilder type)
   at Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.BuildType()
   at Castle.DynamicProxy.Generators.ClassProxyGenerator.GenerateType(String name, Type[] interfaces, INamingScope namingScope)
   at Castle.DynamicProxy.Generators.ClassProxyGenerator.<>c__DisplayClass1_0.<GenerateCode>b__0(String n, INamingScope s)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func`3 factory)
   at Castle.DynamicProxy.Generators.ClassProxyGenerator.GenerateCode(Type[] interfaces, ProxyGenerationOptions options)
   at Castle.DynamicProxy.DefaultProxyBuilder.CreateClassProxyType(Type classToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
   at Castle.DynamicProxy.ProxyGenerator.CreateClassProxy(Type classToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, Object[] constructorArguments, IInterceptor[] interceptors)
   at Castle.DynamicProxy.ProxyGenerator.CreateClassProxy(Type classToProxy, Type[] additionalInterfacesToProxy, IInterceptor[] interceptors)
   at Program.Main() in …

Probable cause of the regression:

MethodSignatureComparer.EqualSignatureTypes(x: typeof(Implementation1<T>), y: typeof(Implementation2<T>)) no longer returns false. It now returns true because it only checks the type arguments for equality, not the actual generic type definition itself.

Possible solution:

See the PR referenced below.

/cc @BitWizJason

[jonorossi]

Whoops, that was an oversight. Merged, thanks.

[BitWizJason]

Sorry for introducing the bug, at least now there is a test for it. Thanks @stakx !

[jonorossi]

@stakx would a 4.2.1 release right now with just this fix be welcomed, it sounds like this might affect quite a lot of people.

/cc @fir3pho3nixx

[stakx]

@jonorossi - Such a 4.2.1 release would be much appreciated. 👍 We've just got in another weird report (devlooped/moq#480) that might be related to the same regression (but I can't confirm yet).

I think having a Castle Core 4.2.1 and a Moq 4.7.987 (or whatever our patch version will be, they're admittedly a little strange) would give us some breathing room cause we'd then have no known regressions on the Castle side, and no version conflicts caused on Moq's side, so users would have something reasonably safe to upgrade to.

[jonorossi]

@stakx 4.2.1 now available: https://github.com/castleproject/Core/releases/tag/v4.2.1

[stakx]

Awesome, thank you very much! 🥇 Will try to get an updated Moq out the door in the next 18 hours. Moq has just been updated to 4.7.142, too.

[thomaslevesque]

Thanks for the fix; we'll soon release a new version of FakeItEasy as well. FakeItEasy/FakeItEasy#1257