fix: non-directive "use strict" should not enable parsing in strict m…

…ode (#11188) * fix: non-directive "use strict" should not enable parsing in strict mode * Remove dead code * Add stricter type annotations * set oldStrict explicitly to null * Fix error * label callback argument * update comment * Address feedback * Remove this.state.octalPosition * Add additional tests * Revert "Remove this.state.octalPosition" This reverts commit bcc78c9. * Remove containsOctal * Report multiple octal literals in single token * Fix comments * remove Array.prototype.flat()
babel · Mar 5, 2020 · 2057d2b · 2057d2b
1 parent e297e40
commit 2057d2b
Show file tree

Hide file tree

Showing 24 changed files with 5,303 additions and 126 deletions.
diff --git a/packages/babel-parser/src/parser/expression.js b/packages/babel-parser/src/parser/expression.js
@@ -55,6 +55,7 @@ export default class ExpressionParser extends LValParser {
   +parseBlock: (
     allowDirectives?: boolean,
     createNewLexicalScope?: boolean,
+    afterBlockParse?: (hasStrictModeDirective: boolean) => void,
   ) => N.BlockStatement;
   +parseClass: (
     node: N.Class,
@@ -1877,68 +1878,70 @@ export default class ExpressionParser extends LValParser {
     isMethod?: boolean = false,
   ): void {
     const isExpression = allowExpression && !this.match(tt.braceL);
-    const oldStrict = this.state.strict;
-    let useStrict = false;
-
     const oldInParameters = this.state.inParameters;
     this.state.inParameters = false;
 
     if (isExpression) {
       node.body = this.parseMaybeAssign();
       this.checkParams(node, false, allowExpression, false);
     } else {
-      const nonSimple = !this.isSimpleParamList(node.params);
-      if (!oldStrict || nonSimple) {
-        useStrict = this.strictDirective(this.state.end);
-        // If this is a strict mode function, verify that argument names
-        // are not repeated, and it does not try to bind the words `eval`
-        // or `arguments`.
-        if (useStrict && nonSimple) {
-          // This logic is here to align the error location with the estree plugin
-          const errorPos =
-            // $FlowIgnore
-            (node.kind === "method" || node.kind === "constructor") &&
-            // $FlowIgnore
-            !!node.key
-              ? node.key.end
-              : node.start;
-          this.raise(errorPos, Errors.IllegalLanguageModeDirective);
-        }
-      }
+      const oldStrict = this.state.strict;
       // Start a new scope with regard to labels
       // flag (restore them to their old value afterwards).
       const oldLabels = this.state.labels;
       this.state.labels = [];
-      if (useStrict) this.state.strict = true;
-      // Add the params to varDeclaredNames to ensure that an error is thrown
-      // if a let/const declaration in the function clashes with one of the params.
-      this.checkParams(
-        node,
-        !oldStrict && !useStrict && !allowExpression && !isMethod && !nonSimple,
-        allowExpression,
-        !oldStrict && useStrict,
-      );
+
       // FunctionBody[Yield, Await]:
       //   StatementList[?Yield, ?Await, +Return] opt
       this.prodParam.enter(this.prodParam.currentFlags() | PARAM_RETURN);
-      node.body = this.parseBlock(true, false);
+      node.body = this.parseBlock(
+        true,
+        false,
+        // Strict mode function checks after we parse the statements in the function body.
+        (hasStrictModeDirective: boolean) => {
+          const nonSimple = !this.isSimpleParamList(node.params);
+
+          if (hasStrictModeDirective && nonSimple) {
+            // This logic is here to align the error location with the ESTree plugin.
+            const errorPos =
+              // $FlowIgnore
+              (node.kind === "method" || node.kind === "constructor") &&
+              // $FlowIgnore
+              !!node.key
+                ? node.key.end
+                : node.start;
+            this.raise(errorPos, Errors.IllegalLanguageModeDirective);
+          }
+
+          const strictModeChanged = !oldStrict && this.state.strict;
+
+          // Add the params to varDeclaredNames to ensure that an error is thrown
+          // if a let/const declaration in the function clashes with one of the params.
+          this.checkParams(
+            node,
+            !this.state.strict && !allowExpression && !isMethod && !nonSimple,
+            allowExpression,
+            strictModeChanged,
+          );
+
+          // Ensure the function name isn't a forbidden identifier in strict mode, e.g. 'eval'
+          if (this.state.strict && node.id) {
+            this.checkLVal(
+              node.id,
+              BIND_OUTSIDE,
+              undefined,
+              "function name",
+              undefined,
+              strictModeChanged,
+            );
+          }
+        },
+      );
       this.prodParam.exit();
       this.state.labels = oldLabels;
     }
 
     this.state.inParameters = oldInParameters;
-    // Ensure the function name isn't a forbidden identifier in strict mode, e.g. 'eval'
-    if (this.state.strict && node.id) {
-      this.checkLVal(
-        node.id,
-        BIND_OUTSIDE,
-        undefined,
-        "function name",
-        undefined,
-        !oldStrict && useStrict,
-      );
-    }
-    this.state.strict = oldStrict;
   }
 
   isSimpleParamList(

diff --git a/packages/babel-parser/src/parser/statement.js b/packages/babel-parser/src/parser/statement.js
@@ -795,13 +795,20 @@ export default class StatementParser extends ExpressionParser {
   parseBlock(
     allowDirectives?: boolean = false,
     createNewLexicalScope?: boolean = true,
+    afterBlockParse?: (hasStrictModeDirective: boolean) => void,
   ): N.BlockStatement {
     const node = this.startNode();
     this.expect(tt.braceL);
     if (createNewLexicalScope) {
       this.scope.enter(SCOPE_OTHER);
     }
-    this.parseBlockBody(node, allowDirectives, false, tt.braceR);
+    this.parseBlockBody(
+      node,
+      allowDirectives,
+      false,
+      tt.braceR,
+      afterBlockParse,
+    );
     if (createNewLexicalScope) {
       this.scope.exit();
     }
@@ -821,6 +828,7 @@ export default class StatementParser extends ExpressionParser {
     allowDirectives: ?boolean,
     topLevel: boolean,
     end: TokenType,
+    afterBlockParse?: (hasStrictModeDirective: boolean) => void,
   ): void {
     const body = (node.body = []);
     const directives = (node.directives = []);
@@ -829,6 +837,7 @@ export default class StatementParser extends ExpressionParser {
       allowDirectives ? directives : undefined,
       topLevel,
       end,
+      afterBlockParse,
     );
   }
 
@@ -838,14 +847,16 @@ export default class StatementParser extends ExpressionParser {
     directives: ?(N.Directive[]),
     topLevel: boolean,
     end: TokenType,
+    afterBlockParse?: (hasStrictModeDirective: boolean) => void,
   ): void {
+    const octalPositions = [];
     let parsedNonDirective = false;
-    let oldStrict;
-    let octalPosition;
+    let oldStrict = null;
 
     while (!this.eat(end)) {
-      if (!parsedNonDirective && this.state.containsOctal && !octalPosition) {
-        octalPosition = this.state.octalPosition;
+      // Track octal literals that occur before a "use strict" directive.
+      if (!parsedNonDirective && this.state.octalPositions.length) {
+        octalPositions.push(...this.state.octalPositions);
       }
 
       const stmt = this.parseStatement(null, topLevel);
@@ -854,13 +865,9 @@ export default class StatementParser extends ExpressionParser {
         const directive = this.stmtToDirective(stmt);
         directives.push(directive);
 
-        if (oldStrict === undefined && directive.value.value === "use strict") {
+        if (oldStrict === null && directive.value.value === "use strict") {
           oldStrict = this.state.strict;
           this.setStrict(true);
-
-          if (octalPosition) {
-            this.raise(octalPosition, Errors.StrictOctalLiteral);
-          }
         }
 
         continue;
@@ -870,6 +877,22 @@ export default class StatementParser extends ExpressionParser {
       body.push(stmt);
     }
 
+    // Throw an error for any octal literals found before a
+    // "use strict" directive. Strict mode will be set at parse
+    // time for any literals that occur after the directive.
+    if (this.state.strict && octalPositions.length) {
+      for (const pos of octalPositions) {
+        this.raise(pos, Errors.StrictOctalLiteral);
+      }
+    }
+
+    if (afterBlockParse) {
+      afterBlockParse.call(
+        this,
+        /* hasStrictModeDirective */ oldStrict !== null,
+      );
+    }
+
     if (oldStrict === false) {
       this.setStrict(false);
     }

diff --git a/packages/babel-parser/src/parser/util.js b/packages/babel-parser/src/parser/util.js
@@ -4,13 +4,11 @@ import { types as tt, type TokenType } from "../tokenizer/types";
 import Tokenizer from "../tokenizer";
 import State from "../tokenizer/state";
 import type { Node } from "../types";
-import { lineBreak, skipWhiteSpace } from "../util/whitespace";
+import { lineBreak } from "../util/whitespace";
 import { isIdentifierChar } from "../util/identifier";
 import * as charCodes from "charcodes";
 import { Errors } from "./location";
 
-const literal = /^('|")((?:\\?.)*?)\1/;
-
 type TryParse<Node, Error, Thrown, Aborted, FailState> = {
   node: Node,
   error: Error,
@@ -193,29 +191,6 @@ export default class UtilParser extends Tokenizer {
     }
   }
 
-  strictDirective(start: number): boolean {
-    for (;;) {
-      // Try to find string literal.
-      skipWhiteSpace.lastIndex = start;
-      // $FlowIgnore
-      start += skipWhiteSpace.exec(this.input)[0].length;
-      const match = literal.exec(this.input.slice(start));
-      if (!match) break;
-      if (match[2] === "use strict") return true;
-      start += match[0].length;
-
-      // Skip semicolon, if any.
-      skipWhiteSpace.lastIndex = start;
-      // $FlowIgnore
-      start += skipWhiteSpace.exec(this.input)[0].length;
-      if (this.input[start] === ";") {
-        start++;
-      }
-    }
-
-    return false;
-  }
-
   // tryParse will clone parser state.
   // It is expensive and should be used with cautions
   tryParse<T: Node | $ReadOnlyArray<Node>>(

diff --git a/packages/babel-parser/src/tokenizer/index.js b/packages/babel-parser/src/tokenizer/index.js
@@ -224,8 +224,7 @@ export default class Tokenizer extends LocationParser {
     const curContext = this.curContext();
     if (!curContext || !curContext.preserveSpace) this.skipSpace();
 
-    this.state.containsOctal = false;
-    this.state.octalPosition = null;
+    this.state.octalPositions = [];
     this.state.start = this.state.pos;
     this.state.startLoc = this.state.curPosition();
     if (this.state.pos >= this.length) {
@@ -1033,11 +1032,7 @@ export default class Tokenizer extends LocationParser {
       this.input.charCodeAt(start) === charCodes.digit0;
     if (octal) {
       if (this.state.strict) {
-        this.raise(
-          start,
-          // todo: merge with Errors.StrictOctalLiteral
-          "Legacy octal literals are not allowed in strict mode",
-        );
+        this.raise(start, Errors.StrictOctalLiteral);
       }
       if (/[89]/.test(this.input.slice(start, this.state.pos))) {
         octal = false;
@@ -1296,11 +1291,11 @@ export default class Tokenizer extends LocationParser {
               return null;
             } else if (this.state.strict) {
               this.raise(codePos, Errors.StrictOctalLiteral);
-            } else if (!this.state.containsOctal) {
-              // These properties are only used to throw an error for an octal which occurs
-              // in a directive which occurs prior to a "use strict" directive.
-              this.state.containsOctal = true;
-              this.state.octalPosition = codePos;
+            } else {
+              // This property is used to throw an error for
+              // an octal literal in a directive that occurs prior
+              // to a "use strict" directive.
+              this.state.octalPositions.push(codePos);
             }
           }
 

diff --git a/packages/babel-parser/src/tokenizer/state.js b/packages/babel-parser/src/tokenizer/state.js
@@ -141,9 +141,10 @@ export default class State {
   // escape sequences must not be interpreted as keywords.
   containsEsc: boolean = false;
 
-  // TODO
-  containsOctal: boolean = false;
-  octalPosition: ?number = null;
+  // This property is used to throw an error for
+  // an octal literal in a directive that occurs prior
+  // to a "use strict" directive.
+  octalPositions: number[] = [];
 
   // Names of exports store. `default` is stored as a name for both
   // `export default foo;` and `export { foo as default };`.