Ability to configure Array.prototype as prototype for array-like object wrappers (#721)

* shouldn't cache conversion from string in DefaultTypeConverter as result may vary by input
* index descriptor should only be built if valid indexing configuration found
* pass engine to WrapObjectHandler because it's generally useful to have and performance is better without closure-bound value

Jint.Tests/Runtime/Domain/FloatIndexer.cs

@@ -1,21 +1,7 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-
-namespace Jint.Tests.Runtime.Domain
+namespace Jint.Tests.Runtime.Domain
 {
     public class FloatIndexer
     {
-		public string this[int index]
-		{
-			get
-			{
-				return "";
-				//throw new Exception();
-			}
-		}
-
-	}
+		public string this[int index] => "";
+    }
 }

Jint.Tests/Runtime/InteropTests.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
@@ -672,6 +673,64 @@ namespace Jint.Tests.Runtime
             ");
         }
 
+        private class ReadOnlyList : IReadOnlyList<Person>
+        {
+            private readonly Person[] _data;
+
+            public ReadOnlyList(params Person[] data)
+            {
+                _data = data;
+            }
+
+            public IEnumerator<Person> GetEnumerator()
+            {
+                return ((IEnumerable<Person>) _data).GetEnumerator();
+            }
+
+            IEnumerator IEnumerable.GetEnumerator()
+            {
+                return _data.GetEnumerator();
+            }
+
+            public int Count => _data.Length;
+
+            public Person this[int index] => _data[index];
+        }
+        
+        [Fact]
+        public void CanAddArrayPrototypeForArrayLikeClrObjects()
+        {
+            var e = new Engine(cfg => cfg
+                .AllowClr(typeof(Person).Assembly)
+                .SetWrapObjectHandler((engine, target) =>
+                {
+                    var instance = new ObjectWrapper(engine, target);
+                    if (instance.IsArrayLike)
+                    {
+                        instance.SetPrototypeOf(engine.Array.PrototypeObject);
+                    }
+                    return instance;
+                })
+            );
+
+            var person = new Person
+            {
+                Age = 12,
+                Name = "John"
+            };
+            
+            dynamic obj = new
+            {
+                values = new ReadOnlyList(person)
+            };
+
+            e.SetValue("o", obj);
+
+            var name = e.Execute("o.values.filter(x => x.age == 12)[0].name").GetCompletionValue().ToString();
+            Assert.Equal("John", name);
+
+        }
+
         [Fact]
         public void ShouldConvertArrayToArrayInstance()
         {

Jint/Native/Array/ArrayInstance.cs

@@ -57,7 +57,7 @@ namespace Jint.Native.Array
             _length = new PropertyDescriptor(length, PropertyFlag.OnlyWritable);
         }
 
-        internal override bool IsArrayLike => true;
+        public override bool IsArrayLike => true;
 
         public override bool DefineOwnProperty(JsValue property, PropertyDescriptor desc)
         {

Jint/Native/JsValue.cs

@@ -348,8 +348,8 @@ namespace Jint.Native
 
             // if no known type could be guessed, wrap it as an ObjectInstance
             var h = engine.Options._WrapObjectHandler;
-            ObjectInstance o = h != null ? h(value) : null;
-            return o ?? new ObjectWrapper(engine, value);
+            var o = h?.Invoke(engine, value) ?? new ObjectWrapper(engine, value);
+            return o;
         }
 
         private static JsValue Convert(Engine e, object v)

Jint/Native/Object/ObjectInstance.cs

@@ -1090,9 +1090,9 @@ namespace Jint.Native.Object
             }
         }
 
-        internal virtual bool IsArrayLike => TryGetValue(CommonProperties.Length, out var lengthValue)
-                                             && lengthValue.IsNumber()
-                                             && ((JsNumber) lengthValue)._value >= 0;
+        public virtual bool IsArrayLike => TryGetValue(CommonProperties.Length, out var lengthValue)
+                                           && lengthValue.IsNumber()
+                                           && ((JsNumber) lengthValue)._value >= 0;
 
 
         public virtual uint Length => (uint) TypeConverter.ToLength(Get(CommonProperties.Length));

