Merge pull request #666 from stakx/refactor/accessibility-check

Remove redundant method accessibility checks & optimize `ProxyUtil.IsAccessibleMethod`
castleproject · Sep 10, 2023 · 222b1b8 · 222b1b8
2 parents 57e666d + c662c90
commit 222b1b8
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 44 deletions.
diff --git a/src/Castle.Core/DynamicProxy/Contributors/ClassMembersCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/ClassMembersCollector.cs
@@ -28,11 +28,6 @@ public ClassMembersCollector(Type targetType)
 
 		protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone)
 		{
-			if (ProxyUtil.IsAccessibleMethod(method) == false)
-			{
-				return null;
-			}
-
 			var accepted = AcceptMethod(method, true, hook);
 			if (!accepted && !method.IsAbstract)
 			{

diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersCollector.cs
@@ -28,11 +28,6 @@ public InterfaceMembersCollector(Type @interface)
 
 		protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone)
 		{
-			if (ProxyUtil.IsAccessibleMethod(method) == false)
-			{
-				return null;
-			}
-
 			var proxyable = AcceptMethod(method, true, hook);
 			if (!proxyable && !method.IsAbstract)
 			{

diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersOnClassCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceMembersOnClassCollector.cs
@@ -32,11 +32,6 @@ public InterfaceMembersOnClassCollector(Type type, bool onlyProxyVirtual, Interf
 
 		protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone)
 		{
-			if (ProxyUtil.IsAccessibleMethod(method) == false)
-			{
-				return null;
-			}
-
 			if (onlyProxyVirtual && IsVirtuallyImplementedInterfaceMethod(method))
 			{
 				return null;

diff --git a/src/Castle.Core/DynamicProxy/Contributors/MembersCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/MembersCollector.cs
@@ -143,6 +143,11 @@ MetaMethod AddMethod(MethodInfo method, bool isStandalone)
 					return null;
 				}
 
+				if (ProxyUtil.IsAccessibleMethod(method) == false)
+				{
+					return null;
+				}
+
 				var methodToGenerate = GetMethodToGenerate(method, hook, isStandalone);
 				if (methodToGenerate != null)
 				{
@@ -173,10 +178,7 @@ protected bool AcceptMethod(MethodInfo method, bool onlyVirtuals, IProxyGenerati
 		/// </remarks>
 		protected bool AcceptMethodPreScreen(MethodInfo method, bool onlyVirtuals, IProxyGenerationHook hook)
 		{
-			if (IsInternalAndNotVisibleToDynamicProxy(method))
-			{
-				return false;
-			}
+			// NOTE: at this point, the method's accessibility should already have been checked (see `AddMethod` above)
 
 			var isOverridable = method.IsVirtual && !method.IsFinal;
 			if (onlyVirtuals && !isOverridable)
@@ -200,12 +202,6 @@ protected bool AcceptMethodPreScreen(MethodInfo method, bool onlyVirtuals, IProx
 				return false;
 			}
 
-			//can only proxy methods that are public or protected (or internals that have already been checked above)
-			if ((method.IsPublic || method.IsFamily || method.IsAssembly || method.IsFamilyOrAssembly || method.IsFamilyAndAssembly) == false)
-			{
-				return false;
-			}
-
 			if (method.DeclaringType == typeof(MarshalByRefObject))
 			{
 				return false;
@@ -218,11 +214,5 @@ protected bool AcceptMethodPreScreen(MethodInfo method, bool onlyVirtuals, IProx
 
 			return true;
 		}
-
-		private static bool IsInternalAndNotVisibleToDynamicProxy(MethodInfo method)
-		{
-			return ProxyUtil.IsInternal(method) &&
-				   ProxyUtil.AreInternalsVisibleToDynamicProxy(method.DeclaringType.Assembly) == false;
-		}
 	}
 }
diff --git a/src/Castle.Core/DynamicProxy/Contributors/WrappedClassMembersCollector.cs b/src/Castle.Core/DynamicProxy/Contributors/WrappedClassMembersCollector.cs
@@ -37,11 +37,6 @@ public override void CollectMembersToProxy(IProxyGenerationHook hook, IMembersCo
 
 		protected override MetaMethod GetMethodToGenerate(MethodInfo method, IProxyGenerationHook hook, bool isStandalone)
 		{
-			if (ProxyUtil.IsAccessibleMethod(method) == false)
-			{
-				return null;
-			}
-
 			var interceptable = AcceptMethodPreScreen(method, true, hook);
 			if (!interceptable)
 			{

diff --git a/src/Castle.Core/DynamicProxy/ProxyUtil.cs b/src/Castle.Core/DynamicProxy/ProxyUtil.cs
@@ -194,17 +194,20 @@ internal static bool IsAccessibleType(Type target)
 		/// <returns><c>true</c> if the method is accessible to DynamicProxy, <c>false</c> otherwise.</returns>
 		internal static bool IsAccessibleMethod(MethodBase method)
 		{
-			if (method.IsPublic || method.IsFamily || method.IsFamilyOrAssembly)
+			switch (method.Attributes & MethodAttributes.MemberAccessMask)
 			{
-				return true;
-			}
+				case MethodAttributes.Assembly:
+				case MethodAttributes.FamANDAssem:
+					return AreInternalsVisibleToDynamicProxy(method.DeclaringType.Assembly);
 
-			if (method.IsAssembly || method.IsFamilyAndAssembly)
-			{
-				return AreInternalsVisibleToDynamicProxy(method.DeclaringType.Assembly);
-			}
+				case MethodAttributes.Family:
+				case MethodAttributes.FamORAssem:
+				case MethodAttributes.Public:
+					return true;
 
-			return false;
+				default:  // `MethodAttributes.Private` or `MethodAttributes.PrivateScope`
+					return false;
+			}
 		}
 
 		/// <summary>