Improve for loops, improve reported reference errors (#566)

Jint.Tests.Ecma/TestCases/alltests.json

@@ -15885,13 +15885,13 @@
     source: "ch12/12.6/12.6.3/S12.6.3_A8_T3.js"
   },
   {
-    skip: false,
-    reason: "",
+    skip: true,
+    reason: "obsolete",
     source: "ch12/12.6/12.6.3/S12.6.3_A9.1.js"
   },
   {
-    skip: false,
-    reason: "",
+    skip: true,
+    reason: "obsolete",
     source: "ch12/12.6/12.6.3/S12.6.3_A9.js"
   },
   {

Jint.Tests.Test262/StatementTests.cs

@@ -0,0 +1,31 @@
+using Xunit;
+
+namespace Jint.Tests.Test262
+{
+    public class StatementTests : Test262Test
+    {
+        [Theory(DisplayName = "language\\statements\\for")]
+        [MemberData(nameof(SourceFiles), "language\\statements\\for", false)]
+        [MemberData(nameof(SourceFiles), "language\\statements\\for", true, Skip = "Skipped")]
+        protected void For(SourceFile sourceFile)
+        {
+            RunTestInternal(sourceFile);
+        }
+
+        [Theory(DisplayName = "language\\statements\\for-in")]
+        [MemberData(nameof(SourceFiles), "language\\statements\\for-in", false)]
+        [MemberData(nameof(SourceFiles), "language\\statements\\for-in", true, Skip = "Skipped")]
+        protected void ForIn(SourceFile sourceFile)
+        {
+            RunTestInternal(sourceFile);
+        }
+
+        [Theory(Skip = "for of not implemented", DisplayName = "language\\statements\\for-of")]
+        [MemberData(nameof(SourceFiles), "language\\statements\\for-of", false)]
+        [MemberData(nameof(SourceFiles), "language\\statements\\for-of", true, Skip = "Skipped")]
+        protected void ForOf(SourceFile sourceFile)
+        {
+            RunTestInternal(sourceFile);
+        }
+    }
+}

Jint.Tests.Test262/Test262Test.cs

@@ -21,7 +21,6 @@ namespace Jint.Tests.Test262
         private static readonly Dictionary<string, string> _skipReasons =
             new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
 
-
         static Test262Test()
         {
             //NOTE: The Date tests in test262 assume the local timezone is Pacific Standard Time
@@ -67,6 +66,8 @@ namespace Jint.Tests.Test262
             }
 
             string lastError = null;
+
+            bool negative = code.IndexOf("negative:", StringComparison.Ordinal) > -1;
             try
             {
                 engine.Execute(code);
@@ -80,7 +81,14 @@ namespace Jint.Tests.Test262
                 lastError = e.ToString();
             }
 
-            Assert.Null(lastError);
+            if (negative)
+            {
+                Assert.NotNull(lastError);
+            }
+            else
+            {
+                Assert.Null(lastError);
+            }
         }
 
         protected void RunTestInternal(SourceFile sourceFile)