Jint/Options.cs

@@ -18,7 +18,7 @@ namespace Jint
         private bool _allowClr;
         private bool _allowClrWrite = true;
         private readonly List<IObjectConverter> _objectConverters = new List<IObjectConverter>();
-        private Func<object, ObjectInstance> _wrapObjectHandler;
+        private Func<Engine, object, ObjectInstance> _wrapObjectHandler;
         private int _maxRecursionDepth = -1;
         private TimeSpan _regexTimeoutInterval = TimeSpan.FromSeconds(10);
         private CultureInfo _culture = CultureInfo.CurrentCulture;
@@ -82,7 +82,7 @@ namespace Jint
         /// ObjectInstance using class ObjectWrapper. This function can be used to
         /// register a handler for a customized handling.
         /// </summary>
-        public Options SetWrapObjectHandler(Func<object, ObjectInstance> wrapObjectHandler)
+        public Options SetWrapObjectHandler(Func<Engine, object, ObjectInstance> wrapObjectHandler)
         {
             _wrapObjectHandler = wrapObjectHandler;
             return this;
@@ -201,7 +201,7 @@ namespace Jint
 
         internal List<IConstraint> _Constraints => _constraints;
 
-        internal Func<object, ObjectInstance> _WrapObjectHandler => _wrapObjectHandler;
+        internal Func<Engine, object, ObjectInstance> _WrapObjectHandler => _wrapObjectHandler;
 
         internal int MaxRecursionDepth => _maxRecursionDepth;
 

Jint/Runtime/Descriptors/Specialized/IndexDescriptor.cs

@@ -9,60 +9,73 @@ namespace Jint.Runtime.Descriptors.Specialized
     {
         private readonly Engine _engine;
         private readonly object _key;
-        private readonly object _item;
+        private readonly object _target;
         private readonly PropertyInfo _indexer;
         private readonly MethodInfo _containsKey;
 
-        public IndexDescriptor(Engine engine, Type targetType, string key, object item) : base(PropertyFlag.CustomJsValue)
+        public IndexDescriptor(Engine engine, Type targetType, string key, object target) : base(PropertyFlag.CustomJsValue)
         {
             _engine = engine;
-            _item = item;
+            _target = target;
 
+            if (!TryFindIndexer(engine, targetType, key, out _indexer, out _containsKey, out _key))
+            {
+                ExceptionHelper.ThrowArgumentException("invalid indexing configuration, target indexer not found");
+            }
+
+            Writable = engine.Options._IsClrWriteAllowed;
+        }
+        
+        public IndexDescriptor(Engine engine, string key, object item)
+            : this(engine, item.GetType(), key, item)
+        {
+        }
+
+        internal static bool TryFindIndexer(
+            Engine engine,
+            Type targetType,
+            string propertyName, 
+            out PropertyInfo indexerProperty,
+            out MethodInfo containsKeyMethod, 
+            out object indexerKey)
+        {
             // get all instance indexers with exactly 1 argument
-            var indexers = targetType.GetProperties();
             var paramTypeArray = new Type[1];
 
             // try to find first indexer having either public getter or setter with matching argument type
-            foreach (var indexer in indexers)
+            foreach (var candidate in targetType.GetProperties())
             {
-                var indexParameters = indexer.GetIndexParameters();
+                var indexParameters = candidate.GetIndexParameters();
                 if (indexParameters.Length != 1)
                 {
                     continue;
                 }
 
-                if (indexer.GetGetMethod() != null || indexer.GetSetMethod() != null)
+                if (candidate.GetGetMethod() != null || candidate.GetSetMethod() != null)
                 {
                     var paramType = indexParameters[0].ParameterType;
 
-                    if (_engine.ClrTypeConverter.TryConvert(key, paramType, CultureInfo.InvariantCulture, out _key))
+                    if (engine.ClrTypeConverter.TryConvert(propertyName, paramType, CultureInfo.InvariantCulture, out indexerKey))
                     {
-                        _indexer = indexer;
+                        indexerProperty = candidate;
                         // get contains key method to avoid index exception being thrown in dictionaries
                         paramTypeArray[0] = paramType;
-                        _containsKey = targetType.GetMethod("ContainsKey", paramTypeArray);
-                        break;
+                        containsKeyMethod = targetType.GetMethod("ContainsKey", paramTypeArray);
+                        return true;
                     }
                 }
             }
 
-            Writable = engine.Options._IsClrWriteAllowed;
-        }
-
-        public IndexDescriptor(Engine engine, string key, object item)
-            : this(engine, item.GetType(), key, item)
-        {
+            indexerProperty = default;
+            containsKeyMethod = default;
+            indexerKey = default;
+            return false;
         }
 
         protected internal override JsValue CustomValue
         {
             get
             {
-                if (_indexer == null)
-                {
-                    return JsValue.Undefined;
-                }
-
                 var getter = _indexer.GetGetMethod();
 
                 if (getter == null)
@@ -74,7 +87,7 @@ namespace Jint.Runtime.Descriptors.Specialized
 
                 if (_containsKey != null)
                 {
-                    if ((_containsKey.Invoke(_item, parameters) as bool?) != true)
+                    if ((_containsKey.Invoke(_target, parameters) as bool?) != true)
                     {
                         return JsValue.Undefined;
                     }
@@ -82,7 +95,7 @@ namespace Jint.Runtime.Descriptors.Specialized
 
                 try
                 {
-                    return JsValue.FromObject(_engine, getter.Invoke(_item, parameters));
+                    return JsValue.FromObject(_engine, getter.Invoke(_target, parameters));
                 }
                 catch
                 {
@@ -92,11 +105,6 @@ namespace Jint.Runtime.Descriptors.Specialized
 
             set
             {
-                if (_indexer == null)
-                {
-                    return;
-                }
-
                 var setter = _indexer.GetSetMethod();
                 if (setter == null)
                 {
@@ -104,7 +112,7 @@ namespace Jint.Runtime.Descriptors.Specialized
                 }
 
                 object[] parameters = {_key, value?.ToObject()};
-                setter.Invoke(_item, parameters);
+                setter.Invoke(_target, parameters);
             }
         }
     }

Jint/Runtime/Interop/DefaultTypeConverter.cs

@@ -282,7 +282,8 @@ namespace Jint.Runtime.Interop
             var key = value == null ? $"Null->{type}" : $"{value.GetType()}->{type}";
 #endif
 
-            var canConvert = _knownConversions.GetOrAdd(key, _ =>
+            // string conversion is not stable, "filter" -> int is invalid, "0" -> int is valid
+            var canConvert = value is string || _knownConversions.GetOrAdd(key, _ =>
             {
                 try
                 {

Jint/Runtime/Interop/ObjectWrapper.cs

@@ -18,19 +18,50 @@ namespace Jint.Runtime.Interop
             : base(engine)
         {
             Target = obj;
-            if (obj is ICollection collection)
+            var type = obj.GetType();
+            if (ObjectIsArrayLikeClrCollection(type))
             {
                 IsArrayLike = true;
-                // create a forwarder to produce length from Count
-                var functionInstance = new ClrFunctionInstance(engine, "length", (thisObj, arguments) => collection.Count);
+                // create a forwarder to produce length from Count or Length, whichever is present
+                var lengthProperty = type.GetProperty("Count") ?? type.GetProperty("Length");
+                if (lengthProperty is null)
+                {
+                    ExceptionHelper.ThrowArgumentException("collection is supposed to either have Count or Length property");
+                    return;
+                }
+                var functionInstance = new ClrFunctionInstance(engine, "length", (thisObj, arguments) => JsNumber.Create((int) lengthProperty.GetValue(obj)));
                 var descriptor = new GetSetPropertyDescriptor(functionInstance, Undefined, PropertyFlag.Configurable);
                 AddProperty(CommonProperties.Length, descriptor);
             }
         }
 
+        internal static bool ObjectIsArrayLikeClrCollection(Type type)
+        {
+            if (typeof(ICollection).IsAssignableFrom(type))
+            {
+                return true;
+            }
+            
+            foreach (var interfaceType in type.GetInterfaces())
+            {
+                if (!interfaceType.IsGenericType)
+                {
+                    continue;
+                }
+                
+                if (interfaceType.GetGenericTypeDefinition() == typeof(IReadOnlyCollection<>)
+                    || interfaceType.GetGenericTypeDefinition() == typeof(ICollection<>))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
         public object Target { get; }
 
-        internal override bool IsArrayLike { get; }
+        public override bool IsArrayLike { get; }
 
         public override bool Set(JsValue property, JsValue value, JsValue receiver)
         {
@@ -76,7 +107,7 @@ namespace Jint.Runtime.Interop
             return descriptor;
         }
         
-        private static Func<Engine, object, PropertyDescriptor> ResolveProperty(Type type, string propertyName)
+        private Func<Engine, object, PropertyDescriptor> ResolveProperty(Type type, string propertyName)
         {
             var isNumber = uint.TryParse(propertyName, out _);
 
@@ -121,7 +152,7 @@ namespace Jint.Runtime.Interop
                 {
                     if (EqualsIgnoreCasing(m.Name, propertyName))
                     {
-                        methods = methods ?? new List<MethodInfo>();
+                        methods ??= new List<MethodInfo>();
                         methods.Add(m);
                     }
                 }
@@ -134,17 +165,7 @@ namespace Jint.Runtime.Interop
             }
 
             // if no methods are found check if target implemented indexing
-            PropertyInfo first = null;
-            foreach (var p in type.GetProperties())
-            {
-                if (p.GetIndexParameters().Length != 0)
-                {
-                    first = p;
-                    break;
-                }
-            }
-
-            if (first != null)
+            if (IndexDescriptor.TryFindIndexer(_engine, type, propertyName, out _, out _, out _))
             {
                 return (engine, target) => new IndexDescriptor(engine, propertyName, target);
             }
@@ -157,7 +178,7 @@ namespace Jint.Runtime.Interop
                 {
                     if (EqualsIgnoreCasing(iprop.Name, propertyName))
                     {
-                        list = list ?? new List<PropertyInfo>();
+                        list ??= new List<PropertyInfo>();
                         list.Add(iprop);
                     }
                 }
@@ -176,7 +197,7 @@ namespace Jint.Runtime.Interop
                 {
                     if (EqualsIgnoreCasing(imethod.Name, propertyName))
                     {
-                        explicitMethods = explicitMethods ?? new List<MethodInfo>();
+                        explicitMethods ??= new List<MethodInfo>();
                         explicitMethods.Add(imethod);
                     }
                 }
@@ -188,24 +209,14 @@ namespace Jint.Runtime.Interop
             }
 
             // try to find explicit indexer implementations
-            List<PropertyInfo> explicitIndexers = null;
-            foreach (Type iface in type.GetInterfaces())
+            foreach (Type interfaceType in type.GetInterfaces())
             {
-                foreach (var iprop in iface.GetProperties())
+                if (IndexDescriptor.TryFindIndexer(_engine, interfaceType, propertyName, out _, out _, out _))
                 {
-                    if (iprop.GetIndexParameters().Length != 0)
-                    {
-                        explicitIndexers = explicitIndexers ?? new List<PropertyInfo>();
-                        explicitIndexers.Add(iprop);
-                    }
+                    return (engine, target) => new IndexDescriptor(engine, interfaceType, propertyName, target);
                 }
             }
 
-            if (explicitIndexers?.Count == 1)
-            {
-                return (engine, target) => new IndexDescriptor(engine, explicitIndexers[0].DeclaringType, propertyName, target);
-            }
-
             return (engine, target) => PropertyDescriptor.Undefined;
         }
 

Jint/Runtime/TypeConverter.cs

@@ -576,7 +576,7 @@ namespace Jint.Runtime
                         yield break;
                     }
 
-                    matchingByParameterCount = matchingByParameterCount ?? new List<Tuple<T, JsValue[]>>();
+                    matchingByParameterCount ??= new List<Tuple<T, JsValue[]>>();
                     matchingByParameterCount.Add(new Tuple<T, JsValue[]>(m, arguments));
                 }
                 else if (parameterInfos.Length > arguments.Length)