Various Interop fixes (#1063)

* fix dictionary missing key property access
* fix destructuring with function arg in strict mode
* fix passing functions to cs side
* fix Array.sort crashing script in debug mode
* fix indexer property being enumerated

Jint.Tests/Runtime/ArrayTests.cs

@@ -176,5 +176,25 @@ namespace Jint.Tests.Runtime
             _engine.Execute("assert(!!['hello'].values()[Symbol.iterator])");
             _engine.Execute("assert(!!new Map([['hello', 'world']]).keys()[Symbol.iterator])");
         }
+
+
+
+        [Fact]
+        public void ArraySortDoesNotCrashInDebugMode()
+        {
+            var engine = new Engine(o =>
+            {
+                o.DebugMode(true);
+            });
+            engine.SetValue("equal", new Action<object, object>(Assert.Equal));
+
+            const string code = @"
+                var items = [5,2,4,1];
+                items.sort((a,b) => a - b);
+                equal('1,2,4,5', items.join());
+            ";
+
+            engine.Execute(code);
+        }
     }
 }

Jint.Tests/Runtime/ConstTests.cs

@@ -39,5 +39,30 @@ namespace Jint.Tests.Runtime
                 }
             ");
         }
+
+        [Fact]
+        public void DestructuringWithFunctionArgReferenceInStrictMode()
+        {
+            _engine.Execute(@"
+                'use strict';
+                function tst(a) {
+                    let [let1, let2, let3] = a;
+                    const [const1, const2, const3] = a;
+                    var [var1, var2, var3] = a;
+                    
+                    equal(1, var1);
+                    equal(2, var2);
+                    equal(undefined, var3);
+                    equal(1, const1);
+                    equal(2, const2);
+                    equal(undefined, const3);
+                    equal(1, let1);
+                    equal(2, let2);
+                    equal(undefined, let3);
+                }
+
+                tst([1,2])
+            ");
+        }
     }
 }

Jint.Tests/Runtime/FunctionTests.cs

@@ -124,5 +124,39 @@ assertEqual(booleanCount, 1);
             Assert.Equal("abc", arrayInstance[0]);
             Assert.Equal(123, arrayInstance[1]);
         }
+
+
+        [Fact]
+        public void FunctionInstancesCanBePassedToHost()
+        {
+            var engine = new Engine();
+            Func<JsValue, JsValue[], JsValue> ev = null;
+
+            void addListener(Func<JsValue, JsValue[], JsValue> callback)
+            {
+                ev = callback;
+            }
+
+            engine.SetValue("addListener", new Action<Func<JsValue, JsValue[], JsValue>>(addListener));
+
+            engine.Execute(@"
+                var a = 5;
+
+                (function() {
+                    var acc = 10;
+                    addListener(function (val) {
+                        a = (val || 0) + acc;
+                    });
+                })();
+");
+
+            Assert.Equal(5, engine.Evaluate("a"));
+
+            ev(null, new JsValue[0]);
+            Assert.Equal(10, engine.Evaluate("a"));
+
+            ev(null, new JsValue[] { 20 });
+            Assert.Equal(30, engine.Evaluate("a"));
+        }
     }
 }

Jint.Tests/Runtime/InteropTests.MemberAccess.cs

@@ -1,4 +1,7 @@
 using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using Jint.Native;
 using Jint.Runtime.Interop;
 using Jint.Tests.Runtime.Domain;
@@ -122,6 +125,31 @@ namespace Jint.Tests.Runtime
             Assert.True(engine.Evaluate("EchoService.Hidden").IsUndefined());
         }
 
+        [Fact]
+        public void CustomDictionaryPropertyAccessTests()
+        {
+            var engine = new Engine(options =>
+            {
+                options.AllowClr();
+            });
+
+            var dc = new CustomDictionary<float>();
+            dc["a"] = 1;
+
+            engine.SetValue("dc", dc);
+
+            Assert.Equal(1, engine.Evaluate("dc.a"));
+            Assert.Equal(1, engine.Evaluate("dc['a']"));
+            Assert.NotEqual(JsValue.Undefined, engine.Evaluate("dc.Add"));
+            Assert.NotEqual(0, engine.Evaluate("dc.Add"));
+            Assert.Equal(JsValue.Undefined, engine.Evaluate("dc.b"));
+
+            engine.Execute("dc.b = 5");
+            engine.Execute("dc.Add('c', 10)");
+            Assert.Equal(5, engine.Evaluate("dc.b"));
+            Assert.Equal(10, engine.Evaluate("dc.c"));
+        }
+
         private static class EchoService
         {
             public static string Echo(string message) => message;
@@ -129,5 +157,42 @@ namespace Jint.Tests.Runtime
             [Obsolete]
             public static string Hidden(string message) => message;
         }
