Browse Source

Tweak object wrapper reported members logic (#1956)

Marko Lahma 11 months ago
parent
commit
eb091eb23b

+ 64 - 41
Jint.Tests/Runtime/InteropTests.cs

@@ -3297,9 +3297,13 @@ try {
         public BaseClass Get() => _child;
         public BaseClass Get() => _child;
     }
     }
 
 
-    private class BaseClass { }
+    private class BaseClass
+    {
+    }
 
 
-    private class Child : BaseClass { }
+    private class Child : BaseClass
+    {
+    }
 
 
     [Fact]
     [Fact]
     public void AccessingBaseTypeShouldBeEqualToAccessingDerivedType()
     public void AccessingBaseTypeShouldBeEqualToAccessingDerivedType()
@@ -3328,6 +3332,7 @@ try {
     public class Strings : IStringCollection
     public class Strings : IStringCollection
     {
     {
         private readonly string[] _strings;
         private readonly string[] _strings;
+
         public Strings(string[] strings)
         public Strings(string[] strings)
         {
         {
             _strings = strings;
             _strings = strings;
@@ -3340,7 +3345,7 @@ try {
 
 
     public class Utils
     public class Utils
     {
     {
-        public IStringCollection GetStrings() => new Strings(new [] { "a", "b", "c" });
+        public IStringCollection GetStrings() => new Strings(new[] { "a", "b", "c" });
     }
     }
 
 
     [Fact]
     [Fact]
@@ -3384,6 +3389,7 @@ try {
         public bool ContainsKey(string key) => throw new NotImplementedException();
         public bool ContainsKey(string key) => throw new NotImplementedException();
         public void Add(string key, object value) => throw new NotImplementedException();
         public void Add(string key, object value) => throw new NotImplementedException();
         public bool Remove(string key) => throw new NotImplementedException();
         public bool Remove(string key) => throw new NotImplementedException();
+
         public bool TryGetValue(string key, out object value)
         public bool TryGetValue(string key, out object value)
         {
         {
             value = "from-wrapper";
             value = "from-wrapper";
@@ -3467,6 +3473,7 @@ try {
         });
         });
 
 
         var result = new List<string>();
         var result = new List<string>();
+
         void Debug(object o)
         void Debug(object o)
         {
         {
             result.Add($"{o?.GetType().Name ?? "null"}: {o ?? "null"}");
             result.Add($"{o?.GetType().Name ?? "null"}: {o ?? "null"}");
@@ -3487,73 +3494,97 @@ try {
 
 
     private class ClrMembersVisibilityTestClass
     private class ClrMembersVisibilityTestClass
     {
     {
-        public int A { get; set; } = 10;
+        public string Field = "field";
 
 
-        public int F()
+        public int Property { get; set; } = 10;
+
+        public int Method()
         {
         {
             return 4;
             return 4;
         }
         }
+
+        public string Extras { get; set; }
     }
     }
 
 
     [Fact]
     [Fact]
-    public void ShouldNotSeeClrMethods()
+    public void PropertiesShouldNotSeeReportMethodsWhenMemberTypesActive()
     {
     {
         var engine = new Engine(opt =>
         var engine = new Engine(opt =>
         {
         {
             opt.Interop.ObjectWrapperReportedMemberTypes = MemberTypes.Field | MemberTypes.Property;
             opt.Interop.ObjectWrapperReportedMemberTypes = MemberTypes.Field | MemberTypes.Property;
         });
         });
-        
+
         engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass());
         engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass());
-        
-         var val = engine.GetValue("clrInstance");
 
 
-         var obj = val.AsObject();
-         var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();
-         
-         props.Should().BeEquivalentTo(["A"]);
+        var val = engine.GetValue("clrInstance");
+
+        var obj = val.AsObject();
+        var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();
+
+        props.Should().BeEquivalentTo("Property", "Extras", "Field");
     }
     }
-    
+
     [Fact]
     [Fact]
-    public void ShouldSeeClrMethods()
+    public void PropertyKeysShouldReportMethods()
     {
     {
         var engine = new Engine();
         var engine = new Engine();
-        
+
+        engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass());
+
+        var val = engine.GetValue("clrInstance");
+        var obj = val.AsObject();
+        var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();
+
+        props.Should().BeEquivalentTo("Property", "Extras", "Field", "Method");
+    }
+
+    [Fact]
+    public void PropertyKeysShouldObeyMemberFilter()
+    {
+        var engine = new Engine(options =>
+        {
+            options.SetTypeResolver(new TypeResolver
+            {
+                MemberFilter = member => member.Name == "Extras"
+            });
+        });
+
         engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass());
         engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass());
-        
+
         var val = engine.GetValue("clrInstance");
         var val = engine.GetValue("clrInstance");
         var obj = val.AsObject();
         var obj = val.AsObject();
         var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();
         var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();
 
 
-        props.Should().BeEquivalentTo(["A", "F"]);
+        props.Should().BeEquivalentTo("Extras");
     }
     }
-    
+
     private class ClrMembersVisibilityTestClass2
     private class ClrMembersVisibilityTestClass2
     {
     {
         public int Get_A { get; set; } = 5;
         public int Get_A { get; set; } = 5;
     }
     }
-    
+
     [Fact]
     [Fact]
     public void ShouldSeeClrMethods2()
     public void ShouldSeeClrMethods2()
     {
     {
         var engine = new Engine();
         var engine = new Engine();
-        
+
         engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass2());
         engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass2());
-        
+
         var val = engine.GetValue("clrInstance");
         var val = engine.GetValue("clrInstance");
 
 
         var obj = val.AsObject();
         var obj = val.AsObject();
         var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();
         var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();
-         
-        props.Should().BeEquivalentTo(["Get_A"]);
+
+        props.Should().BeEquivalentTo("Get_A");
     }
     }
-    
+
     [Fact]
     [Fact]
     public void ShouldNotThrowOnInspectingClrFunction()
     public void ShouldNotThrowOnInspectingClrFunction()
     {
     {
         var engine = new Engine();
         var engine = new Engine();
-        
+
         engine.SetValue("clrDelegate", () => 4);
         engine.SetValue("clrDelegate", () => 4);
-        
+
         var val = engine.GetValue("clrDelegate");
         var val = engine.GetValue("clrDelegate");
 
 
         var fn = val as Function;
         var fn = val as Function;
@@ -3561,7 +3592,7 @@ try {
 
 
         decl.Should().BeNull();
         decl.Should().BeNull();
     }
     }
-    
+
     private class ShouldNotThrowOnInspectingClrFunctionTestClass
     private class ShouldNotThrowOnInspectingClrFunctionTestClass
     {
     {
         public int MyInt()
         public int MyInt()
@@ -3569,20 +3600,20 @@ try {
             return 4;
             return 4;
         }
         }
     }
     }
-    
+
     [Fact]
     [Fact]
     public void ShouldNotThrowOnInspectingClrClassFunction()
     public void ShouldNotThrowOnInspectingClrClassFunction()
     {
     {
         var engine = new Engine();
         var engine = new Engine();
-        
+
         engine.SetValue("clrCls", new ShouldNotThrowOnInspectingClrFunctionTestClass());
         engine.SetValue("clrCls", new ShouldNotThrowOnInspectingClrFunctionTestClass());
-        
+
         var val = engine.GetValue("clrCls");
         var val = engine.GetValue("clrCls");
         var clrFn = val.Get("MyInt");
         var clrFn = val.Get("MyInt");
-        
+
         var fn = clrFn as Function;
         var fn = clrFn as Function;
         var decl = fn!.FunctionDeclaration;
         var decl = fn!.FunctionDeclaration;
-        
+
         decl.Should().BeNull();
         decl.Should().BeNull();
     }
     }
 
 
@@ -3592,13 +3623,5 @@ try {
         var engine = new Engine();
         var engine = new Engine();
         engine.SetValue("c", new Circle(12.34));
         engine.SetValue("c", new Circle(12.34));
         engine.Evaluate("JSON.stringify(c)").ToString().Should().Be("{\"Radius\":12.34,\"Color\":0,\"Id\":123}");
         engine.Evaluate("JSON.stringify(c)").ToString().Should().Be("{\"Radius\":12.34,\"Color\":0,\"Id\":123}");
-
-
-        engine = new Engine(options =>
-        {
-            options.Interop.ObjectWrapperReportOnlyDeclaredMembers = true;
-        });
-        engine.SetValue("c", new Circle(12.34));
-        engine.Evaluate("JSON.stringify(c)").ToString().Should().Be("{\"Radius\":12.34}");
     }
     }
 }
 }

