castleproject / Core

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

Castle.DynamicProxy.DynamicProxyException: Duplicate element when interface contains multiple similar functions (case-insensitive name comparison) #691

Open dark-dark-darkness opened 1 week ago

dark-dark-darkness commented 1 week ago

Description

When mocking an interface that contains multiple methods with similar signatures using Moq, an exception is thrown during proxy generation. This issue arises because the proxy generator performs a case-insensitive comparison when determining if methods are duplicates, leading to a conflict even when method names differ only by case.

Moq version: 4.20.72 Castle.Core: 5.1.1 .Net version: net48;net8.0

Program.cs

using Moq;

_ = new Mock<IDialogService>().Object;

public interface IDialogService
{
    bool? YESNoCancel(string? message, Exception? exception);
    bool? YesNOCancel(string? message, Exception? exception);
    bool? YesNoCANCEL(string? message, Exception? exception);
}

Exception

image

The exception message is as follows:

Castle.DynamicProxy.DynamicProxyException: Duplicate element: Castle.DynamicProxy.Generators.MetaMethod
   at Castle.DynamicProxy.Generators.MetaTypeElementCollection`1.Add(TElement item)
   at Castle.DynamicProxy.Generators.MetaType.AddMethod(MetaMethod method)
   at Castle.DynamicProxy.Contributors.CompositeTypeContributor.MembersCollectorSink.Add(MetaMethod method)
   at Castle.DynamicProxy.Contributors.MembersCollector.<CollectMembersToProxy>g__AddMethod|7_5(MethodInfo method, Boolean isStandalone, <>c__DisplayClass7_0&)
   at Castle.DynamicProxy.Contributors.MembersCollector.<CollectMembersToProxy>g__CollectMethods|7_2(<>c__DisplayClass7_0&)
   at Castle.DynamicProxy.Contributors.MembersCollector.CollectMembersToProxy(IProxyGenerationHook hook, IMembersCollectorSink sink)
   at Castle.DynamicProxy.Contributors.CompositeTypeContributor.CollectElementsToProxy(IProxyGenerationHook hook, MetaType model)
   at Castle.DynamicProxy.Generators.BaseInterfaceProxyGenerator.GenerateType(String typeName, INamingScope namingScope)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.<>c__DisplayClass13_0.<GetProxyType>b__0(CacheKey cacheKey)
   at Castle.Core.Internal.SynchronizedDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.GetProxyType()
   at Castle.DynamicProxy.DefaultProxyBuilder.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
   at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
   at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, IInterceptor[] interceptors)
   at Moq.CastleProxyFactory.CreateProxy(Type mockType, IInterceptor interceptor, Type[] interfaces, Object[] arguments) in /_/src/Moq/Interception/CastleProxyFactory.cs:line 50
   at Moq.Mock`1.InitializeInstance() in /_/src/Moq/Mock`1.cs:line 309
   at Moq.Mock`1.OnGetObject() in /_/src/Moq/Mock`1.cs:line 323
   at Moq.Mock.get_Object() in /_/src/Moq/Mock.cs:line 181
   at Moq.Mock`1.get_Object() in /_/src/Moq/Mock`1.cs:line 281
   at Program.<Main>$(String[] args) in D:\repos\ConsoleApp2\ConsoleApp2\Program.cs:line 3
stakx commented 1 week ago

Thanks for reporting this, @dark-dark-darkness.

This is caused by the case-insensitive method name comparison here:

https://github.com/castleproject/Core/blob/8411ea66539c2ee0860e262af468f74c99f918db/src/Castle.Core/DynamicProxy/Generators/MetaMethod.cs#L72

which was introduced a long time ago, in https://github.com/castleproject/Core/commit/d1d62269fe4cad4fec24fe69b0fe79c4149569fa.

I'm not sure why StringComparer.OrdinalIgnoreCase was chosen over StringComparer.Ordinal – perhaps in consideration of case-insensitive languages like VB.NET or the CLS (Common Language Specification)?

In any case, I tried changing the above line of code to use StringComparer.Ordinal instead, which broke no tests and fixed the above scenario. So we could probably classify this as a bug, and go ahead and make the method name comparisons case-insensitive.

Any thoughts?