Merge pull request castleproject#580 from stakx/generic-parameter-name-mismatch

stakx · web-flow · commit d88e94447150 · 2021-03-25T22:21:08.000+01:00
Match generic parameters by position, not by name
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ Enhancements:
 - Significant performance improvements with proxy type generation for interface proxies without target. (Up until now, DynamicProxy generated a separate `IInvocation` implementation type for every single proxied method &ndash; it is now able to reuse a single predefined type in many cases, thereby reducing the total amount of dynamic type generation.) (@stakx, #573)
 
 Bugfixes:
+- Generic method with differently named generic arguments to parent throws `KeyNotFoundException` (@stakx, #106)
 - Proxying certain `[Serializable]` classes produces proxy types that fail PEVerify test (@stakx, #367)
 - `private protected` methods are not intercepted (@CrispyDrone, #535)
 - `System.UIntPtr` unsupported (@stakx, #546)
diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/GenericMethodsProxyTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/GenericMethodsProxyTestCase.cs
@@ -92,5 +92,32 @@ public void ProxyInterfaceWithGenericMethodWithTwoGenericParametersWhereOneIsBas
 		{
 			generator.CreateInterfaceProxyWithoutTarget<GenericMethodWhereOneGenParamInheritsTheOther>();
 		}
+
+		[Test]
+		[TestCase(typeof(Test))]
+		[TestCase(typeof(TestVirtual))]
+		public void GenericMethodDifferentlyNamedGenericArguments(Type classType)
+		{
+			generator.CreateClassProxy(classType, new[] { typeof(ITest) });
+		}
+
+		public interface ITest
+		{
+			void Hi<T>();
+		}
+
+		public class Test : ITest
+		{
+			public void Hi<U>()
+			{
+			}
+		}
+
+		public class TestVirtual : ITest
+		{
+			public virtual void Hi<U>()
+			{
+			}
+		}
 	}
 }
diff --git a/src/Castle.Core/DynamicProxy/Generators/Emitters/AbstractTypeEmitter.cs b/src/Castle.Core/DynamicProxy/Generators/Emitters/AbstractTypeEmitter.cs
@@ -36,7 +36,6 @@ internal abstract class AbstractTypeEmitter
 
 		private readonly List<MethodEmitter> methods;
 
-		private readonly Dictionary<string, GenericTypeParameterBuilder> name2GenericType;
 		private readonly List<NestedClassEmitter> nested;
 		private readonly List<PropertyEmitter> properties;
 		private readonly TypeBuilder typebuilder;
@@ -51,7 +50,6 @@ protected AbstractTypeEmitter(TypeBuilder typeBuilder)
 			constructors = new List<ConstructorEmitter>();
 			properties = new List<PropertyEmitter>();
 			events = new List<EventEmitter>();
-			name2GenericType = new Dictionary<string, GenericTypeParameterBuilder>();
 		}
 
 		public Type BaseType
@@ -113,7 +111,7 @@ public void CopyGenericParametersFromMethod(MethodInfo methodToCopyGenericsFrom)
 				throw new InvalidOperationException("Cannot invoke me twice");
 			}
 
-			SetGenericTypeParameters(GenericUtil.CopyGenericArguments(methodToCopyGenericsFrom, typebuilder, name2GenericType));
+			SetGenericTypeParameters(GenericUtil.CopyGenericArguments(methodToCopyGenericsFrom, typebuilder));
 		}
 
 		public ConstructorEmitter CreateConstructor(params ArgumentReference[] arguments)
@@ -266,42 +264,72 @@ public FieldReference GetField(string name)
 			return value;
 		}
 
