Adding syntax errors for duplicate parameters scenarios,

1. If the source code matching this production is strict code, the EarlyError rules for StrictFormalParameters:FormalParameters are applied. 2. It is a Syntax Error if ContainsUseStrict of FunctionBody is true and IsSimpleParameterList of FormalParameters is false. 3. It is a Syntax Error if IsSimpleParameterList of FormalParameterList is false and BoundNames of FormalParameterList contains any duplicate elements.
chakra-core · Jan 19, 2016 · d8bef2e · d8bef2e
1 parent a0f58b0
commit d8bef2e
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 12 deletions.
diff --git a/lib/Parser/parse.cpp b/lib/Parser/parse.cpp
@@ -899,6 +899,13 @@ Symbol* Parser::AddDeclForPid(ParseNodePtr pnode, IdentPtr pid, SymbolType symbo
             }
             break;
         case knopVarDecl:
+            if (m_currentScope->GetScopeType() == ScopeType_Parameter)
+            {
+                // If this is a parameter list, mark the scope to indicate that it has duplicate definition.
+                // If later this turns out to be a non-simple param list (like function f(a, a, c = 1) {}) then it is a SyntaxError to have duplicate formals.
+                m_currentScope->SetHasDuplicateFormals();
+            }
+
             if (sym->GetDecl() == nullptr)
             {
                 Assert(symbolType == STFunction);
@@ -5363,10 +5370,8 @@ void Parser::ParseFncFormals(ParseNodePtr pnodeFnc, ushort flags)
                             MapFormalsWithoutRest(m_currentNodeFunc, [&](ParseNodePtr pnodeArg) { pnodeArg->sxVar.sym->SetIsNonSimpleParameter(true); });
                         }
                     }
-                    else
-                    {
-                        isNonSimpleParameterList = true;
-                    }
+
+                    isNonSimpleParameterList = true;
                 }
                 else
                 {
@@ -5432,13 +5437,19 @@ void Parser::ParseFncFormals(ParseNodePtr pnodeFnc, ushort flags)
                         }
                     }
 
+                    // In defer parse mode we have to flag the function node to indicate that it has default arguments
+                    // so that it will be considered for any syntax error scenario.
+                    ParseNode* currentFncNode = GetCurrentFunctionNode();
+                    if (!currentFncNode->sxFnc.HasDefaultArguments())
+                    {
+                        currentFncNode->sxFnc.SetHasDefaultArguments();
+                        currentFncNode->sxFnc.firstDefaultArg = argPos;
+                    }
 
                     if (buildAST)
                     {
                         if (!m_currentNodeFunc->sxFnc.HasDefaultArguments())
                         {
-                            m_currentNodeFunc->sxFnc.SetHasDefaultArguments();
-                            m_currentNodeFunc->sxFnc.firstDefaultArg = argPos;
                             CHAKRATEL_LANGSTATS_INC_LANGFEATURECOUNT(DefaultArgFunctionCount, m_scriptContext);
                         }
                         pnodeT->sxVar.pnodeInit = pnodeInit;
@@ -5447,6 +5458,11 @@ void Parser::ParseFncFormals(ParseNodePtr pnodeFnc, ushort flags)
                 }
             }
 
