Fix iteration in interop (#967)

Jint.Tests/Runtime/Domain/Company.cs

@@ -35,5 +35,13 @@ namespace Jint.Tests.Runtime.Domain
         {
             return string.Compare(_name, other.Name, StringComparison.CurrentCulture);
         }
+
+        public IEnumerable<char> GetNameChars()
+        {
+            foreach (var c in _name)
+            {
+                yield return c;
+            }
+        }
     }
 }

Jint.Tests/Runtime/InteropTests.cs

@@ -529,6 +529,71 @@ namespace Jint.Tests.Runtime
             Assert.Equal("Donald Duck", list[1]);
         }
 
+        [Fact]
+        public void ShouldForOfOnLists()
+        {
+            _engine.SetValue("list", new List<string> { "a", "b" });
+
+            var result = _engine.Evaluate("var l = ''; for (var x of list) l += x; return l;").AsString();
+
+            Assert.Equal("ab", result);
+        }
+
+        [Fact]
+        public void ShouldForOfOnArrays()
+        {
+            _engine.SetValue("arr", new[] { "a", "b" });
+
+            var result = _engine.Evaluate("var l = ''; for (var x of arr) l += x; return l;").AsString();
+
+            Assert.Equal("ab", result);
+        }
+
+        [Fact]
+        public void ShouldForOfOnDictionaries()
+        {
+            _engine.SetValue("dict", new Dictionary<string, string> { {"a", "1"}, {"b", "2"} });
+
+            var result = _engine.Evaluate("var l = ''; for (var x of dict) l += x; return l;").AsString();
+
+            Assert.Equal("a,1b,2", result);
+        }
+
+        [Fact]
+        public void ShouldForOfOnExpandoObject()
+        {
+            dynamic o = new ExpandoObject();
+            o.a = 1;
+            o.b = 2;
+
+            _engine.SetValue("dynamic", o);
+
+            var result = _engine.Evaluate("var l = ''; for (var x of dynamic) l += x; return l;").AsString();
+
+            Assert.Equal("a,1b,2", result);
+        }
+
+        [Fact]
+        public void ShouldForOfOnEnumerable()
+        {
+            _engine.SetValue("c", new Company("name"));
+
+            var result = _engine.Evaluate("var l = ''; for (var x of c.getNameChars()) l += x + ','; return l;").AsString();
+
+            Assert.Equal("n,a,m,e,", result);
+        }
+
+        [Fact]
+        public void ShouldThrowWhenForOfOnObject()
+        {
+            // normal objects are not iterable in javascript
+            var o = new { A = 1, B = 2 };
+            _engine.SetValue("anonymous", o);
+
+            var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("for (var x of anonymous) {}"));
+            Assert.Equal("The value is not iterable", ex.Message);
+        }
+
         [Fact]
         public void CanAccessAnonymousObject()
         {

Jint/Native/Array/ArrayIteratorPrototype.cs

@@ -18,16 +18,6 @@ namespace Jint.Native.Array
         {
         }
 
-        internal IteratorInstance Construct(ObjectInstance array, Func<Intrinsics, Prototype> prototypeSelector)
-        {
-            var instance = new ArrayLikeIterator(Engine, array, ArrayIteratorType.KeyAndValue)
-            {
-                _prototype = prototypeSelector(_realm.Intrinsics)
-            };
-
-            return instance;
-        }
-
         internal IteratorInstance Construct(ObjectInstance array, ArrayIteratorType kind)
         {
             var instance = new ArrayLikeIterator(Engine, array, kind)

Jint/Native/JsValue.cs

@@ -193,7 +193,7 @@ namespace Jint.Native
             var objectInstance = TypeConverter.ToObject(realm, this);
 
             if (!objectInstance.TryGetValue(GlobalSymbolRegistry.Iterator, out var value)
-                || !(value is ICallable callable))
+                || value is not ICallable callable)
             {
                 iterator = null;
                 return false;

Jint/Native/Map/MapIteratorPrototype.cs

@@ -74,6 +74,5 @@ namespace Jint.Native.Map
                 return false;
             }
         }
-
     }
 }

Jint/Runtime/Interop/ObjectWrapper.cs

