Refactor the member binding logic of ClassManager

So that we can use that same logic when deserializing Maybe* types
pythonnet · lostmsu · Jan 7, 2021 · Nov 9, 2020 · Nov 9, 2020 · Nov 18, 2020
commit fe9678166f21019675cbc4ffb467c46267296688
diff --git a/src/runtime/StateSerialization/MaybeMemberInfo.cs b/src/runtime/StateSerialization/MaybeMemberInfo.cs
@@ -13,11 +13,6 @@ internal struct MaybeMemberInfo<T> : ISerializable where T: MemberInfo
 
         string name;
         MemberInfo info;
-
-        // As seen in ClassManager.GetClassInfo
-        const BindingFlags k_flags = BindingFlags.Static |
-                        BindingFlags.Instance |
-                        BindingFlags.Public ;
 
         public string DeletedMessage 
         {
@@ -64,14 +59,41 @@ internal MaybeMemberInfo(SerializationInfo serializationInfo, StreamingContext c
                 if (tp != null)
                 {
                     var field_name = serializationInfo.GetString("f");
-                    info = tp.GetField(field_name, k_flags);
+                    MemberInfo mi = tp.GetField(field_name, ClassManager.BindingFlags);
+                    if (mi != null && ShouldBindMember(mi))
+                    {
+                        info = mi;
+                    }
                 }
             }
             catch 
             {
             }
         }
 