+
+        private class CustomDictionary<TValue> : IDictionary<string, TValue>
+        {
+            Dictionary<string, TValue> _dictionary = new Dictionary<string, TValue>();
+
+            public TValue this[string key]
+            {
+                get
+                {
+                    if (TryGetValue(key, out var val)) return val;
+
+                    // Normally, dictionary Item accessor throws an error when key does not exist
+                    // But this is a custom implementation
+                    return default;
+                }
+                set
+                {
+                    _dictionary[key] = value;
+                }
+            }
+
+            public ICollection<string> Keys => _dictionary.Keys;
+            public ICollection<TValue> Values => _dictionary.Values;
+            public int Count => _dictionary.Count;
+            public bool IsReadOnly => false;
+            public void Add(string key, TValue value) => _dictionary.Add(key, value);
+            public void Add(KeyValuePair<string, TValue> item) => _dictionary.Add(item.Key, item.Value);
+            public void Clear() => _dictionary.Clear();
+            public bool Contains(KeyValuePair<string, TValue> item) => _dictionary.ContainsKey(item.Key);
+            public bool ContainsKey(string key) => _dictionary.ContainsKey(key);
+            public void CopyTo(KeyValuePair<string, TValue>[] array, int arrayIndex) => throw new NotImplementedException();
+            public IEnumerator<KeyValuePair<string, TValue>> GetEnumerator() => _dictionary.GetEnumerator();
+            public bool Remove(string key) => _dictionary.Remove(key);
+            public bool Remove(KeyValuePair<string, TValue> item) => _dictionary.Remove(item.Key);
+            public bool TryGetValue(string key, [MaybeNullWhen(false)] out TValue value) => _dictionary.TryGetValue(key, out value);
+            IEnumerator IEnumerable.GetEnumerator() => _dictionary.GetEnumerator();
+        }
     }
 }

Jint.Tests/Runtime/MethodAmbiguityTests.cs

@@ -99,6 +99,7 @@ namespace Jint.Tests.Runtime
             Assert.Equal("Class2.Double[]", engine.Evaluate("Class2.Print([ 1, 2 ]); "));
             Assert.Equal("Class2.ExpandoObject", engine.Evaluate("Class2.Print({ x: 1, y: 2 });"));
             Assert.Equal("Class2.Int32", engine.Evaluate("Class2.Print(5);"));
+            Assert.Equal("Class2.Object", engine.Evaluate("Class2.Print(() => '');"));
         }
 
         private struct Class1

Jint.Tests/Runtime/StringTests.cs

@@ -35,8 +35,8 @@ bar += 'bar';
         public void TrimLeftRightShouldBeSameAsTrimStartEnd()
         {
             _engine.Execute(@"
-                equal(''.trimLeft, ''.trimStart);
-                equal(''.trimRight, ''.trimEnd);
+                assert(''.trimLeft === ''.trimStart);
+                assert(''.trimRight === ''.trimEnd);
 ");
         }
     }

Jint/Engine.cs

@@ -368,7 +368,7 @@ namespace Jint
                 constraint.Check();
             }
 
-            if (_isDebugMode)
+            if (_isDebugMode && statement != null)
             {
                 DebugHandler.OnStep(statement);
             }

Jint/Native/Object/ObjectInstance.cs

@@ -426,7 +426,7 @@ namespace Jint.Native.Object
                     return true;
                 }
 
-                var getter = desc.Get ??  Undefined;
+                var getter = desc.Get ?? Undefined;
                 if (getter.IsUndefined())
                 {
                     value = Undefined;
@@ -961,7 +961,8 @@ namespace Jint.Native.Object
                 case ObjectClass.Function:
                     if (this is FunctionInstance function)
                     {
-                        converted = (Func<JsValue, JsValue[], JsValue>) function.Call;
+                        converted = new Func<JsValue, JsValue[], JsValue>(
+                            (thisVal, args) => function.Engine.Invoke(function, (object) thisVal, args));
                     }
 
                     break;
@@ -1311,7 +1312,7 @@ namespace Jint.Native.Object
                         else
                         {
                             var objectInstance = _engine.Realm.Intrinsics.Array.ArrayCreate(2);
-                            objectInstance.SetIndexValue(0,  property, updateLength: false);
+                            objectInstance.SetIndexValue(0, property, updateLength: false);
                             objectInstance.SetIndexValue(1, value, updateLength: false);
                             array.SetIndexValue(index, objectInstance, updateLength: false);
                         }

Jint/Runtime/Descriptors/Specialized/ReflectionDescriptor.cs

@@ -13,8 +13,9 @@ namespace Jint.Runtime.Descriptors.Specialized
         public ReflectionDescriptor(
             Engine engine,
             ReflectionAccessor reflectionAccessor,
-            object target)
-            : base(PropertyFlag.Enumerable | PropertyFlag.CustomJsValue)
+            object target,
+            bool enumerable)
+            : base((enumerable ? PropertyFlag.Enumerable : PropertyFlag.None) | PropertyFlag.CustomJsValue)
         {
             _engine = engine;
             _reflectionAccessor = reflectionAccessor;

Jint/Runtime/Interop/Reflection/IndexerAccessor.cs

@@ -4,6 +4,8 @@ using System.Collections.Generic;
 using System.Globalization;
 using System.Reflection;
 using Jint.Native;
+using Jint.Runtime.Descriptors;
+using Jint.Runtime.Descriptors.Specialized;
 
 namespace Jint.Runtime.Interop.Reflection
 {
@@ -151,5 +153,10 @@ namespace Jint.Runtime.Interop.Reflection
             object[] parameters = {_key, value};
             _setter!.Invoke(target, parameters);
         }
+
+        public override PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target)
+        {
+            return new ReflectionDescriptor(engine, this, target, false);
+        }
     }
 }

