Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: non-directive "use strict" should not enable parsing in strict mode #11188

Merged
merged 15 commits into from Mar 5, 2020
89 changes: 46 additions & 43 deletions packages/babel-parser/src/parser/expression.js
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
43 changes: 33 additions & 10 deletions packages/babel-parser/src/parser/statement.js
Expand Up @@ -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();
}
Expand All @@ -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 = []);
Expand All @@ -829,6 +837,7 @@ export default class StatementParser extends ExpressionParser {
allowDirectives ? directives : undefined,
topLevel,
end,
afterBlockParse,
);
}

Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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);
}
Expand Down
27 changes: 1 addition & 26 deletions packages/babel-parser/src/parser/util.js
Expand Up @@ -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/;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted regexp -> Good news 😆


type TryParse<Node, Error, Thrown, Aborted, FailState> = {
node: Node,
error: Error,
Expand Down Expand Up @@ -193,29 +191,6 @@ export default class UtilParser extends Tokenizer {
}
}

strictDirective(start: number): boolean {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now dead code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Spoiler ⚠️

This is probably because before this PR, we first checked if there was a use strict directive and then tokenized the invalid literals.
Now, we first tokenize the invalid literals, then set this.state.strict to true, and in parseBlockOrModuleBlockBody only throw for the first invalid one. It should be fixed there.

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>>(
Expand Down
19 changes: 7 additions & 12 deletions packages/babel-parser/src/tokenizer/index.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this change unifies the language for all the octal literal strict mode errors.

}
if (/[89]/.test(this.input.slice(start, this.state.pos))) {
octal = false;
Expand Down Expand Up @@ -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);
}
}

Expand Down
7 changes: 4 additions & 3 deletions packages/babel-parser/src/tokenizer/state.js
Expand Up @@ -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 };`.
Expand Down