Fixing labels, should throw syntax errors when labels are followed by lexical declarations

Fixes #352

Author: Kenji Fukuda

lib/Parser/Parse.cpp

@@ -9631,6 +9631,7 @@ ParseNodePtr Parser::ParseStatement()
     uint fnop;
     bool expressionStmt = false;
     bool isAsyncMethod = false;
+    bool labelledStatement = false;
     tokens tok;
 #if EXCEPTION_RECOVERY
     ParseNodeTryCatch * pParentTryCatch = nullptr;
@@ -9704,6 +9705,21 @@ ParseNodePtr Parser::ParseStatement()
         {
             pnode = ParseFncDeclCheckScope<buildAST>(fFncDeclaration | (isAsyncMethod ? fFncAsync : fFncNoFlgs));
         }
+
+        Assert(pnode != nullptr);
+        ParseNodeFnc* pNodeFnc = (ParseNodeFnc*)pnode;
+        if (labelledStatement)
+        {
+            if (pNodeFnc->IsAsync())
+            {
+                Error(ERRLabelBeforeAsyncFncDeclaration);
+            }
+            else if (pNodeFnc->IsGenerator())
+            {
+                Error(ERRLabelBeforeGeneratorDeclaration);
+            }
+        }
+        
         if (isAsyncMethod)
         {
             pnode->AsParseNodeFnc()->cbMin = iecpMin;
@@ -9713,7 +9729,11 @@ ParseNodePtr Parser::ParseStatement()
     }
 
     case tkCLASS:
-        if (m_scriptContext->GetConfig()->IsES6ClassAndExtendsEnabled())
+        if (labelledStatement)
+        {
+            Error(ERRLabelBeforeClassDeclaration);
+        }
+        else if (m_scriptContext->GetConfig()->IsES6ClassAndExtendsEnabled())
         {
             pnode = ParseClassDecl<buildAST>(TRUE, nullptr, nullptr, nullptr);
         }
@@ -9726,6 +9746,10 @@ ParseNodePtr Parser::ParseStatement()
     case tkID:
         if (m_token.GetIdentifier(this->GetHashTbl()) == wellKnownPropertyPids.let)
         {
+            if (labelledStatement)
+            {
+                Error(ERRLabelBeforeLexicalDeclaration);
+            }
             // We see "let" at the start of a statement. This could either be a declaration or an identifier
             // reference. The next token determines which.
             RestorePoint parsedLet;
@@ -9759,6 +9783,10 @@ ParseNodePtr Parser::ParseStatement()
 
     case tkCONST:
     case tkLET:
+        if (labelledStatement)
+        {
+            Error(ERRLabelBeforeLexicalDeclaration);
+        }
         ichMin = this->GetScanner()->IchMinTok();
 
         this->GetScanner()->Scan();
@@ -10628,6 +10656,7 @@ ParseNodePtr Parser::ParseStatement()
                 pLabelId->next = pLabelIdList;
                 pLabelIdList = pLabelId;
                 this->GetScanner()->Scan();
+                labelledStatement = true;
                 goto LRestart;
             }
 

lib/Parser/perrors.h

@@ -102,3 +102,7 @@ LSC_ERROR_MSG(1087, ERRInvalidExportName, "Unable to resolve module export name"
 LSC_ERROR_MSG(1088, ERRLetIDInLexicalDecl, "'let' is not an allowed identifier in lexical declarations")
 
 LSC_ERROR_MSG(1089, ERRInvalidLHSInFor, "Invalid left-hand side in for loop")
+LSC_ERROR_MSG(1090, ERRLabelBeforeLexicalDeclaration, "Labels not allowed before lexical declaration")
+LSC_ERROR_MSG(1091, ERRLabelBeforeGeneratorDeclaration, "Labels not allowed before generator declaration")
+LSC_ERROR_MSG(1092, ERRLabelBeforeAsyncFncDeclaration, "Labels not allowed before async function declaration")
+LSC_ERROR_MSG(1093, ERRLabelBeforeClassDeclaration, "Labels not allowed before class declaration")

test/Error/Labels.js

@@ -0,0 +1,81 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+
+
+testFunctionAfterLabel =
+    `a: 
+        function x()
+        {
+        }`
+testVarAfterLabel = 
+    `b:
+        var a;`
+testForLoopAfterLabel = 
+    `c:
+        for(i = 0; i < 10; i++)
+        {
+            d:
+                for(j = 0; j < 10; j++)
+                {
+                    if((i%j)%3 === 0)
+                        break d;
+                }
+        }`
+testWhileLoopAfterLabel = 
+    `i = 1, j = 1, product = 1
+    c:
+        while(i <= 20)
+        {
+            d:
+                while(j <= 5)
+                {
+                    j = j+2
+                    product *= i*j;
+                    break c;
+                }
+            i = i+1
+        }
+        assert.areEqual(3, product)
+        `
+testNonLexicalDeclarationAfterLabel=
+    `d:
+        g = "Hola, Mundo."`
+var tests = [
+    {
+        name : "Labels followed by declarations which should all be syntax errors.",
+        body : function () 
+        {
+            assert.throws(() => eval("a: let x;"), SyntaxError, 
+            "A let declaration must not be labelled", "Labels not allowed before lexical declaration");
+            assert.throws(() => eval("b: const y = 0;"), SyntaxError, 
+            "A const declaration must not be labelled", "Labels not allowed before lexical declaration");
+            assert.throws(() => eval("c: class z {}"), SyntaxError, 
+            "An class declaration must not be labelled", "Labels not allowed before class declaration");
+            assert.throws(() => eval("d: function* w() {}"), SyntaxError, 
+            "A generator must not be labelled", "Labels not allowed before generator declaration");
+            assert.throws(() => eval("e: async function u() { }"), SyntaxError, 
+            "An async function must not be labelled", "Labels not allowed before async function declaration");
+            /*assert.throws(() => eval("e: async function* u() { }"), SyntaxError, //TODO: Uncomment when async generators supported.
+            "An async generator must not be labelled", "Labels not allowed before async function declaration");*/
+        }
+    },
+    {
+        name : "Labels followed by non-declarations are valid syntax (non-generator non-async function declarations and non-lexical variable declarations are allowed)",
+        body : function () 
+        {
+            assert.doesNotThrow(() => eval(testFunctionAfterLabel), "Function declarations should be allowed after a label."); 
+            assert.doesNotThrow(() => eval(testVarAfterLabel), "var declarations should be allowed after a label.");
+            assert.doesNotThrow(() => eval(testForLoopAfterLabel), "for loop statements should be allowed after a label.");
+            assert.doesNotThrow(() => eval(testNonLexicalDeclarationAfterLabel), "non-lexical declarations should be allowed after a label.");
+            assert.doesNotThrow(() => eval(testWhileLoopAfterLabel), "while loops should be allowed after a label");
+        }
+    }
+];
+
+testRunner.runTests(tests, {
+    verbose : WScript.Arguments[0] != "summary"
+});

test/Error/rlexe.xml

@@ -178,4 +178,10 @@
       <compile-flags>-ExtendedErrorStackForTestHost -force:DeferParse</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>Labels.js</files>
+      <compile-flags>-args summary -endargs</compile-flags>
+    </default>
+  </test>
 </regress-exe>