Fix Array.from, entries, spread and other iterator related methods (#927)

* add failing string and iterator tests
* fix Array.entries
* add Symbol.iterator property to iterator
* add string trimLeft and trimRight
* added babel-standalone test
* add test for Symbol.iterator check
* fix iterator logic of Array.from, entries & spread
* enrich tests

Jint.Tests.CommonScripts/SunSpiderTests.cs

@@ -12,7 +12,7 @@ namespace Jint.Tests.CommonScripts
         {
             var engine = new Engine()
                 .SetValue("log", new Action<object>(Console.WriteLine))
-                .SetValue("assert", new Action<bool>(Assert.True))
+                .SetValue("assert", new Action<bool, string>(Assert.True))
             ;
 
             try
@@ -54,6 +54,7 @@ namespace Jint.Tests.CommonScripts
         [InlineData("string-tagcloud.js")]
         [InlineData("string-unpack-code.js")]
         [InlineData("string-validate-input.js")]
+        [InlineData("babel-standalone.js")]
         public void RunScript(string url)
         {
             var content = GetEmbeddedFile(url);

Jint.Tests/Runtime/ArrayTests.cs

@@ -129,5 +129,36 @@ namespace Jint.Tests.Runtime
             _engine.Evaluate("const a = new MyArr(1,2);");
             Assert.True(_engine.Evaluate("a instanceof MyArr").AsBoolean());
         }
+
+        [Fact]
+        public void IteratorShouldBeConvertibleToArray()
+        {
+            Assert.Equal("hello;again", _engine.Evaluate("Array.from(['hello', 'again'].values()).join(';')"));
+            Assert.Equal("hello;another", _engine.Evaluate("Array.from(new Map([['hello', 'world'], ['another', 'value']]).keys()).join(';')"));
+        }
+
+        [Fact]
+        public void ArrayFromShouldNotFlattenInputArray()
+        {
+            Assert.Equal("a;b", _engine.Evaluate("[...['a', 'b']].join(';')"));
+            Assert.Equal("0,a;1,b", _engine.Evaluate("[...['a', 'b'].entries()].join(';')"));
+            Assert.Equal("0,c;1,d", _engine.Evaluate("Array.from(['c', 'd'].entries()).join(';')"));
+            Assert.Equal("0,e;1,f", _engine.Evaluate("Array.from([[0, 'e'],[1, 'f']]).join(';')"));
+        }
+
+        [Fact]
+        public void ArrayEntriesShouldReturnKeyValuePairs()
+        {
+            Assert.Equal("0,hello,1,world", _engine.Evaluate("Array.from(['hello', 'world'].entries()).join()"));
+            Assert.Equal("0,hello;1,world", _engine.Evaluate("Array.from(['hello', 'world'].entries()).join(';')"));
+            Assert.Equal("0,;1,1;2,5", _engine.Evaluate("Array.from([,1,5,].entries()).join(';')"));
+        }
+
+        [Fact]
+        public void IteratorsShouldHaveIteratorSymbol()
+        {
+            _engine.Execute("assert(!!['hello'].values()[Symbol.iterator])");
+            _engine.Execute("assert(!!new Map([['hello', 'world']]).keys()[Symbol.iterator])");
+        }
     }
-}
+}

Jint.Tests/Runtime/StringTests.cs

@@ -30,5 +30,14 @@ bar += 'bar';
             Assert.Equal("foofoo", foo);
             Assert.Equal("foofoobar", bar);
         }
+
+        [Fact]
+        public void TrimLeftRightShouldBeSameAsTrimStartEnd()
+        {
+            _engine.Execute(@"
+                equal(''.trimLeft, ''.trimStart);
+                equal(''.trimRight, ''.trimEnd);
+");
+        }
     }
-}
+}

Jint/Native/Array/ArrayConstructor.cs