@@ -4,6 +4,7 @@ using System.Collections.Generic;
 using System.Globalization;
 using System.Reflection;
 using Jint.Native;
+using Jint.Native.Iterator;
 using Jint.Native.Object;
 using Jint.Native.Symbol;
 using Jint.Runtime.Descriptors;
@@ -23,21 +24,15 @@ namespace Jint.Runtime.Interop
         {
             Target = obj;
             _typeDescriptor = TypeDescriptor.Get(obj.GetType());
-            if (_typeDescriptor.IsArrayLike)
+            if (_typeDescriptor.LengthProperty is not null)
             {
                 // create a forwarder to produce length from Count or Length if one of them is present
-                var lengthProperty = obj.GetType().GetProperty("Count") ?? obj.GetType().GetProperty("Length");
-                if (lengthProperty is null)
-                {
-                    return;
-                }
-                var functionInstance = new ClrFunctionInstance(engine, "length", (_, _) => JsNumber.Create((int) lengthProperty.GetValue(obj)));
+                var functionInstance = new ClrFunctionInstance(engine, "length", GetLength);
                 var descriptor = new GetSetPropertyDescriptor(functionInstance, Undefined, PropertyFlag.Configurable);
                 SetProperty(KnownKeys.Length, descriptor);
             }
         }
 
-
         public object Target { get; }
 
         public override bool IsArrayLike => _typeDescriptor.IsArrayLike;
@@ -213,20 +208,23 @@ namespace Jint.Runtime.Interop
                 return x;
             }
 
-            if (property.IsSymbol() && property == GlobalSymbolRegistry.Iterator)
+            // if we have array-like or dictionary or expando, we can provide iterator
+            if (property.IsSymbol() && property == GlobalSymbolRegistry.Iterator && _typeDescriptor.Iterable)
             {
                 var iteratorFunction = new ClrFunctionInstance(
-                    Engine, "iterator",
-                    (thisObject, arguments) => _engine.Realm.Intrinsics.ArrayIteratorPrototype.Construct(this, intrinsics => intrinsics.IteratorPrototype),
+                    Engine,
+                    "iterator",
+                    Iterator,
                     1,
                     PropertyFlag.Configurable);
+
                 var iteratorProperty = new PropertyDescriptor(iteratorFunction, PropertyFlag.Configurable | PropertyFlag.Writable);
                 SetProperty(GlobalSymbolRegistry.Iterator, iteratorProperty);
                 return iteratorProperty;
             }
 
             var member = property.ToString();
-            var result = Engine.Options.Interop.MemberAccessor?.Invoke(Engine, Target, member);
+            var result = Engine.Options.Interop.MemberAccessor(Engine, Target, member);
             if (result is not null)
             {
                 return new PropertyDescriptor(result, PropertyFlag.OnlyEnumerable);
@@ -255,6 +253,21 @@ namespace Jint.Runtime.Interop
             return engine.Options.Interop.TypeResolver.GetAccessor(engine, target.GetType(), member.Name, Factory).CreatePropertyDescriptor(engine, target);
         }
 