@@ -121,66 +129,85 @@ namespace Jint.Tests.Test262
                     var items = features.Groups[1].Captures[0].Value.Split(",");
                     foreach (var item in items.Select(x => x.Trim()))
                     {
-                        // TODO implement
-                        if (item == "cross-realm")
-                        {
-                            skip = true;
-                            reason = "realms not implemented";
-                        }
-                        else if (item == "Symbol.species")
-                        {
-                            skip = true;
-                            reason = "Symbol.species not implemented";
-                        }
-                        else if (item == "Proxy")
-                        {
-                            skip = true;
-                            reason = "Proxies not implemented";
-                        }
-                        else if (item == "Symbol.unscopables")
-                        {
-                            skip = true;
-                            reason = "Symbol.unscopables not implemented";
-                        }
-                        else if (item == "Symbol.match")
-                        {
-                            skip = true;
-                            reason = "Symbol.match not implemented";
-                        }
-                        else if (item == "Symbol.matchAll")
-                        {
-                            skip = true;
-                            reason = "Symbol.matchAll not implemented";
-                        }
-                        else if (item == "Symbol.split")
-                        {
-                            skip = true;
-                            reason = "Symbol.split not implemented";
-                        }
-                        else if (item == "String.prototype.matchAll")
-                        {
-                            skip = true;
-                            reason = "proposal stage";
-                        }
-                        else if (item == "Symbol.search")
-                        {
-                            skip = true;
-                            reason = "Symbol.search not implemented";
-                        }
-                        else if (item == "Symbol.replace")
-                        {
-                            skip = true;
-                            reason = "Symbol.replace not implemented";
-                        }
-                        else if (item == "Symbol.toStringTag")
-                        {
-                            skip = true;
-                            reason = "Symbol.toStringTag not implemented";
-                        }
-                        else if (item == "BigInt")
+                        switch (item)
                         {
-                            skip = true;
-                            reason = "BigInt not implemented";
+                            // TODO implement
+                            case "cross-realm":
+                                skip = true;
+                                reason = "realms not implemented";
+                                break;
+                            case "tail-call-optimization":
+                                skip = true;
+                                reason = "tail-calls not implemented";
+                                break;
+                            case "class":
+                                skip = true;
+                                reason = "class keyword not implemented";
+                                break;
+                            case "Symbol.species":
+                                skip = true;
+                                reason = "Symbol.species not implemented";
+                                break;
+                            case "Proxy":
+                                skip = true;
+                                reason = "Proxies not implemented";
+                                break;
+                            case "object-spread":
+                                skip = true;
+                                reason = "Object spread not implemented";
+                                break;
+                            case "Symbol.unscopables":
+                                skip = true;
+                                reason = "Symbol.unscopables not implemented";
+                                break;
+                            case "Symbol.match":
+                                skip = true;
+                                reason = "Symbol.match not implemented";
+                                break;
+                            case "Symbol.matchAll":
+                                skip = true;
+                                reason = "Symbol.matchAll not implemented";
+                                break;
+                            case "Symbol.split":
+                                skip = true;
+                                reason = "Symbol.split not implemented";
+                                break;
+                            case "String.prototype.matchAll":
+                                skip = true;
+                                reason = "proposal stage";
+                                break;
+                            case "Symbol.search":
+                                skip = true;
+                                reason = "Symbol.search not implemented";
+                                break;
+                            case "Symbol.replace":
+                                skip = true;
+                                reason = "Symbol.replace not implemented";
+                                break;
+                            case "Symbol.toStringTag":
+                                skip = true;
+                                reason = "Symbol.toStringTag not implemented";
+                                break;
+                            case "BigInt":
+                                skip = true;
+                                reason = "BigInt not implemented";
+                                break;
+                            case "generators":
+                                skip = true;
+                                reason = "generators not implemented";
+                                break;
+                            case "destructuring-binding":
+                                skip = true;
+                                reason = "destructuring-binding not implemented";
+                                break;
+                            case "let":
+                                skip = true;
+                                reason = "let not implemented";
+                                break;
+                            case "async-functions":
+                                skip = true;
+                                reason = "async-functions not implemented";
+                                break;
                         }
                     }
                 }

Jint.Tests.Test262/test/skipped.json

@@ -289,5 +289,57 @@
   {
     "source": "built-ins/StringIteratorPrototype/next/next-iteration-surrogate-pairs.js",
     "reason": "code point iteration not implemented"
+  },
+  {
+    "source": "language/statements/for-in/head-const-bound-names-fordecl-tdz.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for-in/head-const-fresh-binding-per-iteration.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for-in/head-decl-expr.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for-in/head-let-bound-names-fordecl-tdz.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for-in/head-let-destructuring.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for-in/head-let-fresh-binding-per-iteration.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for-in/head-lhs-let.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for-in/scope-head-var-none.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for/head-const-fresh-binding-per-iteration.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for/head-let-destructuring.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for/head-let-fresh-binding-per-iteration.js",
+    "reason": "let/const not implemented"
+  },
+  {
+    "source": "language/statements/for/head-lhs-let.js",
+    "reason": "Esprima edge case"
+  },
+  {
+    "source": "language/statements/for-in/head-var-bound-names-dup.js",
+    "reason": "destructing not implemented"
   }
 ]

Jint.Tests/Runtime/EngineTests.cs

@@ -108,6 +108,22 @@ namespace Jint.Tests.Runtime
             Assert.Equal(expected, result);
         }
 
+        [Fact]
+        public void ShouldHaveProperReferenceErrorMessage()
+        {
+            RunTest(@"
+                'use strict';
+                var arr = [1, 2];
+                try {
+                    for (i in arr) { }
+                    assert(false);
+                }
+                catch (ex) {
+                    assert(ex.message === 'i is not defined');
+                }
+            ");
+        }
+
         [Fact]
         public void ShouldEvaluateHasOwnProperty()
         {

Jint/Engine.cs

@@ -523,7 +523,7 @@ namespace Jint
                     return val;
                 }
 
-                ExceptionHelper.ThrowReferenceError(this, reference.GetReferencedName() + " is not defined");
+                ExceptionHelper.ThrowReferenceError(this, reference);
             }
 
             var baseValue = reference._baseValue;