@@ -186,17 +186,16 @@ namespace Jint.Native.Array
             protected override void ProcessItem(JsValue[] args, JsValue currentValue)
             {
                 _index++;
-                var sourceValue = ExtractValueFromIteratorInstance(currentValue);
                 JsValue jsValue;
                 if (!ReferenceEquals(_callable, null))
                 {
-                    args[0] = sourceValue;
+                    args[0] = currentValue;
                     args[1] = _index;
                     jsValue = _callable.Call(_thisArg, args);
                 }
                 else
                 {
-                    jsValue = sourceValue;
+                    jsValue = currentValue;
                 }
 
                 _instance.Set((uint) _index, jsValue, updateLength: false, throwOnError: true);

Jint/Native/Array/ArrayPrototype.cs

@@ -125,9 +125,9 @@ namespace Jint.Native.Array
 
         private ObjectInstance Entries(JsValue thisObj, JsValue[] arguments)
         {
-            if (thisObj is ObjectInstance oi)
+            if (thisObj is ObjectInstance oi && oi.IsArrayLike)
             {
-                return _realm.Intrinsics.Iterator.Construct(oi, intrinsics => intrinsics.Iterator.ArrayIteratorPrototypeObject);
+                return _realm.Intrinsics.Iterator.ConstructArrayLikeEntriesIterator(oi);
             }
 
             ExceptionHelper.ThrowTypeError(_realm, "cannot construct iterator");

Jint/Native/Iterator/IteratorConstructor.cs

@@ -124,6 +124,16 @@ namespace Jint.Native.Iterator
             return instance;
         }
 
+        internal ObjectInstance ConstructArrayLikeEntriesIterator(ObjectInstance array)
+        {
+            var instance = new IteratorInstance.ArrayLikeEntriesIterator(Engine, array)
+            {
+                _prototype = ArrayIteratorPrototypeObject
+            };
+
+            return instance;
+        }
+
         internal ObjectInstance CreateRegExpStringIterator(ObjectInstance iteratingRegExp, string iteratedString, bool global, bool unicode)
         {
             var instance = new IteratorInstance.RegExpStringIterator(Engine, iteratingRegExp, iteratedString, global, unicode)

Jint/Native/Iterator/IteratorInstance.cs

@@ -111,7 +111,7 @@ namespace Jint.Native.Iterator
             {
                 if (_position < _map.GetSize())
                 {
-                    var key  = _map._map.GetKey(_position);
+                    var key = _map._map.GetKey(_position);
                     var value = _map._map[key];
 
                     _position++;
@@ -178,7 +178,7 @@ namespace Jint.Native.Iterator
                 {
                     var value = _set._set[_position];
                     _position++;
-                    nextItem = new  ValueIteratorPosition(_engine, value);
+                    nextItem = new ValueIteratorPosition(_engine, value);
                     return true;
                 }
 
@@ -204,7 +204,7 @@ namespace Jint.Native.Iterator
                 {
                     var value = _set._set[_position];
                     _position++;
-                    nextItem = new  KeyValueIteratorPosition(_engine, value, value);
+                    nextItem = new KeyValueIteratorPosition(_engine, value, value);
                     return true;
                 }
 
@@ -231,7 +231,7 @@ namespace Jint.Native.Iterator
                 {
                     var value = _values[_position];
                     _position++;
-                    nextItem = new  ValueIteratorPosition(_engine, value);
+                    nextItem = new ValueIteratorPosition(_engine, value);
                     return true;
                 }
 
@@ -258,7 +258,7 @@ namespace Jint.Native.Iterator
                 var length = _operations.GetLength();
                 if (!_closed && _position < length)
                 {
-                    nextItem = new  ValueIteratorPosition(_engine, _position++);
+                    nextItem = new ValueIteratorPosition(_engine, _position++);
                     return true;
                 }
 
@@ -296,6 +296,35 @@ namespace Jint.Native.Iterator
             }
         }
 
+        public class ArrayLikeEntriesIterator : IteratorInstance
+        {
+            private readonly ArrayOperations _operations;
+            private uint _position;
+            private bool _closed;
+
+            public ArrayLikeEntriesIterator(Engine engine, ObjectInstance objectInstance) : base(engine)
+            {
+                _operations = ArrayOperations.For(objectInstance);
+                _position = 0;
+            }
+
+            public override bool TryIteratorStep(out ObjectInstance nextItem)
+            {
+                var length = _operations.GetLength();
+                if (!_closed && _position < length)
+                {
+                    _operations.TryGetValue(_position, out var value);
+                    nextItem = new KeyValueIteratorPosition(_engine, _position, value);
+                    _position++;
+                    return true;
+                }
+
+                _closed = true;
+                nextItem = KeyValueIteratorPosition.Done;
+                return false;
+            }
+        }
+
         internal class ObjectIterator : IIterator
         {
             private readonly ObjectInstance _target;
@@ -422,7 +451,7 @@ namespace Jint.Native.Iterator
                     return false;
                 }
 
-                var match  = RegExpPrototype.RegExpExec(_iteratingRegExp, _s);
+                var match = RegExpPrototype.RegExpExec(_iteratingRegExp, _s);
                 if (match.IsNull())
                 {
                     _done = true;
@@ -450,4 +479,4 @@ namespace Jint.Native.Iterator
             }
         }
     }
-}
+}

Jint/Native/Iterator/IteratorProtocol.cs

@@ -69,23 +69,6 @@ namespace Jint.Native.Iterator
 
         protected abstract void ProcessItem(JsValue[] args, JsValue currentValue);
 
