Improve dictionary based CLR interop (#1127)

Jint.Tests/Runtime/InteropTests.cs

@@ -514,12 +514,9 @@ namespace Jint.Tests.Runtime
             list.Add("Goofy");
 
             _engine.SetValue("list", list);
+            _engine.Evaluate("list[1] = 'Donald Duck';");
 
-            RunTest(@"
-                list[1] = 'Donald Duck';
-                assert(list[1] === 'Donald Duck');
-            ");
-
+            Assert.Equal("Donald Duck", _engine.Evaluate("list[1]").AsString());
             Assert.Equal("Mickey Mouse", list[0]);
             Assert.Equal("Donald Duck", list[1]);
         }
@@ -2798,6 +2795,35 @@ namespace Jint.Tests.Runtime
             Assert.Equal("a,1b,2c,3", _engine.Evaluate(Script));
         }
 
+        [Fact]
+        public void ShouldNotIntroduceNewPropertiesWhenTraversing()
+        {
+            _engine.SetValue("x", new Dictionary<string, int> { { "First", 1 }, { "Second", 2 } });
+
+            Assert.Equal("[\"First\",\"Second\"]", _engine.Evaluate("JSON.stringify(Object.keys(x))"));
+
+            Assert.Equal("x['First']: 1", _engine.Evaluate("\"x['First']: \" + x['First']"));
+            Assert.Equal("[\"First\",\"Second\"]", _engine.Evaluate("JSON.stringify(Object.keys(x))"));
+            
+            Assert.Equal("x['Third']: undefined", _engine.Evaluate("\"x['Third']: \" + x['Third']"));
+            Assert.Equal("[\"First\",\"Second\"]", _engine.Evaluate("JSON.stringify(Object.keys(x))"));
+            
+            Assert.Equal(JsValue.Undefined, _engine.Evaluate("x.length"));
+            Assert.Equal("[\"First\",\"Second\"]", _engine.Evaluate("JSON.stringify(Object.keys(x))"));
+            
+            Assert.Equal(2, _engine.Evaluate("x.Count").AsNumber());
+            Assert.Equal("[\"First\",\"Second\"]", _engine.Evaluate("JSON.stringify(Object.keys(x))"));
+
+            _engine.Evaluate("x.Clear();");
+
+            Assert.Equal("[]", _engine.Evaluate("JSON.stringify(Object.keys(x))"));
+
+            _engine.Evaluate("x['Fourth'] = 4;");
+            Assert.Equal("[\"Fourth\"]", _engine.Evaluate("JSON.stringify(Object.keys(x))"));
+
+            Assert.False(_engine.Evaluate("Object.prototype.hasOwnProperty.call(x, 'Third')").AsBoolean());
+        }
+
         private class Profile
         {
             public int AnyProperty => throw new NotSupportedException("NOT SUPPORTED");

Jint/Runtime/Interop/ObjectWrapper.cs

@@ -220,7 +220,11 @@ namespace Jint.Runtime.Interop
 
             var member = property.ToString();
 
-            if (_typeDescriptor.IsStringKeyedGenericDictionary)
+            // if type is dictionary, we cannot enumerate anything other than keys
+            // and we cannot store accessors as dictionary can change dynamically
+            
+            var isDictionary = _typeDescriptor.IsStringKeyedGenericDictionary;
+            if (isDictionary)
             {
                 if (_typeDescriptor.TryGetValue(Target, member, out var value))
                 {
@@ -235,8 +239,11 @@ namespace Jint.Runtime.Interop
             }
 
             var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, Target.GetType(), member);
-            var descriptor = accessor.CreatePropertyDescriptor(_engine, Target);
-            SetProperty(member, descriptor);
+            var descriptor = accessor.CreatePropertyDescriptor(_engine, Target, enumerable: !isDictionary);
+            if (!isDictionary)
+            {
+                SetProperty(member, descriptor);
+            }
             return descriptor;
         }
 

Jint/Runtime/Interop/Reflection/ConstantValueAccessor.cs

@@ -27,11 +27,11 @@ namespace Jint.Runtime.Interop.Reflection
             throw new InvalidOperationException();
         }
 
-        public override PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target)
+        public override PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target, bool enumerable = true)
         {
             return ConstantValue is null 
                 ? PropertyDescriptor.Undefined 
                 : new(ConstantValue, PropertyFlag.AllForbidden);
         }
     }
-}
+}

Jint/Runtime/Interop/Reflection/IndexerAccessor.cs

@@ -63,7 +63,7 @@ namespace Jint.Runtime.Interop.Reflection
                     // get contains key method to avoid index exception being thrown in dictionaries
                     paramTypeArray[0] = paramType;
                     var containsKeyMethod = targetType.GetMethod(nameof(IDictionary<string, string>.ContainsKey), paramTypeArray);
-                    if (containsKeyMethod is null)
+                    if (containsKeyMethod is null && targetType.IsAssignableFrom(typeof(IDictionary)))
                     {
                         paramTypeArray[0] = typeof(object);
                         containsKeyMethod = targetType.GetMethod(nameof(IDictionary.Contains), paramTypeArray);
@@ -130,7 +130,7 @@ namespace Jint.Runtime.Interop.Reflection
                 return null;
             }
 
-            object[] parameters = {_key};
+            object[] parameters = { _key };
 
             if (_containsKey != null)
             {
@@ -150,13 +150,21 @@ namespace Jint.Runtime.Interop.Reflection
                 ExceptionHelper.ThrowInvalidOperationException("Indexer has no public setter.");
             }
 
-            object[] parameters = {_key, value};
+            object[] parameters = { _key, value };
             _setter!.Invoke(target, parameters);
         }
 
-        public override PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target)
+        public override PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target, bool enumerable = true)
         {
+            if (_containsKey != null)
+            {
+                if (_containsKey.Invoke(target, new[] { _key }) as bool? != true)
+                {
+                    return PropertyDescriptor.Undefined;
+                }
+            }
+
             return new ReflectionDescriptor(engine, this, target, false);
         }
     }
-}
+}

Jint/Runtime/Interop/Reflection/MethodAccessor.cs

@@ -22,7 +22,7 @@ namespace Jint.Runtime.Interop.Reflection
         {
         }
 
-        public override PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target)
+        public override PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target, bool enumerable = true)
         {
             return new(new MethodInfoFunctionInstance(engine, _methods), PropertyFlag.AllForbidden);
         }

Jint/Runtime/Interop/Reflection/ReflectionAccessor.cs

@@ -112,9 +112,9 @@ namespace Jint.Runtime.Interop.Reflection
             return engine.ClrTypeConverter.Convert(value, _memberType, CultureInfo.InvariantCulture);
         }
 
-        public virtual PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target)
+        public virtual PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target, bool enumerable = true)
         {
-            return new ReflectionDescriptor(engine, this, target, true);
+            return new ReflectionDescriptor(engine, this, target, enumerable);
         }
     }
-}
+}

Jint/Runtime/Interop/TypeReference.cs

@@ -153,7 +153,7 @@ namespace Jint.Runtime.Interop
         {
             var key = new Tuple<Type, string>(ReferenceType, name);
             var accessor = _memberAccessors.GetOrAdd(key, x => ResolveMemberAccessor(x.Item1, x.Item2));
-            return accessor.CreatePropertyDescriptor(_engine, ReferenceType);
+            return accessor.CreatePropertyDescriptor(_engine, ReferenceType, enumerable: true);
         }
 
         private ReflectionAccessor ResolveMemberAccessor(Type type, string name)