+        private static JsValue Iterator(JsValue thisObj, JsValue[] arguments)
+        {
+            var wrapper = (ObjectWrapper) thisObj;
+
+            return wrapper._typeDescriptor.IsDictionary
+                ? new DictionaryIterator(wrapper._engine, wrapper)
+                : new EnumerableIterator(wrapper._engine, (IEnumerable) wrapper.Target);
+        }
+
+        private static JsValue GetLength(JsValue thisObj, JsValue[] arguments)
+        {
+            var wrapper = (ObjectWrapper) thisObj;
+            return JsNumber.Create((int) wrapper._typeDescriptor.LengthProperty.GetValue(wrapper.Target));
+        }
+
         public override bool Equals(JsValue obj)
         {
             return Equals(obj as ObjectWrapper);
@@ -284,5 +297,55 @@ namespace Jint.Runtime.Interop
         {
             return Target?.GetHashCode() ?? 0;
         }
+
+        private sealed class DictionaryIterator : IteratorInstance
+        {
+            private readonly ObjectWrapper _target;
+            private readonly IEnumerator<JsValue> _enumerator;
+
+            public DictionaryIterator(Engine engine, ObjectWrapper target) : base(engine)
+            {
+                _target = target;
+                _enumerator = target.EnumerateOwnPropertyKeys(Types.String).GetEnumerator();
+            }
+
+            public override bool TryIteratorStep(out ObjectInstance nextItem)
+            {
+                if (_enumerator.MoveNext())
+                {
+                    var key = _enumerator.Current;
+                    var value = _target.Get(key);
+
+                    nextItem = new KeyValueIteratorPosition(_engine, key, value);
+                    return true;
+                }
+
+                nextItem = KeyValueIteratorPosition.Done;
+                return false;
+            }
+        }
+
+        private sealed class EnumerableIterator : IteratorInstance
+        {
+            private readonly IEnumerator _enumerator;
+
+            public EnumerableIterator(Engine engine, IEnumerable target) : base(engine)
+            {
+                _enumerator = target.GetEnumerator();
+            }
+
+            public override bool TryIteratorStep(out ObjectInstance nextItem)
+            {
+                if (_enumerator.MoveNext())
+                {
+                    var value = _enumerator.Current;
+                    nextItem = new ValueIteratorPosition(_engine, FromObject(_engine, value));
+                    return true;
+                }
+
+                nextItem = KeyValueIteratorPosition.Done;
+                return false;
+            }
+        }
     }
 }

Jint/Runtime/Interop/TypeDescriptor.cs

@@ -2,6 +2,7 @@ using System;
 using System.Collections;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Reflection;
 
 namespace Jint.Runtime.Interop
 {
@@ -12,18 +13,29 @@ namespace Jint.Runtime.Interop
         private TypeDescriptor(Type type)
         {
             IsArrayLike = DetermineIfObjectIsArrayLikeClrCollection(type);
-            IsIntegerIndexedArray = typeof(IList).IsAssignableFrom(type);
-        }
+            IsDictionary = typeof(IDictionary).IsAssignableFrom(type) || typeof(IDictionary<string, object>).IsAssignableFrom(type);
+            IsEnumerable = typeof(IEnumerable).IsAssignableFrom(type);
 
+            if (IsArrayLike)
+            {
+                LengthProperty = type.GetProperty("Count") ?? type.GetProperty("Length");
+                IsIntegerIndexedArray = typeof(IList).IsAssignableFrom(type);
+            }
+        }
 
         public bool IsArrayLike { get; }
         public bool IsIntegerIndexedArray { get; }
+        public bool IsDictionary { get; }
+        public bool IsEnumerable { get; }
+        public PropertyInfo LengthProperty { get; }
+
+        public bool Iterable => IsArrayLike || IsDictionary || IsEnumerable;
 
         public static TypeDescriptor Get(Type type)
         {
             return _cache.GetOrAdd(type, t => new TypeDescriptor(t));
         }
-        
+
         private static bool DetermineIfObjectIsArrayLikeClrCollection(Type type)
         {
             if (typeof(IDictionary).IsAssignableFrom(type))
@@ -36,14 +48,14 @@ namespace Jint.Runtime.Interop
             {
                 return true;
             }
-            
+
             foreach (var interfaceType in type.GetInterfaces())
             {
                 if (!interfaceType.IsGenericType)
                 {
                     continue;
                 }
-                
+
                 if (interfaceType.GetGenericTypeDefinition() == typeof(IReadOnlyCollection<>)
                     || interfaceType.GetGenericTypeDefinition() == typeof(ICollection<>))
                 {
@@ -53,6 +65,5 @@ namespace Jint.Runtime.Interop
 
             return false;
         }
-
     }
 }

Jint/Runtime/Interpreter/Statements/JintForInForOfStatement.cs

@@ -340,7 +340,7 @@ namespace Jint.Runtime.Interpreter.Statements
             AsyncIterate
         }
 
-        internal class ObjectKeyVisitor : IteratorInstance
+        private sealed class ObjectKeyVisitor : IteratorInstance
         {
             public ObjectKeyVisitor(Engine engine, ObjectInstance obj)
                 : base(engine, CreateEnumerator(engine, obj))