Simplify delegate type check

...and standardize on using `Delegate` instead of `MulticastDelegate` in public method signatures. (This might seem contradictory, since the delegate type check is based on the latter. But it will arguably cause users less confusion to see `Delegate` than the less familiar `Multi- castDelegate`. Also, BCL APIs such `Delegate.CreateDelegate` have a declared return type of `Delegate`. Let's do the same.)
castleproject · Mar 29, 2019 · fcf6c63 · fcf6c63
1 parent 6b37302
commit fcf6c63
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 18 deletions.
diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassProxyInstanceContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassProxyInstanceContributor.cs
@@ -187,7 +187,7 @@ private bool VerifyIfBaseImplementsGetObjectData(Type baseType, IList<MethodInfo
 				return false;
 			}
 
-			if (IsDelegate(baseType))
+			if (baseType.IsDelegateType())
 			{
 				//working around bug in CLR which returns true for "does this type implement ISerializable" for delegates
 				return false;
@@ -229,11 +229,6 @@ private bool VerifyIfBaseImplementsGetObjectData(Type baseType, IList<MethodInfo
 
 			return true;
 		}
-
-		private bool IsDelegate(Type baseType)
-		{
-			return baseType.BaseType == typeof(MulticastDelegate);
-		}
 #endif
 	}
 }
diff --git a/src/Castle.Core/DynamicProxy/Contributors/CompositeTypeContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/CompositeTypeContributor.cs
@@ -99,7 +99,7 @@ public virtual void Generate(ClassEmitter @class, ProxyGenerationOptions options
 		public void AddInterfaceToProxy(Type @interface)
 		{
 			Debug.Assert(@interface != null, "@interface == null", "Shouldn't be adding empty interfaces...");
-			Debug.Assert(@interface.GetTypeInfo().IsInterface || @interface.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)), "@interface.IsInterface || @interface.IsSubclassOf(typeof(MulticastDelegate))", "Should be adding interfaces or delegate types only...");
+			Debug.Assert(@interface.GetTypeInfo().IsInterface || @interface.IsDelegateType(), "@interface.IsInterface || @interface.IsDelegateType()", "Should be adding interfaces or delegate types only...");
 			Debug.Assert(!interfaces.Contains(@interface), "!interfaces.ContainsKey(@interface)",
 			             "Shouldn't be adding same interface twice...");
 

diff --git a/src/Castle.Core/DynamicProxy/Contributors/DelegateTypeMembersCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/DelegateTypeMembersCollector.cs
@@ -18,6 +18,7 @@ namespace Castle.DynamicProxy.Contributors
 	using System.Reflection;
 
 	using Castle.DynamicProxy.Generators;
+	using Castle.DynamicProxy.Internal;
 
 	internal sealed class DelegateTypeMembersCollector : MembersCollector
 	{
@@ -28,7 +29,7 @@ public DelegateTypeMembersCollector(Type delegateType)
 
 		protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone)
 		{
-			if (method.Name == "Invoke" && method.DeclaringType.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)))
+			if (method.Name == "Invoke" && method.DeclaringType.IsDelegateType())
 			{
 				return new MetaMethod(method, method, isStandalone, true, false);
 			}

diff --git a/src/Castle.Core/DynamicProxy/Contributors/MixinContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/MixinContributor.cs
@@ -22,6 +22,7 @@ namespace Castle.DynamicProxy.Contributors
 	using Castle.DynamicProxy.Generators;
 	using Castle.DynamicProxy.Generators.Emitters;
 	using Castle.DynamicProxy.Generators.Emitters.SimpleAST;
+	using Castle.DynamicProxy.Internal;
 
 	public class MixinContributor : CompositeTypeContributor
 	{
@@ -79,7 +80,7 @@ protected override IEnumerable<MembersCollector> CollectElementsToProxyInternal(
 				}
 				else
 				{
-					Debug.Assert(@interface.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)));
+					Debug.Assert(@interface.IsDelegateType());
 					item = new DelegateTypeMembersCollector(@interface);
 				}
 				item.CollectMembersToProxy(hook);

diff --git a/src/Castle.Core/DynamicProxy/Generators/Emitters/ClassEmitter.cs b/src/Castle.Core/DynamicProxy/Generators/Emitters/ClassEmitter.cs
@@ -20,6 +20,8 @@ namespace Castle.DynamicProxy.Generators.Emitters
 	using System.Reflection;
 	using System.Reflection.Emit;
 
+	using Castle.DynamicProxy.Internal;
+
 	public class ClassEmitter : AbstractTypeEmitter
 	{
 		internal const TypeAttributes DefaultAttributes =
@@ -55,7 +57,7 @@ public ClassEmitter(ModuleScope modulescope, String name, Type baseType, IEnumer
 					}
 					else
 					{
-						Debug.Assert(inter.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)));
+						Debug.Assert(inter.IsDelegateType());
 					}
 				}
 			}

diff --git a/src/Castle.Core/DynamicProxy/Internal/TypeUtil.cs b/src/Castle.Core/DynamicProxy/Internal/TypeUtil.cs
@@ -226,6 +226,14 @@ public static MemberInfo[] Sort(MemberInfo[] members)
 			return sortedMembers;
 		}
 