+ 0 - 1
Jint/Native/Atomics/AtomicsInstance.cs

@@ -1,4 +1,3 @@
-using System.Numerics;
 using System.Threading;
 using System.Threading;
 using Jint.Collections;
 using Jint.Collections;
 using Jint.Native.Object;
 using Jint.Native.Object;

+ 0 - 7
Jint/Options.cs

@@ -371,13 +371,6 @@ public class Options
         /// All other values are ignored.
         /// All other values are ignored.
         /// </summary>
         /// </summary>
         public MemberTypes ObjectWrapperReportedMemberTypes { get; set; } = MemberTypes.Field | MemberTypes.Property | MemberTypes.Method;
         public MemberTypes ObjectWrapperReportedMemberTypes { get; set; } = MemberTypes.Field | MemberTypes.Property | MemberTypes.Method;
-
-        /// <summary>
-        /// Whether object wrapper should only report members that are declared on the object type itself, not inherited members. Defaults to false.
-        /// This is different from JS logic where only object's own members are reported and not prototypes.
-        /// </summary>
-        /// <remarks>This configuration does not affect methods, only methods declared in type itself will be reported.</remarks>
-        public bool ObjectWrapperReportOnlyDeclaredMembers { get; set; }
     }
     }
 
 
     public class ConstraintOptions
     public class ConstraintOptions