+        // This is complicated because we bind fields 
+        // based on the visibility of the field, properties 
+        // based on it's setter/getter (which is a method 
+        //  info) visibility and events based on their
+        // AddMethod visibility.
+        static bool ShouldBindMember(MemberInfo mi)
+        {
+            if (mi is PropertyInfo pi)
+            {
+                return ClassManager.ShouldBindProperty(pi);
+            }
+            else if (mi is FieldInfo fi)
+            {
+                return ClassManager.ShouldBindField(fi);
+            }
+            else if (mi is EventInfo ei)
+            {
+                return ClassManager.ShouldBindEvent(ei);
+            }
+
+            return false;
+        }
+
         public void GetObjectData(SerializationInfo serializationInfo, StreamingContext context)
         {
             serializationInfo.AddValue("s", name);

diff --git a/src/runtime/StateSerialization/MaybeMethodBase.cs b/src/runtime/StateSerialization/MaybeMethodBase.cs
@@ -13,10 +13,6 @@ internal struct MaybeMethodBase<T> : ISerializable where T: MethodBase
 
         string name;
         MethodBase info;
-        // As seen in ClassManager.GetClassInfo
-        const BindingFlags k_flags = BindingFlags.Static |
-                        BindingFlags.Instance |
-                        BindingFlags.Public ;
 
         public T Value
         {
@@ -62,11 +58,17 @@ internal MaybeMethodBase(SerializationInfo serializationInfo, StreamingContext c
                     types[i] = Type.GetType(param[i]);
                 }
                 // Try to get the method
-                info = tp.GetMethod(field_name, k_flags, binder:null, types:types, modifiers:null);
+                MethodBase mb = tp.GetMethod(field_name, ClassManager.BindingFlags, binder:null, types:types, modifiers:null);
                 // Try again, may be a constructor
-                if (info == null && name.Contains(".ctor"))
+                if (mb == null && name.Contains(".ctor"))
                 {
-                    info = tp.GetConstructor(k_flags, binder:null, types:types, modifiers:null);
+                    mb = tp.GetConstructor(ClassManager.BindingFlags, binder:null, types:types, modifiers:null);
+                }
+
+                // Do like in ClassManager.GetClassInfo
+                if(mb != null && ClassManager.ShouldBindMethod(mb))
+                {
+                    info = mb;
                 }
             }
             catch

diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs
@@ -18,6 +18,19 @@ namespace Python.Runtime
     /// </summary>
     internal class ClassManager
     {
+
+        // Binding flags to determine which members to expose in Python.
+        // This is complicated because inheritance in Python is name
+        // based. We can't just find DeclaredOnly members, because we
+        // could have a base class A that defines two overloads of a
+        // method and a class B that defines two more. The name-based
+        // descriptor Python will find needs to know about inherited
+        // overloads as well as those declared on the sub class.
+        internal static readonly BindingFlags BindingFlags = BindingFlags.Static |
+                                                             BindingFlags.Instance |
+                                                             BindingFlags.Public |
+                                                             BindingFlags.NonPublic;
+
         private static Dictionary<MaybeType, ClassBase> cache;
         private static readonly Type dtype;
 
@@ -310,6 +323,47 @@ private static void InitClassBase(Type type, ClassBase impl)
             Runtime.PyType_Modified(tp);
         }
 
+        internal static bool ShouldBindMethod(MethodBase mb)
+        {
+            return (mb.IsPublic || mb.IsFamily || mb.IsFamilyOrAssembly);
+        }
+
+        internal static bool ShouldBindField(FieldInfo fi)
+        {
+            return (fi.IsPublic || fi.IsFamily || fi.IsFamilyOrAssembly);
+        }
+
+        internal static bool ShouldBindProperty(PropertyInfo pi)
+        {
+                MethodInfo mm = null;
+                try
+                {
+                    mm = pi.GetGetMethod(true);
+                    if (mm == null)
+                    {
+                        mm = pi.GetSetMethod(true);
+                    }
+                }
+                catch (SecurityException)
+                {
+                    // GetGetMethod may try to get a method protected by
+                    // StrongNameIdentityPermission - effectively private.
+                    return false;
+                }
+
+                if (mm == null)
+                {
+                    return false;
+                }
+
+                return ShouldBindMethod(mm);
+        }
+
+        internal static bool ShouldBindEvent(EventInfo ei)
+        {
+            return ShouldBindMethod(ei.GetAddMethod(true));
+        }
+
         private static ClassInfo GetClassInfo(Type type)
         {
             var ci = new ClassInfo();
@@ -322,18 +376,7 @@ private static ClassInfo GetClassInfo(Type type)
             Type tp;
             int i, n;
 
-            // This is complicated because inheritance in Python is name
-            // based. We can't just find DeclaredOnly members, because we
-            // could have a base class A that defines two overloads of a
-            // method and a class B that defines two more. The name-based
-            // descriptor Python will find needs to know about inherited
-            // overloads as well as those declared on the sub class.
-            BindingFlags flags = BindingFlags.Static |
-                                 BindingFlags.Instance |
-                                 BindingFlags.Public |
-                                 BindingFlags.NonPublic;
-
-            MemberInfo[] info = type.GetMembers(flags);
+            MemberInfo[] info = type.GetMembers(BindingFlags);
             var local = new Hashtable();
             var items = new ArrayList();
             MemberInfo m;
@@ -376,7 +419,7 @@ private static ClassInfo GetClassInfo(Type type)
                 for (i = 0; i < inheritedInterfaces.Length; ++i)
                 {
                     Type inheritedType = inheritedInterfaces[i];
-                
F5DB
    MemberInfo[] imembers = inheritedType.GetMembers(flags);
+                    MemberInfo[] imembers = inheritedType.GetMembers(BindingFlags);
                     for (n = 0; n < imembers.Length; n++)
                     {
                         m = imembers[n];
@@ -407,8 +450,7 @@ private static ClassInfo GetClassInfo(Type type)
                 {
                     case MemberTypes.Method:
                         meth = (MethodInfo)mi;
-                        if (!(meth.IsPublic || meth.IsFamily ||
-                              meth.IsFamilyOrAssembly))
+                        if (!ShouldBindMethod(meth))
                         {
                             continue;
                         }
@@ -425,28 +467,7 @@ private static ClassInfo GetClassInfo(Type type)
                     case MemberTypes.Property:
                         var pi = (PropertyInfo)mi;
 
-                        MethodInfo mm = null;
-                        try
-                        {
-                            mm = pi.GetGetMethod(true);
-                            if (mm == null)
-                            {
-                                mm = pi.GetSetMethod(true);
-                            }
-                        }
-                        catch (SecurityException)
-                        {
-                            // GetGetMethod may try to get a method protected by
-                            // StrongNameIdentityPermission - effectively private.
-                            continue;
-                        }
-
-                        if (mm == null)
-                        {
-                            continue;
-                        }
-
-                        if (!(mm.IsPublic || mm.IsFamily || mm.IsFamilyOrAssembly))
+                        if(!ShouldBindProperty(pi))
                         {
                             continue;
                         }
@@ -471,7 +492,7 @@ private static ClassInfo GetClassInfo(Type type)
 
                     case MemberTypes.Field:
                         var fi = (FieldInfo)mi;
-                        if (!(fi.IsPublic || fi.IsFamily || fi.IsFamilyOrAssembly))
+                        if (!ShouldBindField(fi))
                         {
                             continue;
                         }
@@ -481,8 +502,7 @@ private static ClassInfo GetClassInfo(Type type)
 
                     case MemberTypes.Event:
                         var ei = (EventInfo)mi;
-                        MethodInfo me = ei.GetAddMethod(true);
-                        if (!(me.IsPublic || me.IsFamily || me.IsFamilyOrAssembly))
+                        if (!ShouldBindEvent(ei))
                         {
                             continue;
                         }