@@ -597,7 +597,7 @@ namespace Jint
             {
                 if (reference._strict)
                 {
-                    ExceptionHelper.ThrowReferenceError(this);
+                    ExceptionHelper.ThrowReferenceError(this, reference);
                 }
 
                 Global.Put(reference._name, value, false);

Jint/Native/Function/EvalFunctionInstance.cs

@@ -102,7 +102,7 @@ namespace Jint.Native.Function
             {
                 if (e.Description == Messages.InvalidLHSInAssignment)
                 {
-                    ExceptionHelper.ThrowReferenceError(_engine);
+                    ExceptionHelper.ThrowReferenceError(_engine, (string) null);
                 }
 
                 ExceptionHelper.ThrowSyntaxError(_engine);

Jint/Runtime/Environments/DeclarativeEnvironmentRecord.cs

@@ -185,7 +185,7 @@ namespace Jint.Runtime.Environments
             {
                 if (strict)
                 {
-                    ExceptionHelper.ThrowReferenceError(_engine, "Can't access an uninitialized immutable binding.");
+                    ThrowUninitializedBindingException();
                 }
 
                 return Undefined;
@@ -194,6 +194,11 @@ namespace Jint.Runtime.Environments
             return binding.Value;
         }
 
+        private void ThrowUninitializedBindingException()
+        {
+            throw new JavaScriptException(_engine.ReferenceError, "Can't access an uninitialized immutable binding.");
+        }
+
         public override bool DeleteBinding(string name)
         {
             ref Binding binding = ref GetExistingItem(name);

Jint/Runtime/Environments/ObjectEnvironmentRecord.cs

@@ -66,7 +66,7 @@ namespace Jint.Runtime.Environments
             var desc = _bindingObject.GetProperty(name);
             if (strict && desc == PropertyDescriptor.Undefined)
             {
-                ExceptionHelper.ThrowReferenceError(_engine);
+                ExceptionHelper.ThrowReferenceError(_engine, name);
             }
 
             return ObjectInstance.UnwrapJsValue(desc, this);

Jint/Runtime/ExceptionHelper.cs

@@ -1,5 +1,6 @@
 using System;
 using Jint.Runtime.CallStack;
+using Jint.Runtime.References;
 
 namespace Jint.Runtime
 {
@@ -32,8 +33,14 @@ namespace Jint.Runtime
             throw new ArgumentException(message, paramName);
         }
 
-        public static void ThrowReferenceError(Engine engine, string message = null)
+        public static void ThrowReferenceError(Engine engine, Reference reference)
         {
+            ThrowReferenceError(engine, reference?.GetReferencedName());
+        }
+
+        public static void ThrowReferenceError(Engine engine, string name)
+        {
+            var message = name != null ? name + " is not defined" : null;
             throw new JavaScriptException(engine.ReferenceError, message);
         }
 

Jint/Runtime/Interpreter/Statements/JintForInStatement.cs

@@ -8,19 +8,28 @@ using Jint.Runtime.References;
 namespace Jint.Runtime.Interpreter.Statements
 {
     /// <summary>
-    /// http://www.ecma-international.org/ecma-262/5.1/#sec-12.6.4
+    ///     http://www.ecma-international.org/ecma-262/5.1/#sec-12.6.4
     /// </summary>
     internal sealed class JintForInStatement : JintStatement<ForInStatement>
     {
-        private readonly JintExpression _identifier;
         private readonly JintStatement _body;
+        private readonly JintExpression _identifier;
         private readonly JintExpression _right;
 
         public JintForInStatement(Engine engine, ForInStatement statement) : base(engine, statement)
         {
-            _identifier = JintExpression.Build(engine, _statement.Left.Type == Nodes.VariableDeclaration
-                ? (Identifier) ((VariableDeclaration) _statement.Left).Declarations[0].Id
-                : (Identifier) _statement.Left);
+            if (_statement.Left.Type == Nodes.VariableDeclaration)
+            {
+                _identifier = JintExpression.Build(engine, (Identifier) ((VariableDeclaration) _statement.Left).Declarations[0].Id);
+            }
+            else if (_statement.Left.Type == Nodes.MemberExpression)
+            {
+                _identifier = JintExpression.Build(engine, ((MemberExpression) _statement.Left));
+            }
+            else
+            {
+                _identifier = JintExpression.Build(engine, (Identifier) _statement.Left);
+            }
 
             _body = Build(engine, _statement.Body);
             _right = JintExpression.Build(engine, statement.Right);
@@ -36,7 +45,7 @@ namespace Jint.Runtime.Interpreter.Statements
             }
 
             var obj = TypeConverter.ToObject(_engine, experValue);
-            JsValue v = Null.Instance;
+            var v = Undefined.Instance;
 
             // keys are constructed using the prototype chain
             var cursor = obj;

Jint/Runtime/Interpreter/Statements/JintForStatement.cs

@@ -67,7 +67,7 @@ namespace Jint.Runtime.Interpreter.Statements
                 var stmtType = stmt.Type;
                 if (stmtType == CompletionType.Break && (stmt.Identifier == null || stmt.Identifier == _statement?.LabelSet?.Name))
                 {
-                    return new Completion(CompletionType.Normal, v, null);
+                    return new Completion(CompletionType.Normal, stmt.Value, null);
                 }
 
                 if (stmtType != CompletionType.Continue || ((stmt.Identifier != null) && stmt.Identifier != _statement?.LabelSet?.Name))