-		public Type GetGenericArgument(string genericArgumentName)
+		public Type GetClosedParameterType(Type parameter)
 		{
-			if (name2GenericType.TryGetValue(genericArgumentName, out var genericTypeParameterBuilder))
-				return genericTypeParameterBuilder;
+			if (parameter.IsGenericType)
+			{
+				// ECMA-335 section II.9.4: "The CLI does not support partial instantiation
+				// of generic types. And generic types shall not appear uninstantiated any-
+				// where in metadata signature blobs." (And parameters are defined there!)
+				Debug.Assert(parameter.IsGenericTypeDefinition == false);
 
-			return null;
-		}
+				var arguments = parameter.GetGenericArguments();
+				if (CloseGenericParametersIfAny(arguments))
+				{
+					return parameter.GetGenericTypeDefinition().MakeGenericType(arguments);
+				}
+			}
 
-		public Type[] GetGenericArgumentsFor(Type genericType)
-		{
-			var types = new List<Type>();
+			if (parameter.IsGenericParameter)
+			{
+				return GetGenericArgument(parameter.GenericParameterPosition);
+			}
 
-			foreach (var genType in genericType.GetGenericArguments())
+			if (parameter.IsArray)
 			{
-				if (genType.IsGenericParameter)
-				{
-					types.Add(name2GenericType[genType.Name]);
-				}
-				else
+				var elementType = GetClosedParameterType(parameter.GetElementType());
+				int rank = parameter.GetArrayRank();
+				return rank == 1
+					? elementType.MakeArrayType()
+					: elementType.MakeArrayType(rank);
+			}
+
+			if (parameter.IsByRef)
+			{
+				var elementType = GetClosedParameterType(parameter.GetElementType());
+				return elementType.MakeByRefType();
+			}
+
+			return parameter;
+
+			bool CloseGenericParametersIfAny(Type[] arguments)
+			{
+				var hasAnyGenericParameters = false;
+				for (var i = 0; i < arguments.Length; i++)
 				{
-					types.Add(genType);
+					var newType = GetClosedParameterType(arguments[i]);
+					if (newType != null && !ReferenceEquals(newType, arguments[i]))
+					{
+						arguments[i] = newType;
+						hasAnyGenericParameters = true;
+					}
 				}
+				return hasAnyGenericParameters;
 			}
+		}
+
+		public Type GetGenericArgument(int position)
+		{
+			Debug.Assert(0 <= position && position < genericTypeParams.Length);
 
-			return types.ToArray();
+			return genericTypeParams[position];
 		}
 
 		public Type[] GetGenericArgumentsFor(MethodInfo genericMethod)
 		{
-			var types = new List<Type>();
-			foreach (var genType in genericMethod.GetGenericArguments())
-			{
-				types.Add(name2GenericType[genType.Name]);
-			}
+			Debug.Assert(genericMethod.GetGenericArguments().Length == genericTypeParams.Length);
 
-			return types.ToArray();
+			return genericTypeParams;
 		}
 
 		public void SetGenericTypeParameters(GenericTypeParameterBuilder[] genericTypeParameterBuilders)