-        protected static JsValue ExtractValueFromIteratorInstance(JsValue jsValue)
-        {
-            if (jsValue is ArrayInstance ai)
-            {
-                uint index = 0;
-                if (ai.GetLength() > 1)
-                {
-                    index = 1;
-                }
-
-                ai.TryGetValue(index, out var value);
-                return value;
-            }
-
-            return jsValue;
-        }
-
         internal static void AddEntriesFromIterable(ObjectInstance target, IIterator iterable, object adder)
         {
             var callable = adder as ICallable;

Jint/Native/Iterator/IteratorPrototype.cs

@@ -30,14 +30,21 @@ namespace Jint.Native.Iterator
             };
             SetProperties(properties);
 
+            var symbols = new SymbolDictionary(_name != null ? 2 : 1)
+            {
+                [GlobalSymbolRegistry.Iterator] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "iterator", ToIterator, 1, PropertyFlag.Configurable), true, false, true),
+            };
+
             if (_name != null)
             {
-                var symbols = new SymbolDictionary(1)
-                {
-                    [GlobalSymbolRegistry.ToStringTag] = new PropertyDescriptor(_name, PropertyFlag.Configurable)
-                };
-                SetSymbols(symbols);
+                symbols[GlobalSymbolRegistry.ToStringTag] = new PropertyDescriptor(_name, PropertyFlag.Configurable);
             }
+            SetSymbols(symbols);
+        }
+
+        private JsValue ToIterator(JsValue thisObj, JsValue[] arguments)
+        {
+            return thisObj;
         }
 
         private JsValue Next(JsValue thisObj, JsValue[] arguments)
@@ -52,4 +59,4 @@ namespace Jint.Native.Iterator
             return result;
         }
     }
-}
+}

Jint/Native/String/StringPrototype.cs

@@ -39,6 +39,9 @@ namespace Jint.Native.String
         {
             const PropertyFlag lengthFlags = PropertyFlag.Configurable;
             const PropertyFlag propertyFlags = lengthFlags | PropertyFlag.Writable;
+
+            var trimStart = new PropertyDescriptor(new ClrFunctionInstance(Engine, "trimStart", TrimStart, 0, lengthFlags), propertyFlags);
+            var trimEnd = new PropertyDescriptor(new ClrFunctionInstance(Engine, "trimEnd", TrimEnd, 0, lengthFlags), propertyFlags);
             var properties = new PropertyDictionary(35, checkExistingKeys: false)
             {
                 ["constructor"] = new PropertyDescriptor(_constructor, PropertyFlag.NonEnumerable),
@@ -66,8 +69,10 @@ namespace Jint.Native.String
                 ["toUpperCase"] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "toUpperCase", ToUpperCase, 0, lengthFlags), propertyFlags),
                 ["toLocaleUpperCase"] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "toLocaleUpperCase", ToLocaleUpperCase, 0, lengthFlags), propertyFlags),
                 ["trim"] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "trim", Trim, 0, lengthFlags), propertyFlags),
-                ["trimStart"] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "trimStart", TrimStart, 0, lengthFlags), propertyFlags),
-                ["trimEnd"] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "trimEnd", TrimEnd, 0, lengthFlags), propertyFlags),
+                ["trimStart"] = trimStart,
+                ["trimEnd"] = trimEnd,
+                ["trimLeft"] = trimStart,
+                ["trimRight"] = trimEnd,
                 ["padStart"] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "padStart", PadStart, 1, lengthFlags), propertyFlags),
                 ["padEnd"] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "padEnd", PadEnd, 1, lengthFlags), propertyFlags),
                 ["includes"] = new PropertyDescriptor(new ClrFunctionInstance(Engine, "includes", Includes, 1, lengthFlags), propertyFlags),

Jint/Runtime/Environments/FunctionEnvironmentRecord.cs

@@ -407,8 +407,7 @@ namespace Jint.Runtime.Environments
             protected override void ProcessItem(JsValue[] args, JsValue currentValue)
             {
                 _index++;
-                var jsValue = ExtractValueFromIteratorInstance(currentValue);
-                _instance.SetIndexValue((uint) _index, jsValue, updateLength: false);
+                _instance.SetIndexValue((uint) _index, currentValue, updateLength: false);
             }
 
             protected override bool ShouldContinue => _index < _max;

Jint/Runtime/Interpreter/Expressions/JintArrayExpression.cs

@@ -104,9 +104,7 @@ namespace Jint.Runtime.Interpreter.Expressions
             {
                 _index++;
                 _addedCount++;
-                var jsValue = ExtractValueFromIteratorInstance(currentValue);
-
-                _instance.SetIndexValue((uint) _index, jsValue, updateLength: false);
+                _instance.SetIndexValue((uint) _index, currentValue, updateLength: false);
             }
         }
     }

Jint/Runtime/Interpreter/Expressions/JintExpression.cs

@@ -411,8 +411,7 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override void ProcessItem(JsValue[] args, JsValue currentValue)
             {
-                var jsValue = ExtractValueFromIteratorInstance(currentValue);
-                _instance.Add(jsValue);
+                _instance.Add(currentValue);
             }
         }