+		/// <summary>
+		///   Checks whether the specified <paramref name="type"/> is a delegate type (i.e. a direct subclass of <see cref="MulticastDelegate"/>).
+		/// </summary>
+		internal static bool IsDelegateType(this Type type)
+		{
+			return type.GetTypeInfo().BaseType == typeof(MulticastDelegate);
+		}
+
 		private static bool CloseGenericParametersIfAny(AbstractTypeEmitter emitter, Type[] arguments)
 		{
 			var hasAnyGenericParameters = false;

diff --git a/src/Castle.Core/DynamicProxy/MixinData.cs b/src/Castle.Core/DynamicProxy/MixinData.cs
@@ -21,6 +21,7 @@ namespace Castle.DynamicProxy
 	using System.Reflection;
 
 	using Castle.DynamicProxy.Generators;
+	using Castle.DynamicProxy.Internal;
 
 	public class MixinData
 	{
@@ -55,7 +56,7 @@ public MixinData(IEnumerable<object> mixinInstances)
 						mixinInterfaces = new[] { mixin.GetType() };
 						target = mixin;
 					}
-					else if (mixin is Type delegateType && delegateType.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)))
+					else if (mixin is Type delegateType && delegateType.IsDelegateType())
 					{
 						++delegateMixinCount;
 						mixinInterfaces = new[] { delegateType };
@@ -84,7 +85,7 @@ public MixinData(IEnumerable<object> mixinInstances)
 							}
 							else
 							{
-								Debug.Assert(inter.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)));
+								Debug.Assert(inter.IsDelegateType());
 								message = string.Format(
 									"The list of mixins already contains a mixin for delegate type '{0}'.",
 									inter.FullName);
@@ -102,7 +103,7 @@ public MixinData(IEnumerable<object> mixinInstances)
 					var invokeMethods = new HashSet<MethodInfo>();
 					foreach (var mixedInType in interface2Mixin.Keys)
 					{
-						if (mixedInType.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)))
+						if (mixedInType.IsDelegateType())
 						{
 							var invokeMethod = mixedInType.GetMethod("Invoke");
 							if (invokeMethods.Contains(invokeMethod, MethodSignatureComparer.Instance))
@@ -176,8 +177,8 @@ public override bool Equals(object obj)
 
 			if (delegateMixinCount > 0)
 			{
-				var delegateMixinTypes = mixinPositions.Select(m => m.Key).Where(t => t.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)));
-				var otherDelegateMixinTypes = other.mixinPositions.Select(m => m.Key).Where(t => t.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)));
+				var delegateMixinTypes = mixinPositions.Select(m => m.Key).Where(TypeUtil.IsDelegateType);
+				var otherDelegateMixinTypes = other.mixinPositions.Select(m => m.Key).Where(TypeUtil.IsDelegateType);
 				return Enumerable.SequenceEqual(delegateMixinTypes, otherDelegateMixinTypes);
 			}
 

diff --git a/src/Castle.Core/DynamicProxy/ProxyGenerationOptions.cs b/src/Castle.Core/DynamicProxy/ProxyGenerationOptions.cs
@@ -26,6 +26,7 @@ namespace Castle.DynamicProxy
 #endif
 
 	using Castle.Core.Internal;
+	using Castle.DynamicProxy.Internal;
 
 #if FEATURE_SERIALIZATION
 	[Serializable]
@@ -140,7 +141,7 @@ public void AddDelegateTypeMixin(Type delegateType)
 				throw new ArgumentNullException(nameof(delegateType));
 			}
 
-			if (delegateType.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)) == false)
+			if (!delegateType.IsDelegateType())
 			{
 				throw new ArgumentException("Type must be a delegate type.", nameof(delegateType));
 			}
@@ -155,7 +156,7 @@ public void AddDelegateTypeMixin(Type delegateType)
 		/// </summary>
 		/// <param name="delegate">The delegate that should act as the target for calls to `Invoke` methods with a matching signature.</param>
 		/// <exception cref="ArgumentNullException"><paramref name="delegate"/> is <see langword="null"/>.</exception>
-		public void AddDelegateMixin(MulticastDelegate @delegate)
+		public void AddDelegateMixin(Delegate @delegate)
 		{
 			if (@delegate == null)
 			{

diff --git a/src/Castle.Core/DynamicProxy/ProxyUtil.cs b/src/Castle.Core/DynamicProxy/ProxyUtil.cs
@@ -27,6 +27,7 @@ namespace Castle.DynamicProxy
 
 	using Castle.Core.Internal;
 	using Castle.DynamicProxy.Generators;
+	using Castle.DynamicProxy.Internal;
 
 	public static class ProxyUtil
 	{
@@ -74,7 +75,7 @@ public static bool TryCreateDelegateToMixin(object proxy, Type delegateType, out
 				throw new ArgumentNullException(nameof(delegateType));
 			}
 
-			if (delegateType.GetTypeInfo().IsSubclassOf(typeof(MulticastDelegate)) == false)
+			if (!delegateType.IsDelegateType())
 			{
 				throw new ArgumentException("Type is not a delegate type.", nameof(delegateType));
 			}