+            if (isNonSimpleParameterList && m_currentScope->GetHasDuplicateFormals())
+            {
+                Error(ERRFormalSame);
+            }
+
             if (m_token.tk != tkComma)
             {
                 break;
@@ -9484,6 +9500,12 @@ void Parser::ParseStmtList(ParseNodePtr *ppnodeList, ParseNodePtr **pppnodeLast,
             {
                 if (isUseStrictDirective)
                 {
+                    // Functions with non-simple parameter list cannot be made strict mode
+                    if (!GetCurrentFunctionNode()->sxFnc.IsSimpleParameterList())
+                    {
+                        Error(ERRNonSimpleParamListInStrictMode);
+                    }
+
                     if (seenDirectiveContainingOctal)
                     {
                         // Directives seen before a "use strict" cannot contain an octal.

diff --git a/lib/Parser/perrors.h b/lib/Parser/perrors.h
@@ -87,3 +87,4 @@ LSC_ERROR_MSG( 1077, ERRDestructNotInit, "Destructuring declarations cannot have
 LSC_ERROR_MSG(1079, ERRInvalidNewTarget, "Invalid use of the 'new.target' keyword")
 LSC_ERROR_MSG(1080, ERRForInNoInitAllowed, "for-in loop head declarations cannot have an initializer")
 LSC_ERROR_MSG(1081, ERRForOfNoInitAllowed, "for-of loop head declarations cannot have an initializer")
+LSC_ERROR_MSG(1082, ERRNonSimpleParamListInStrictMode, "Cannot apply strict mode on functions with non-simple parameter list")
diff --git a/lib/Runtime/ByteCode/Scope.h b/lib/Runtime/ByteCode/Scope.h
@@ -36,6 +36,7 @@ class Scope
     BYTE capturesAll : 1;
     BYTE mustInstantiate : 1;
     BYTE hasCrossScopeFuncAssignment : 1;
+    BYTE hasDuplicateFormals : 1;
 public:
 #if DBG
     BYTE isRestored : 1;
@@ -50,6 +51,7 @@ class Scope
         capturesAll(false),
         mustInstantiate(false),
         hasCrossScopeFuncAssignment(false),
+        hasDuplicateFormals(false),
         location(Js::Constants::NoRegister),
         symbolTable(nullptr),
         m_symList(nullptr),
@@ -280,6 +282,9 @@ class Scope
     void SetScopeSlotCount(uint i) { scopeSlotCount = i; }
     uint GetScopeSlotCount() const { return scopeSlotCount; }
 
+    void SetHasDuplicateFormals() { hasDuplicateFormals = true; }
+    bool GetHasDuplicateFormals() { return hasDuplicateFormals; }
+
     void SetHasLocalInClosure(bool has);
 
     bool HasInnerScopeIndex() const { return innerScopeIndex != (uint)-1; }

diff --git a/test/es6/default.js b/test/es6/default.js
@@ -20,7 +20,18 @@ var tests = [
       assert.throws(function () { eval("var x = function*(a =) { return a; }"); },          SyntaxError, "Incomplete default expression throws in a generator function",      "Syntax error");
       assert.throws(function () { eval("var x = class { * foo(a =) { return a; } }"); },    SyntaxError, "Incomplete default expression throws in a class generator method",  "Syntax error");
 
-      assert.throws(function () { eval("function foo(a *= 5)"); }, SyntaxError, "Other assignment operators do not work");
+      // Duplicate parameters
+      assert.throws(function () { eval("function f(a, b, a, c = 10) { }"); },               SyntaxError, "Duplicate parameters are not allowed before the default argument", "Duplicate formal parameter names not allowed in this context");
+      assert.throws(function () { eval("function f(a, b = 10, a) { }"); },                  SyntaxError, "Duplicate parameters are not allolwed after the default argument", "Duplicate formal parameter names not allowed in this context");
+      assert.throws(function () { eval("function f(a, b, a, c) { \"use strict\"; }"); },    SyntaxError, "When function is in strict mode duplicate parameters are not allowed for simple parameter list", "Duplicate formal parameter names not allowed in strict mode");
+      assert.throws(function () { eval("function f(a, b = 1) { \"use strict\"; }"); },      SyntaxError, "Strict mode cannot be applied to functions with default parameters", "Cannot apply strict mode on functions with non-simple parameter list");
+      assert.throws(function () { eval("function f() { \"use strict\"; function g(a, b, a) { } }"); },      SyntaxError, "When a function is already in strict mode duplicate parameters are not allowed for simple parameter list", "Duplicate formal parameter names not allowed in strict mode");
+      assert.throws(function () { eval("function f() { \"use strict\"; function g(a, b, a = 10) { } }"); }, SyntaxError, "When a function is already in strict mode duplicate parameters are not allowed for formal parameter list", "Duplicate formal parameter names not allowed in strict mode");
+
+      assert.doesNotThrow(function f() { "use strict"; function g(a, b = 10) { } },           "Default arguments are allowed for functions which are already in strict mode");
+      assert.doesNotThrow(function f(a, b, a, c) { return a + b + c; },                       "In non-strict mode duplicate parameters are allowed");
+
+      assert.throws(function () { eval("function foo(a *= 5)"); },                          SyntaxError, "Other assignment operators do not work");
 
       // Redeclaration errors - non-simple in this case means any parameter list with a default expression
       assert.doesNotThrow(function () { eval("function foo(a = 1) { var a; }"); },            "Var redeclaration with a non-simple parameter list");
@@ -35,10 +46,6 @@ var tests = [
 
       assert.throws(function () { eval("x = 3 => x"); },                                    SyntaxError, "Lambda formals without parentheses cannot have default expressions", "Expected \'(\'");
       assert.throws(function () { eval("var a = 0, b = 0; (x = ++a,++b) => x"); },          SyntaxError, "Default expressions cannot have comma separated expressions",        "Expected identifier");
-
-      // Bug 263626: Checking strict formal parameters with defaults should not throw
-      function foostrict1(a = 1, b) { "use strict"; }
-      function foostrict2(a, b = 1) { "use strict"; }
     }
   },
   {

diff --git a/test/es6/rest.js b/test/es6/rest.js
@@ -11,7 +11,13 @@ var tests = [
     body: function () {
       assert.throws(function () { eval("function foo(...a, ...b) {}")},          SyntaxError,    "More than one rest parameter throws", "The rest parameter must be the last parameter in a formals list.");
       assert.throws(function () { eval("function foo(a, ...b, c) => {}")},       SyntaxError,    "Rest parameter not in the last position throws", "The rest parameter must be the last parameter in a formals list.");
-      assert.throws(function () { eval("var obj = class { method(a, b = 1, ...c = [2,3]) {} };")}, SyntaxError, "Rest parameter cannot have a default value");
+      assert.throws(function () { eval("var obj = class { method(a, b = 1, ...c = [2,3]) {} };")},         SyntaxError, "Rest parameter cannot have a default value");
+      assert.throws(function () { eval("function f(c, a, ...a) { }")},           SyntaxError,    "Duplicate parameters are not allowed for non-simple parameter list with only rest", "Duplicate formal parameter names not allowed in this context");
+      assert.throws(function () { eval("function f(c = 10, a, ...a) { }")},      SyntaxError,    "Duplicate parameters are not allowed for non-simple parameter list with both rest and default", "Duplicate formal parameter names not allowed in this context");
+      assert.throws(function () { eval("function f(a, ...b) { \"use strict\"; }")},                        SyntaxError, "Cannot apply strict mode to functions with rest parameter", "Cannot apply strict mode on functions with non-simple parameter list");
+      assert.throws(function () { eval("function f() { \"use strict\"; function g(a, b, c, ...a) { } }")}, SyntaxError, "Cannot have duplicate parameters for a function with non-simple parameter list, which is already in strict mode", "Duplicate formal parameter names not allowed in strict mode");
+      assert.throws(function () { eval("function f() { \"use strict\"; function g(a, b, a, ...c) { } }")}, SyntaxError, "Cannot have duplicate parameters for a function with non-simple parameter list with rest, which is already in strict mode", "Duplicate formal parameter names not allowed in strict mode");
+
       assert.throws(function () { eval("function foo(a = b, ...b) {}; foo();")}, ReferenceError, "Rest parameters cannot be referenced in default expressions (use before declaration)", "Use before declaration");
 
       // Redeclaration errors - non-simple in this case means any parameter list with a rest parameter