[MERGE #379] Few destructuring fixes

Merge pull request #379 from akroshg:destruct3
1. Object pattern can now support the initializer on the short-hand. ({x = 1} = {}); Thing to note that object literal cannot allow the initializer to shorthand but that object literal can be object pattern so we need to defer the error until we are sure that the object literal is not the object pattern. Current we are giving the favor to object pattern and defer the error. If the object literal on the LHS, the error will be ignored. Look at test for more scenarios.
2. bug fix: rest pattern cannot have initializer. We have done that for rest identifier but the rest pattern was missing (as the rest pattern was itself the late addition to the spec).
3. The spec says that for object pattern assignment - the AssignmentExpression must be object coercible. So {} = undefined/null; should not be allowed (raise TypeError). Fixed that by adding the bytecodes which is essentially checking the assignment expression with undefined or null.

lib/Parser/Parse.cpp

@@ -123,7 +123,7 @@ Parser::Parser(Js::ScriptContext* scriptContext, BOOL strictMode, PageAllocator
     m_parseType = ParseType_Upfront;
 
     m_deferEllipsisError = false;
-
+    m_hasDeferredShorthandInitError = false;
     m_parsingSuperRestrictionState = ParsingSuperRestrictionState_SuperDisallowed;
 }
 
@@ -3766,7 +3766,7 @@ Parse a list of object members. e.g. { x:foo, 'y me':bar }
 template<bool buildAST>
 ParseNodePtr Parser::ParseMemberList(LPCOLESTR pNameHint, ulong* pNameHintLength, tokens declarationType)
 {
-    ParseNodePtr pnodeArg;
+    ParseNodePtr pnodeArg = nullptr;
     ParseNodePtr pnodeName = nullptr;
     ParseNodePtr pnodeList = nullptr;
     ParseNodePtr *lastNodeRef = nullptr;
@@ -3786,6 +3786,8 @@ ParseNodePtr Parser::ParseMemberList(LPCOLESTR pNameHint, ulong* pNameHintLength
 
     ArenaAllocator tempAllocator(L"MemberNames", m_nodeAllocator.GetPageAllocator(), Parser::OutOfMemory);
 
+    bool hasDeferredInitError = false;
+
     for (;;)
     {
         bool isComputedName = false;
@@ -4044,7 +4046,7 @@ ParseNodePtr Parser::ParseMemberList(LPCOLESTR pNameHint, ulong* pNameHintLength
                     }
                 }
             }
-            else if ((m_token.tk == tkRCurly || m_token.tk == tkComma || (isObjectPattern && m_token.tk == tkAsg)) && m_scriptContext->GetConfig()->IsES6ObjectLiteralsEnabled())
+            else if ((m_token.tk == tkRCurly || m_token.tk == tkComma || m_token.tk == tkAsg) && m_scriptContext->GetConfig()->IsES6ObjectLiteralsEnabled())
             {
                 // Shorthand {foo} -> {foo:foo} syntax.
                 // {foo = <initializer>} supported only when on object pattern rules are being applied
@@ -4065,6 +4067,17 @@ ParseNodePtr Parser::ParseMemberList(LPCOLESTR pNameHint, ulong* pNameHintLength
                     CheckArgumentsUse(pidHint, GetCurrentFunctionNode());
                 }
 
+                bool couldBeObjectPattern = !isObjectPattern && m_token.tk == tkAsg;
+
+                if (couldBeObjectPattern)
+                {
+                    declarationType = tkLCurly;
+                    isObjectPattern = true;
+
+                    // This may be an error but we are deferring for favouring destructuring.
+                    hasDeferredInitError = true;
+                }
+
                 ParseNodePtr pnodeIdent = nullptr;
                 if (isObjectPattern)
                 {
@@ -4090,7 +4103,7 @@ ParseNodePtr Parser::ParseMemberList(LPCOLESTR pNameHint, ulong* pNameHintLength
                         pnodeIdent->sxPid.SetSymRef(ref);
                     }
 
-                    pnodeArg = CreateBinNode(isObjectPattern ? knopObjectPatternMember : knopMemberShort, pnodeName, pnodeIdent);
+                    pnodeArg = CreateBinNode(isObjectPattern && !couldBeObjectPattern ? knopObjectPatternMember : knopMemberShort, pnodeName, pnodeIdent);
                 }
             }
             else
@@ -4128,6 +4141,8 @@ ParseNodePtr Parser::ParseMemberList(LPCOLESTR pNameHint, ulong* pNameHintLength
         }
     }
 
+    m_hasDeferredShorthandInitError = m_hasDeferredShorthandInitError || hasDeferredInitError;
+
     if (buildAST)
     {
         AssertMem(lastNodeRef);
@@ -7698,6 +7713,10 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
 
     m_pscan->Capture(&termStart);
 
+    bool deferredErrorFoundOnLeftSide = false;
+    bool savedDeferredInitError = m_hasDeferredShorthandInitError;
+    m_hasDeferredShorthandInitError = false;
+
     // Is the current token a unary operator?
     if (m_phtbl->TokIsUnop(m_token.tk, &opl, &nop) && nop != knopNone)
     {
@@ -7876,6 +7895,9 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
             {
                 pnode = ConvertToPattern(pnode);
             }
+
+            // The left-hand side is found to be destructuring pattern - so the shorthand can have initializer.
+            m_hasDeferredShorthandInitError = false;
         }
 
         if (buildAST)
@@ -7944,6 +7966,8 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
         }
     }
 