diff --git a/src/Castle.Core/DynamicProxy/Generators/Emitters/GenericUtil.cs b/src/Castle.Core/DynamicProxy/Generators/Emitters/GenericUtil.cs
@@ -15,7 +15,6 @@
 namespace Castle.DynamicProxy.Generators.Emitters
 {
 	using System;
-	using System.Collections.Generic;
 	using System.Diagnostics;
 	using System.Reflection;
 	using System.Reflection.Emit;
@@ -29,95 +28,16 @@ internal class GenericUtil
 	{
 		public static GenericTypeParameterBuilder[] CopyGenericArguments(
 			MethodInfo methodToCopyGenericsFrom,
-			TypeBuilder builder,
-			Dictionary<string, GenericTypeParameterBuilder> name2GenericType)
+			TypeBuilder builder)
 		{
-			return
-				CopyGenericArguments(methodToCopyGenericsFrom, name2GenericType,
-				                     builder.DefineGenericParameters);
+			return CopyGenericArguments(methodToCopyGenericsFrom, builder.DefineGenericParameters);
 		}
 
 		public static GenericTypeParameterBuilder[] CopyGenericArguments(
 			MethodInfo methodToCopyGenericsFrom,
-			MethodBuilder builder,
-			Dictionary<string, GenericTypeParameterBuilder> name2GenericType)
+			MethodBuilder builder)
 		{
-			return
-				CopyGenericArguments(methodToCopyGenericsFrom, name2GenericType,
-				                     builder.DefineGenericParameters);
-		}
-
-		public static Type ExtractCorrectType(Type paramType, Dictionary<string, GenericTypeParameterBuilder> name2GenericType)
-		{
-			if (paramType.IsArray)
-			{
-				var rank = paramType.GetArrayRank();
-
-				var underlyingType = paramType.GetElementType();
-
-				if (underlyingType.IsGenericParameter)
-				{
-					GenericTypeParameterBuilder genericType;
-					if (name2GenericType.TryGetValue(underlyingType.Name, out genericType) == false)
-					{
-						return paramType;
-					}
-
-					if (rank == 1)
-					{
-						return genericType.MakeArrayType();
-					}
-					return genericType.MakeArrayType(rank);
-				}
-				if (rank == 1)
-				{
-					return underlyingType.MakeArrayType();
-				}
-				return underlyingType.MakeArrayType(rank);
-			}
-
-			if (paramType.IsGenericParameter)
-			{
-				GenericTypeParameterBuilder value;
-				if (name2GenericType.TryGetValue(paramType.Name, out value))
-				{
-					return value;
-				}
-			}
-
-			return paramType;
-		}
-
-		public static Type[] ExtractParametersTypes(
-			ParameterInfo[] baseMethodParameters,
-			Dictionary<string, GenericTypeParameterBuilder> name2GenericType)
-		{
-			var newParameters = new Type[baseMethodParameters.Length];
-
-			for (var i = 0; i < baseMethodParameters.Length; i++)
-			{
-				var param = baseMethodParameters[i];
-				var paramType = param.ParameterType;
-
-				newParameters[i] = ExtractCorrectType(paramType, name2GenericType);
-			}
-
-			return newParameters;
-		}
-
-		public static Dictionary<string, GenericTypeParameterBuilder> GetGenericArgumentsMap(AbstractTypeEmitter parentEmitter)
-		{
-			if (parentEmitter.GenericTypeParams == null || parentEmitter.GenericTypeParams.Length == 0)
-			{
-				return new Dictionary<string, GenericTypeParameterBuilder>(0);
-			}
-
-			var name2GenericType = new Dictionary<string, GenericTypeParameterBuilder>(parentEmitter.GenericTypeParams.Length);
-			foreach (var genType in parentEmitter.GenericTypeParams)
-			{
-				name2GenericType.Add(genType.Name, genType);
-			}
-			return name2GenericType;
+			return CopyGenericArguments(methodToCopyGenericsFrom, builder.DefineGenericParameters);
 		}
 
 		private static Type AdjustConstraintToNewGenericParameters(
@@ -184,7 +104,6 @@ private static Type[] AdjustGenericConstraints(MethodInfo methodToCopyGenericsFr
 
 		private static GenericTypeParameterBuilder[] CopyGenericArguments(
 			MethodInfo methodToCopyGenericsFrom,
-			Dictionary<string, GenericTypeParameterBuilder> name2GenericType,
 			ApplyGenArgs genericParameterGenerator)
 		{
 			var originalGenericArguments = methodToCopyGenericsFrom.GetGenericArguments();
@@ -213,8 +132,6 @@ private static GenericTypeParameterBuilder[] CopyGenericArguments(
 
 					newGenericParameters[i].SetGenericParameterAttributes(GenericParameterAttributes.None);
 				}
-
-				name2GenericType[argumentNames[i]] = newGenericParameters[i];
 			}
 
 			return newGenericParameters;
diff --git a/src/Castle.Core/DynamicProxy/Generators/Emitters/MethodEmitter.cs b/src/Castle.Core/DynamicProxy/Generators/Emitters/MethodEmitter.cs
@@ -15,6 +15,7 @@
 namespace Castle.DynamicProxy.Generators.Emitters
 {
 	using System;
+	using System.Collections.Generic;
 	using System.Diagnostics;
 	using System.Globalization;
 	using System.Linq;
@@ -57,13 +58,15 @@ internal MethodEmitter(AbstractTypeEmitter owner, string name,
 		                       MethodAttributes attributes, MethodInfo methodToUseAsATemplate)
 			: this(owner, name, attributes)
 		{
-			var name2GenericType = GenericUtil.GetGenericArgumentsMap(owner);
+			// All code paths leading up to this constructor can be traced back to
+			// proxy type generation code. At present, proxy types are never generic.
+			Debug.Assert(owner.GenericTypeParams == null || owner.GenericTypeParams.Length == 0);
 
-			var returnType = GenericUtil.ExtractCorrectType(methodToUseAsATemplate.ReturnType, name2GenericType);
+			var returnType = methodToUseAsATemplate.ReturnType;
 			var baseMethodParameters = methodToUseAsATemplate.GetParameters();
-			var parameters = GenericUtil.ExtractParametersTypes(baseMethodParameters, name2GenericType);
+			var parameters = ArgumentsUtil.GetTypes(baseMethodParameters);
 
-			genericTypeParams = GenericUtil.CopyGenericArguments(methodToUseAsATemplate, builder, name2GenericType);
+			genericTypeParams = GenericUtil.CopyGenericArguments(methodToUseAsATemplate, builder);
 			SetParameters(parameters);
 			SetReturnType(returnType);
 			SetSignature(returnType, methodToUseAsATemplate.ReturnParameter, parameters, baseMethodParameters);
diff --git a/src/Castle.Core/DynamicProxy/Internal/TypeUtil.cs b/src/Castle.Core/DynamicProxy/Internal/TypeUtil.cs
@@ -98,44 +98,6 @@ public static Type[] GetAllInterfaces(this Type type)  // NOTE: also used by Win
 			return GetAllInterfaces(new[] { type });
 		}
 
-		internal static Type GetClosedParameterType(this AbstractTypeEmitter type, Type parameter)
-		{
-			if (parameter.IsGenericTypeDefinition)
-			{
-				return parameter.GetGenericTypeDefinition().MakeGenericType(type.GetGenericArgumentsFor(parameter));
-			}
-
-			if (parameter.IsGenericType)
-			{
-				var arguments = parameter.GetGenericArguments();
-				if (CloseGenericParametersIfAny(type, arguments))
-				{
-					return parameter.GetGenericTypeDefinition().MakeGenericType(arguments);
-				}
-			}
-
-			if (parameter.IsGenericParameter)
-			{
-				return type.GetGenericArgument(parameter.Name);
-			}
-
-			if (parameter.IsArray)
-			{
-				var elementType = GetClosedParameterType(type, parameter.GetElementType());
-				int rank = parameter.GetArrayRank();
-				return rank == 1
-					? elementType.MakeArrayType()
-					: elementType.MakeArrayType(rank);
-			}
-
-			if (parameter.IsByRef)
-			{
-				var elementType = GetClosedParameterType(type, parameter.GetElementType());
-				return elementType.MakeByRefType();
-			}
-
-			return parameter;
-		}
 
 		public static Type GetTypeOrNull(object target)
 		{
@@ -232,21 +194,6 @@ internal static bool IsDelegateType(this Type type)
 			return type.BaseType == typeof(MulticastDelegate);
 		}
 
-		private static bool CloseGenericParametersIfAny(AbstractTypeEmitter emitter, Type[] arguments)
-		{
-			var hasAnyGenericParameters = false;
-			for (var i = 0; i < arguments.Length; i++)
-			{
-				var newType = GetClosedParameterType(emitter, arguments[i]);
-				if (newType != null && !ReferenceEquals(newType, arguments[i]))
-				{
-					arguments[i] = newType;
-					hasAnyGenericParameters = true;
-				}
-			}
-			return hasAnyGenericParameters;
-		}
-
 		private static Type[] Sort(ICollection<Type> types)
 		{
 			var array = new Type[types.Count];