Jint/Runtime/Interop/Reflection/ReflectionAccessor.cs

@@ -114,7 +114,7 @@ namespace Jint.Runtime.Interop.Reflection
 
         public virtual PropertyDescriptor CreatePropertyDescriptor(Engine engine, object target)
         {
-            return new ReflectionDescriptor(engine, this, target);
+            return new ReflectionDescriptor(engine, this, target, true);
         }
     }
 }

Jint/Runtime/Interop/TypeDescriptor.cs

@@ -12,8 +12,9 @@ namespace Jint.Runtime.Interop
         private static readonly Type _genericDictionaryType = typeof(IDictionary<,>);
         private static readonly Type _stringType = typeof(string);
 
-        private readonly PropertyInfo _stringIndexer;
+        private readonly MethodInfo _tryGetValueMethod;
         private readonly PropertyInfo _keysAccessor;
+        private readonly Type _valueType;
 
         private TypeDescriptor(Type type)
         {
@@ -24,13 +25,14 @@ namespace Jint.Runtime.Interop
                     && i.GetGenericTypeDefinition() == _genericDictionaryType
                     && i.GenericTypeArguments[0] == _stringType)
                 {
-                    _stringIndexer = i.GetProperty("Item");
+                    _tryGetValueMethod = i.GetMethod("TryGetValue");
                     _keysAccessor = i.GetProperty("Keys");
+                    _valueType = i.GenericTypeArguments[1];
                     break;
                 }
             }
 
-            IsDictionary = _stringIndexer is not null || typeof(IDictionary).IsAssignableFrom(type);
+            IsDictionary = _tryGetValueMethod is not null || typeof(IDictionary).IsAssignableFrom(type);
 
             // dictionaries are considered normal-object-like
             IsArrayLike = !IsDictionary && DetermineIfObjectIsArrayLikeClrCollection(type);
@@ -47,7 +49,7 @@ namespace Jint.Runtime.Interop
         public bool IsArrayLike { get; }
         public bool IsIntegerIndexedArray { get; }
         public bool IsDictionary { get; }
-        public bool IsStringKeyedGenericDictionary => _stringIndexer is not null;
+        public bool IsStringKeyedGenericDictionary => _tryGetValueMethod is not null;
         public bool IsEnumerable { get; }
         public PropertyInfo LengthProperty { get; }
 
@@ -92,8 +94,10 @@ namespace Jint.Runtime.Interop
             // we could throw when indexing with an invalid key
             try
             {
-                o = _stringIndexer.GetValue(target, new [] { member });
-                return true;
+                var parameters = new[] { member, _valueType.IsValueType ? Activator.CreateInstance(_valueType) : null };
+                var result = (bool) _tryGetValueMethod.Invoke(target, parameters);
+                o = parameters[1];
+                return result;
             }
             catch (TargetInvocationException tie) when (tie.InnerException is KeyNotFoundException)
             {
@@ -109,7 +113,7 @@ namespace Jint.Runtime.Interop
                 ExceptionHelper.ThrowInvalidOperationException("Not a string-keyed dictionary");
             }
 
-            return (ICollection<string>)_keysAccessor.GetValue(target);
+            return (ICollection<string>) _keysAccessor.GetValue(target);
         }
     }
-}
+}

Jint/Runtime/Interpreter/Expressions/BindingPatternAssignmentExpression.cs

@@ -52,7 +52,7 @@ namespace Jint.Runtime.Interpreter.Expressions
         {
             if (pattern is ArrayPattern ap)
             {
-                return HandleArrayPattern(context, ap, argument, environment);
+                return HandleArrayPattern(context, ap, argument, environment, checkObjectPatternPropertyReference);
             }
 
             if (pattern is ObjectPattern op)
@@ -83,7 +83,8 @@ namespace Jint.Runtime.Interpreter.Expressions
             EvaluationContext context,
             ArrayPattern pattern,
             JsValue argument,
-            EnvironmentRecord environment)
+            EnvironmentRecord environment,
+            bool checkReference)
         {
             var engine = context.Engine;
             var realm = engine.Realm;
@@ -141,7 +142,7 @@ namespace Jint.Runtime.Interpreter.Expressions
                             ConsumeFromIterator(iterator, out value, out done);
                         }
 
-                        AssignToIdentifier(engine, identifier.Name, value, environment);
+                        AssignToIdentifier(engine, identifier.Name, value, environment, checkReference);
                     }
                     else if (left is MemberExpression me)
                     {

README.md

@@ -40,7 +40,6 @@ The entire execution engine was rebuild with performance in mind, in many cases
 - ✔ Promises (Experimental, API is unstable)
 - ✔ Reflect
 - ✔ Proxies
-- ✔ Reflect
 - ✔ Symbols
 - ❌ Tail calls
 - ✔ Typed arrays