+    deferredErrorFoundOnLeftSide = m_hasDeferredShorthandInitError;
+
     // Process a sequence of operators and operands.
     for (;;)
     {
@@ -8124,6 +8148,15 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
         }
     }
 
+    if (m_hasDeferredShorthandInitError && !deferredErrorFoundOnLeftSide)
+    {
+        // Raise error only if it is found not on the right side of the expression.
+        // such as  <expr> = {x = 1}
+        Error(ERRnoColon);
+    }
+
+    m_hasDeferredShorthandInitError = m_hasDeferredShorthandInitError || savedDeferredInitError;
+
     if (NULL != pfCanAssign)
     {
         *pfCanAssign = fCanAssign;
@@ -9757,6 +9790,11 @@ LDefaultToken:
         IdentToken tok;
         pnode = ParseExpr<buildAST>(koplNo, nullptr, TRUE, FALSE, nullptr, nullptr /*hintLength*/, nullptr /*hintOffset*/, &tok);
 
+        if (m_hasDeferredShorthandInitError)
+        {
+            Error(ERRnoColon);
+        }
+
         if (buildAST)
         {
             // Check for a label.
@@ -11560,6 +11598,11 @@ ParseNodePtr Parser::GetRightSideNodeFromPattern(ParseNodePtr pnode)
 
 ParseNodePtr Parser::ConvertMemberToMemberPattern(ParseNodePtr pnodeMember)
 {
+    if (pnodeMember->nop == knopObjectPatternMember)
+    {
+        return pnodeMember;
+    }
+
     Assert(pnodeMember->nop == knopMember || pnodeMember->nop == knopMemberShort);
 
     ParseNodePtr rightNode = GetRightSideNodeFromPattern(pnodeMember->sxBin.pnode2);
@@ -11615,6 +11658,9 @@ void Parser::ParseDestructuredLiteralWithScopeSave(tokens declarationType,
     charcount_t funcInArraySave = m_funcInArray;
     uint funcInArrayDepthSave = m_funcInArrayDepth;
 
+    // we need to reset this as we are going to parse the grammar again.
+    m_hasDeferredShorthandInitError = false;
+
     ParseDestructuredLiteral<false>(declarationType, isDecl, topLevel, initializerContext, allowIn);
 
     m_currentNodeFunc = pnodeFncSave;
@@ -11691,7 +11737,16 @@ ParseNodePtr Parser::ParseDestructuredInitializer(ParseNodePtr lhsNode,
 
     m_pscan->Scan();
 
+
+    bool alreadyHasInitError = m_hasDeferredShorthandInitError;
+
     ParseNodePtr pnodeDefault = ParseExpr<buildAST>(koplCma, nullptr, allowIn);
+
+    if (m_hasDeferredShorthandInitError && !alreadyHasInitError)
+    {
+        Error(ERRnoColon);
+    }
+
     ParseNodePtr pnodeDestructAsg = nullptr;
     if (buildAST)
     {
@@ -11772,7 +11827,7 @@ ParseNodePtr Parser::ParseDestructuredVarDecl(tokens declarationType, bool isDec
     if (IsPossiblePatternStart())
     {
         // Go recursively
-        pnodeElem = ParseDestructuredLiteral<buildAST>(declarationType, isDecl, false /*topLevel*/);
+        pnodeElem = ParseDestructuredLiteral<buildAST>(declarationType, isDecl, false /*topLevel*/, seenRest ? DIC_ShouldNotParseInitializer : DIC_None);
     }
     else if (m_token.tk == tkSUPER || m_token.tk == tkID)
     {
@@ -11845,8 +11900,14 @@ ParseNodePtr Parser::ParseDestructuredVarDecl(tokens declarationType, bool isDec
         }
         m_pscan->Scan();
 
+        bool alreadyHasInitError = m_hasDeferredShorthandInitError;
         ParseNodePtr pnodeInit = ParseExpr<buildAST>(koplCma);
 
+        if (m_hasDeferredShorthandInitError && !alreadyHasInitError)
+        {
+            Error(ERRnoColon);
+        }
+
         if (buildAST)
         {
             pnodeElem = CreateBinNode(knopAsg, pnodeElem, pnodeInit);

lib/Parser/Parse.h

@@ -366,6 +366,9 @@ private:
     ParseNodePtr * m_ppnodeVar;  // variable list tail
     bool m_inDeferredNestedFunc; // true if parsing a function in deferred mode, nested within the current node
     bool m_isInBackground;
+
+    // This bool is used for deferring the shorthand initializer error ( {x = 1}) - as it is allowed in the destructuring grammar.
+    bool m_hasDeferredShorthandInitError;
     uint * m_pnestedCount; // count of functions nested at one level below the current node
 
     struct WellKnownPropertyPids

lib/Parser/rterrors.h

@@ -338,3 +338,4 @@ RT_ERROR_MSG(JSERR_RegExpExecInvalidReturnType, 5641, "%s: Return value of RegEx
 RT_ERROR_MSG(JSERR_ProxyTrapReturnedFalse, 5642, "Proxy trap `%s` returned false", "Proxy trap returned false", kjstTypeError, 0)
 RT_ERROR_MSG(JSERR_ModuleResolveExport, 5643, "Module export %s cannot be resolved", "Module export cannot be resolved", kjstSyntaxError, 0)
 RT_ERROR_MSG(JSERR_TooManyImportExprots, 5644, "Module has too many import/export definitions", "Module has too many import/export definitions", kjstRangeError, 0)
+RT_ERROR_MSG(JSERR_ObjectCoercible, 5645, "", "Cannot convert null or undefined to object", kjstTypeError, 0)

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

@@ -6188,12 +6188,18 @@ void EmitDestructuredObject(ParseNode *lhs,
 {
     Assert(lhs->nop == knopObjectPattern);
     ParseNodePtr pnode1 = lhs->sxUni.pnode1;
+
+    byteCodeGenerator->StartStatement(lhs);
+
+    Js::ByteCodeLabel skipThrow = byteCodeGenerator->Writer()->DefineLabel();
+    byteCodeGenerator->Writer()->BrReg2(Js::OpCode::BrNeq_A, skipThrow, rhsLocation, funcInfo->undefinedConstantRegister);
+    byteCodeGenerator->Writer()->W1(Js::OpCode::RuntimeTypeError, SCODE_CODE(JSERR_ObjectCoercible));
+    byteCodeGenerator->Writer()->MarkLabel(skipThrow);
+
     if (pnode1 != nullptr)
     {
         Assert(pnode1->nop == knopList || pnode1->nop == knopObjectPatternMember);
 
-        byteCodeGenerator->StartStatement(lhs);
-
         ParseNodePtr current = pnode1;
         while (current->nop == knopList)
         {
@@ -6202,9 +6208,9 @@ void EmitDestructuredObject(ParseNode *lhs,
             current = current->sxBin.pnode2;
         }
         EmitDestructuredObjectMember(current, rhsLocation, byteCodeGenerator, funcInfo);
-
-        byteCodeGenerator->EndStatement(lhs);
     }
+
+    byteCodeGenerator->EndStatement(lhs);
 }
 
 void EmitAssignment(

test/es6/destructuring.js

@@ -66,8 +66,8 @@ var tests = [
       assert.throws(function () { eval("let a; [++a] = [];"); },     SyntaxError, "Destructured let array assignment with an operator throws",    "Unexpected operator in destructuring expression");
       assert.doesNotThrow(function () { eval("var a = [1], i = 0; [a[i++]] = [];"); }, "Destructured var array assignment operators inside an identifier reference does not throw");
       assert.doesNotThrow(function () { eval("let a = [1], i = 0; [a[i++]] = [];"); }, "Destructured var array assignment operators inside an identifier reference does not throw");
-      //assert.doesNotThrow(function () { eval("var a = [1], i = 0; [a[(() => 1 + i)()]] = [];"); }, "Destructured var array assignment operators inside an identifier reference does not throw");
-      //assert.doesNotThrow(function () { eval("let a = [1], i = 0; [a[(() => 1 + i)()]] = [];"); }, "Destructured var array assignment operators inside an identifier reference does not throw");
+      assert.doesNotThrow(function () { eval("var a = [1], i = 0; [a[(() => 1 + i)()]] = [];"); }, "Destructured var array assignment operators inside an identifier reference does not throw");
+      assert.doesNotThrow(function () { eval("let a = [1], i = 0; [a[(() => 1 + i)()]] = [];"); }, "Destructured var array assignment operators inside an identifier reference does not throw");
 
       // Missing values
       assert.doesNotThrow(function () { eval("var [,] = [];"); },   "Destructured var array declaration with no identifiers and missing values does not throw");
@@ -202,14 +202,6 @@ var tests = [
       assert.doesNotThrow(function () { eval("var a, b; [[(a)], ((((((([b])))))))] = [[],[]];"); }, "Destructured var array assignment with valid mixed paren and array nesting does not throw");
       assert.doesNotThrow(function () { eval("let a, b; [[(a)], ((((((([b])))))))] = [[],[]];"); }, "Destructured let array assignment with valid mixed paren and array nesting does not throw");
 
-      // All the things
-      // Currently disabled until default scoping is implemented.
-//      assert.doesNotThrow(function () { eval("var [a = () => 1, [b, [c, d] = [1, 2], ([e])]] = [,[,,[]]];"); },                "Destructured var array declaration with mixed valid syntax does not throw");
-//      assert.doesNotThrow(function () { eval("let [a = () => 1, [b, [c, d] = [1, 2], ([e])]] = [,[,,[]]];"); },                "Destructured let array declaration with mixed valid syntax does not throw");
-//      assert.doesNotThrow(function () { eval("const [a = () => 1, [b, [c, d] = [1, 2], ([e])]] = [,[,,[]]];"); },              "Destructured const array declaration with mixed valid syntax does not throw");
-//      assert.doesNotThrow(function () { eval("var a, b, c, d, e; [a = () => 1, [b, [c, d] = [1, 2], ([e])]] = [,[,,[]]];"); }, "Destructured var array declaration with mixed valid syntax does not throw");
-//      assert.doesNotThrow(function () { eval("let a, b, c, d, e; [a = () => 1, [b, [c, d] = [1, 2], ([e])]] = [,[,,[]]];"); }, "Destructured let array declaration with mixed valid syntax does not throw");
-
       // Redeclarations
       assert.doesNotThrow(function () { eval("var [a, a] = [];"); },    "Destructured var array declaration with a repeated identifier reference does not throw");
       assert.throws(function () { eval("let [a, a] = [];"); },   SyntaxError, "Destructured let array declaration with a repeated identifier reference throws", "Let/Const redeclaration");
@@ -245,32 +237,9 @@ var tests = [
       assert.doesNotThrow(function () { eval("class foo { method() { [super.x] = []; } }"); },      "Destructured var array assignment with super a property reference does not throw");
       assert.doesNotThrow(function () { eval("class foo { method() { [super[\"x\"]] = []; } }"); }, "Destructured array assignment with a super property reference does not throw");
 
-      // for in/of
-      // Not yet implemented.
-//      assert.doesNotThrow(function () { eval("for (var [a] in [1]) { }"); },    "Destructured var array declaration in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("for (let [a] in [1]) { }"); },    "Destructured let array declaration in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("for (const [a] in [1]) { }"); },  "Destructured const array declaration in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("var a; for ([a] in [1]) { }"); }, "Destructured var array assignment in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("let a; for ([a] in [1]) { }"); }, "Destructured let array assignment in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("for (var [a] of [1]) { }"); },    "Destructured var array declaration in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("for (let [a] of [1]) { }"); },    "Destructured let array declaration in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("for (const [a] of [1]) { }"); },  "Destructured const array declaration in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("var a; for ([a] of [1]) { }"); }, "Destructured var array assignment in a for in statement does not throw");
-//      assert.doesNotThrow(function () { eval("let a; for ([a] of [1]) { }"); }, "Destructured let array assignment in a for in statement does not throw");
-
       // Destructured array expressions used in other constructs
       assert.doesNotThrow(function () { eval("var a; `${[a] = [1]}`"); }, "Destructured assignment does not throw inside a string template");
 
-     /* Function and Catch bindings - not yet implemented
-      assert.doesNotThrow(function () { eval("try { } catch ([a]) { } finally { }"); }, "x = function() { try { } catch ([...a]) { } finally { } }; // ");
-      assert.throws(function () { eval("try { } catch (...a) { } finally { }"); }, SyntaxError, ""); // error
-      assert.throws(function () { eval("try { } catch ([a], b) { } finally { }"); }, SyntaxError, ""); // error
-
-      assert.doesNotThrow(function () { eval("function foo([a]) {}"); }, "x = function() { var obj = { method([a]) {} } }; // ");
-      assert.doesNotThrow(function () { eval("class { method([a]) {} }"); }, "x = function() { ([a]) => a }; // ");
-      assert.throws(function () { eval("[a] => a"); }, SyntaxError, ""); // error
-      */
-
       // OS 597319: Parens before a default value should not throw
       assert.doesNotThrow(function () { eval("[((((vrh190 )))) = 5184] = []"); }, "Destructured array assignment with parens before a default expression should not throw");
     }

test/es6/destructuring_bugs.js

@@ -62,6 +62,17 @@ var tests = [
       assert.throws(function () { eval("for (var a in {b: foo().bar()} = {}) { }"); }, SyntaxError, "for.in loop has destructuring pattern which has linked call expression instead of a reference node", "Invalid destructuring assignment target");
       assert.doesNotThrow(function () { eval("for (var a in {b: foo().bar} = {}) { }"); }, "for.in loop has destructuring pattern which has a reference node is valid syntax", );
     }
+  },
+  {
+    name: "Destructuring bug fix - object coercible",
+    body: function () {
+      assert.throws(function () { eval("var {} = undefined"); }, TypeError, "Object declaration - RHS cannot be be undefined", "Cannot convert null or undefined to object");
+      assert.throws(function () { eval("var {} = null"); }, TypeError, "Object declaration - RHS cannot be be null", "Cannot convert null or undefined to object");
+      assert.throws(function () { eval("({} = undefined);"); }, TypeError, "Object assignment pattern - RHS cannot be be undefined", "Cannot convert null or undefined to object");
+      assert.throws(function () { eval("([{}] = []);"); }, TypeError, "Object assignment pattern nested with array pattern has evaluated to have undefined as RHS", "Cannot convert null or undefined to object");
+      assert.throws(function () { eval("function f({}){}; f();"); }, TypeError, "Object pattern on function - evaluated to have undefined from assignment expression", "Cannot convert null or undefined to object");
+      assert.throws(function () { eval("function f({}){}; f(null);"); }, TypeError, "Object pattern on function - evaluated to have null from assignment expression", "Cannot convert null or undefined to object");
+    }
   }
 ];
 

test/es6/destructuring_obj.js

@@ -117,6 +117,10 @@ var tests = [
 
       assert.throws(function () { eval("let a; [...[a+1] = [{}];"); }, SyntaxError, "Under expression, having rest element as pattern which has operator is not valid syntax",   "Unexpected operator in destructuring expression");
       assert.throws(function () { eval("function foo() { return {x:1}; }; [...foo()] = [10];"); }, SyntaxError, "Under expression, having rest element as call expression is not valid syntax", "Invalid destructuring assignment target");
+      assert.throws(function () { eval("let [...[a] = []] = [[]];"); }, SyntaxError, "Under declaration - rest as array pattern cannot have initializer", "The rest parameter cannot have a default initializer.");
+      assert.throws(function () { eval("let [...{x} = {}] = [{}];"); }, SyntaxError, "Under declaration - rest as object pattern cannot have initializer", "The rest parameter cannot have a default initializer.");
+      assert.throws(function () { eval("let a; ([...[a] = []] = [[]]);"); }, SyntaxError, "Under assignment - rest as array pattern cannot have initializer", "The rest parameter cannot have a default initializer.");
+      assert.throws(function () { eval("let x; ([...{x} = {}] = [{}]);"); }, SyntaxError, "Under assignment - rest as object pattern cannot have initializer", "The rest parameter cannot have a default initializer.");
     }
    },
    {
@@ -182,6 +186,28 @@ var tests = [
         assert.areEqual(4, y, "`set` is a valid object destructuring name mapping");
     }
   },
+  {
+    name: "Object destructuring with shorthand initializer",
+    body: function () {
+        assert.doesNotThrow(function () { eval("({x = 1} = {});"); }, "Object pattern has shorthand with initializer is a valid syntax");
+        assert.doesNotThrow(function () { eval("({x, y = 1, z = 2} = {});"); }, "Object pattern has multiple shorthands with initializer is a valid syntax");
+        assert.throws(function () { eval("var a = 1; ({x, y = 1, z = 2} = {a = 2});"); }, SyntaxError,"Initializer is allowed on shorthand of object pattern but not on object literal", "Expected ':'");
+        assert.throws(function () { eval("var a = 1; ({x, y = {a = 1}} = {});"); }, SyntaxError,"Object literal is within object pattern but has shorthand initializer is not valid syntax", "Expected ':'");
+        assert.doesNotThrow(function () { eval("var a = 1; ({x = {a = 1} = {}} = {});"); }, "Chained object patterns have shorthand initializers is a valid syntax");
+        assert.doesNotThrow(function () { eval("var a = 1; var {x = {a = 1} = {}} = {};"); }, "Chained object declaration pattern have shorthand initializers is a valid syntax");
+        assert.doesNotThrow(function () { eval("[{x : [{y:{z = 1}}] }] = [{x:[{y:{}}]}];"); }, "Mixed nesting pattern has shorthand initializer is a valid syntax");
+        assert.doesNotThrow(function () { eval("[{x : [{y:{z = 1}, z1 = 2}] }, {x2 = 3}, {x3 : {y3:[{z3 = 4}]}} ] = [{x:[{y:{}}]}, {}, {x3:{y3:[{}]}}];"); }, 
+                                            "Mixed object patterns both on nested and on same level have initializers on shorthand and is a valid syntax");
+        assert.doesNotThrow(function () { eval("var [{x : [{y:{z = 1}, z1 = 2}] }, {x2 = 3}, {x3 : {y3:[{z3 = 4}]}} ] = [{x:[{y:{}}]}, {}, {x3:{y3:[{}]}}];"); }, 
+                                            "Declaration - mixed object patterns both on nested and on same level have initializers on shorthand and is a valid syntax");
+        assert.doesNotThrow(function () { eval("[...{x = 1}] = [{}]"); }, "Object pattern following rest has shorthand initializer is a valid syntax");
+        {
+            let a1 = 1;
+            ({x:{a1 = 2}} = {x:{}});
+            assert.areEqual(a1, 2);
+        }
+    }
+  },
   {
     name: "Object destructuring basic functionality",
     body: function () {