+ 13 - 6
Jint/Runtime/Interop/ObjectWrapper.cs

@@ -257,15 +257,16 @@ public class ObjectWrapper : ObjectInstance, IObjectWrapper, IEquatable<ObjectWr
 
 
             // we take public properties, fields and methods
             // we take public properties, fields and methods
             var bindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
             var bindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
-            if (interopOptions.ObjectWrapperReportOnlyDeclaredMembers)
-            {
-                bindingFlags |= BindingFlags.DeclaredOnly;
-            }
 
 
             if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Property) == MemberTypes.Property)
             if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Property) == MemberTypes.Property)
             {
             {
                 foreach (var p in ClrType.GetProperties(bindingFlags))
                 foreach (var p in ClrType.GetProperties(bindingFlags))
                 {
                 {
+                    if (!interopOptions.TypeResolver.Filter(_engine, ClrType, p))
+                    {
+                        continue;
+                    }
+
                     var indexParameters = p.GetIndexParameters();
                     var indexParameters = p.GetIndexParameters();
                     if (indexParameters.Length == 0)
                     if (indexParameters.Length == 0)
                     {
                     {
@@ -278,15 +279,21 @@ public class ObjectWrapper : ObjectInstance, IObjectWrapper, IEquatable<ObjectWr
             {
             {
                 foreach (var f in ClrType.GetFields(bindingFlags))
                 foreach (var f in ClrType.GetFields(bindingFlags))
                 {
                 {
+                    if (!interopOptions.TypeResolver.Filter(_engine, ClrType, f))
+                    {
+                        continue;
+                    }
+
                     yield return JsString.Create(f.Name);
                     yield return JsString.Create(f.Name);
                 }
                 }
             }
             }
 
 
             if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Method) == MemberTypes.Method)
             if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Method) == MemberTypes.Method)
             {
             {
-                foreach (var m in ClrType.GetMethods(bindingFlags | BindingFlags.DeclaredOnly))
+                foreach (var m in ClrType.GetMethods(bindingFlags))
                 {
                 {
-                    if (m.IsSpecialName)
+                    // we won't report anything from base object as it would usually not be something to expect from JS perspective
+                    if (m.DeclaringType == typeof(object) || m.IsSpecialName || !interopOptions.TypeResolver.Filter(_engine, ClrType, m))
                     {
                     {
                         continue;
                         continue;
                     }
                     }

+ 0 - 1
Jint/Runtime/Interop/Reflection/DynamicObjectAccessor.cs

@@ -1,5 +1,4 @@
 using System.Dynamic;
 using System.Dynamic;
-using System.Reflection;
 using Jint.Native;
 using Jint.Native;
 
 
 #pragma warning disable IL2092
 #pragma warning disable IL2092