Skip to content

Commit

Permalink
Make sure babel parser throws exactly same recoverable errors when es…
Browse files Browse the repository at this point in the history
…tree plugin is enabled (#12375)

* refactor: introduce isPrivateName and getPrivateNameSV

* feat: check recoverable errors on estree-throw

* fix: pass through all params of parseBlockBody

* fix: set bigInt to null when invalid bigInt value is parsed

e.g. 0.1n

* fix: use string literal value in error message

When estree plugin is enabled, stringLiteral#extra.raw is not accessible. Use StringLiteral#value instead.

* refactor: introduce hasPropertyAsPrivateName

* fix: adapt to ChainExpression

* fix: port checkLVal early return for method in object pattern

* fix: throw new a?.() on estree

* fix: early return for __proto__ in accessors

* fix: test record element via isObjectProperty

* fix: pass through isLHS in toAssignable

* refactor: introduce isObjectMethod methods
  • Loading branch information
JLHwung committed Dec 3, 2020
1 parent c6aea4e commit 8478027
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 91 deletions.
2 changes: 1 addition & 1 deletion packages/babel-parser/src/parser/error-message.js
Expand Up @@ -51,7 +51,7 @@ export const ErrorMessages = Object.freeze({
ElementAfterRest: "Rest element must be last element",
EscapedCharNotAnIdentifier: "Invalid Unicode escape",
ExportBindingIsString:
"A string literal cannot be used as an exported binding without `from`.\n- Did you mean `export { %0 as '%1' } from 'some-module'`?",
"A string literal cannot be used as an exported binding without `from`.\n- Did you mean `export { '%0' as '%1' } from 'some-module'`?",
ExportDefaultFromAsIdentifier:
"'from' is not allowed as an identifier after 'export default'",
ForInOfLoopInitializer:
Expand Down
28 changes: 12 additions & 16 deletions packages/babel-parser/src/parser/expression.js
Expand Up @@ -94,8 +94,9 @@ export default class ExpressionParser extends LValParser {
): void {
if (
prop.type === "SpreadElement" ||
prop.type === "ObjectMethod" ||
this.isObjectMethod(prop) ||
prop.computed ||
// $FlowIgnore
prop.shorthand
) {
return;
Expand Down Expand Up @@ -515,11 +516,7 @@ export default class ExpressionParser extends LValParser {

if (arg.type === "Identifier") {
this.raise(node.start, Errors.StrictDelete);
} else if (
(arg.type === "MemberExpression" ||
arg.type === "OptionalMemberExpression") &&
arg.property.type === "PrivateName"
) {
} else if (this.hasPropertyAsPrivateName(arg)) {
this.raise(node.start, Errors.DeletePrivateField);
}
}
Expand Down Expand Up @@ -618,12 +615,12 @@ export default class ExpressionParser extends LValParser {

let optional = false;
if (this.match(tt.questionDot)) {
state.optionalChainMember = optional = true;
if (noCalls && this.lookaheadCharCode() === charCodes.leftParenthesis) {
// stop at `?.` when parsing `new a?.()`
state.stop = true;
return base;
}
state.optionalChainMember = optional = true;
this.next();
}

Expand Down Expand Up @@ -662,11 +659,14 @@ export default class ExpressionParser extends LValParser {
? this.parseExpression()
: this.parseMaybePrivateName(true);

if (property.type === "PrivateName") {
if (this.isPrivateName(property)) {
if (node.object.type === "Super") {
this.raise(startPos, Errors.SuperPrivateField);
}
this.classScope.usePrivateName(property.id.name, property.start);
this.classScope.usePrivateName(
this.getPrivateNameSV(property),
property.start,
);
}
node.property = property;

Expand Down Expand Up @@ -1496,13 +1496,9 @@ export default class ExpressionParser extends LValParser {
// https://tc39.es/ecma262/#prod-NewExpression
parseNew(node: N.Expression): N.NewExpression {
node.callee = this.parseNoCallExpr();

if (node.callee.type === "Import") {
this.raise(node.callee.start, Errors.ImportCallNotNewExpression);
} else if (
node.callee.type === "OptionalMemberExpression" ||
node.callee.type === "OptionalCallExpression"
) {
} else if (this.isOptionalChain(node.callee)) {
this.raise(this.state.lastTokEnd, Errors.OptionalChainingNoNew);
} else if (this.eat(tt.questionDot)) {
this.raise(this.state.start, Errors.OptionalChainingNoNew);
Expand Down Expand Up @@ -1604,7 +1600,7 @@ export default class ExpressionParser extends LValParser {

if (
isRecord &&
prop.type !== "ObjectProperty" &&
!this.isObjectProperty(prop) &&
prop.type !== "SpreadElement"
) {
this.raise(prop.start, Errors.InvalidRecordProperty);
Expand Down Expand Up @@ -1923,7 +1919,7 @@ export default class ExpressionParser extends LValParser {
? this.parseExprAtom()
: this.parseMaybePrivateName(isPrivateNameAllowed);

if (prop.key.type !== "PrivateName") {
if (!this.isPrivateName(prop.key)) {
// ClassPrivateProperty is never computed, so we don't assign in that case.
prop.computed = false;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/babel-parser/src/parser/lval.js
Expand Up @@ -445,11 +445,11 @@ export default class LValParser extends NodeUtils {

case "ObjectPattern":
for (let prop of expr.properties) {
if (prop.type === "ObjectProperty") prop = prop.value;
if (this.isObjectProperty(prop)) prop = prop.value;
// If we find here an ObjectMethod, it's because this was originally
// an ObjectExpression which has then been converted.
// toAssignable already reported this error with a nicer message.
else if (prop.type === "ObjectMethod") continue;
else if (this.isObjectMethod(prop)) continue;

this.checkLVal(
prop,
Expand Down
23 changes: 15 additions & 8 deletions packages/babel-parser/src/parser/statement.js
Expand Up @@ -1346,7 +1346,7 @@ export default class StatementParser extends ExpressionParser {
method.kind = "method";
this.parseClassElementName(method);
if (method.key.type === "PrivateName") {
if (this.isPrivateName(method.key)) {
// Private generator method
this.pushClassPrivateMethod(classBody, privateMethod, true, false);
return;
Expand All @@ -1370,7 +1370,7 @@ export default class StatementParser extends ExpressionParser {
const containsEsc = this.state.containsEsc;
const key = this.parseClassElementName(member);
const isPrivate = key.type === "PrivateName";
const isPrivate = this.isPrivateName(key);
// Check the key is not a computed expression or string literal.
const isSimple = key.type === "Identifier";
const maybeQuestionTokenStart = this.state.start;
Expand Down Expand Up @@ -1431,7 +1431,7 @@ export default class StatementParser extends ExpressionParser {
this.parseClassElementName(method);
this.parsePostMemberNameModifiers(publicMember);

if (method.key.type === "PrivateName") {
if (this.isPrivateName(method.key)) {
// private async method
this.pushClassPrivateMethod(
classBody,
Expand Down Expand Up @@ -1465,7 +1465,7 @@ export default class StatementParser extends ExpressionParser {
// The so-called parsed name would have been "get/set": get the real name.
this.parseClassElementName(publicMethod);

if (method.key.type === "PrivateName") {
if (this.isPrivateName(method.key)) {
// private getter/setter
this.pushClassPrivateMethod(classBody, privateMethod, false, false);
} else {
Expand Down Expand Up @@ -1508,7 +1508,10 @@ export default class StatementParser extends ExpressionParser {
this.raise(key.start, Errors.StaticPrototype);
}

if (key.type === "PrivateName" && key.id.name === "constructor") {
if (
this.isPrivateName(key) &&
this.getPrivateNameSV(key) === "constructor"
) {
this.raise(key.start, Errors.ConstructorClassPrivateField);
}

Expand Down Expand Up @@ -1571,7 +1574,7 @@ export default class StatementParser extends ExpressionParser {
classBody.body.push(node);

this.classScope.declarePrivateName(
node.key.id.name,
this.getPrivateNameSV(node.key),
CLASS_ELEMENT_OTHER,
node.key.start,
);
Expand Down Expand Up @@ -1627,7 +1630,11 @@ export default class StatementParser extends ExpressionParser {
? CLASS_ELEMENT_STATIC_SETTER
: CLASS_ELEMENT_INSTANCE_SETTER
: CLASS_ELEMENT_OTHER;
this.classScope.declarePrivateName(node.key.id.name, kind, node.key.start);
this.classScope.declarePrivateName(
this.getPrivateNameSV(node.key),
kind,
node.key.start,
);
}

// Overridden in typescript.js
Expand Down Expand Up @@ -1980,7 +1987,7 @@ export default class StatementParser extends ExpressionParser {
this.raise(
specifier.start,
Errors.ExportBindingIsString,
local.extra.raw,
local.value,
exportedName,
);
} else {
Expand Down
45 changes: 45 additions & 0 deletions packages/babel-parser/src/parser/util.js
Expand Up @@ -252,6 +252,51 @@ export default class UtilParser extends Tokenizer {
this.match(tt.decimal)
);
}

/*
* Test if given node is a PrivateName
* will be overridden in ESTree plugin
*/
isPrivateName(node: Node): boolean {
return node.type === "PrivateName";
}

/*
* Return the string value of a given private name
* WITHOUT `#`
* @see {@link https://tc39.es/proposal-class-fields/#sec-private-names-static-semantics-stringvalue}
*/
getPrivateNameSV(node: Node): string {
return node.id.name;
}

/*
* Return whether the given node is a member/optional chain that
* contains a private name as its property
* It is overridden in ESTree plugin
*/
hasPropertyAsPrivateName(node: Node): boolean {
return (
(node.type === "MemberExpression" ||
node.type === "OptionalMemberExpression") &&
this.isPrivateName(node.property)
);
}

isOptionalChain(node: Node): boolean {
return (
node.type === "OptionalMemberExpression" ||
node.type === "OptionalCallExpression"
);
}

isObjectProperty(node: Node): boolean {
return node.type === "ObjectProperty";
}

isObjectMethod(node: Node): boolean {
return node.type === "ObjectMethod";
}
}

/**
Expand Down
92 changes: 33 additions & 59 deletions packages/babel-parser/src/plugins/estree.js
Expand Up @@ -5,18 +5,8 @@ import type Parser from "../parser";
import type { ExpressionErrors } from "../parser/util";
import * as N from "../types";
import type { Position } from "../util/location";
import { type BindingTypes } from "../util/scopeflags";
import { Errors } from "../parser/error";

function isSimpleProperty(node: N.Node): boolean {
return (
node != null &&
node.type === "Property" &&
node.kind === "init" &&
node.method === false
);
}

export default (superClass: Class<Parser>): Class<Parser> =>
class extends superClass {
estreeParseRegExpLiteral({ pattern, flags }: N.RegExpLiteral): N.Node {
Expand All @@ -35,8 +25,13 @@ export default (superClass: Class<Parser>): Class<Parser> =>

estreeParseBigIntLiteral(value: any): N.Node {
// https://github.com/estree/estree/blob/master/es2020.md#bigintliteral
// $FlowIgnore
const bigInt = typeof BigInt !== "undefined" ? BigInt(value) : null;
let bigInt;
try {
// $FlowIgnore
bigInt = BigInt(value);
} catch {
bigInt = null;
}
const node = this.estreeParseLiteral(bigInt);
node.bigint = String(node.value || value);

Expand Down Expand Up @@ -98,7 +93,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}

checkDeclaration(node: N.Pattern | N.ObjectProperty): void {
if (isSimpleProperty(node)) {
if (node != null && this.isObjectProperty(node)) {
this.checkDeclaration(((node: any): N.EstreeProperty).value);
} else {
super.checkDeclaration(node);
Expand All @@ -110,44 +105,6 @@ export default (superClass: Class<Parser>): Class<Parser> =>
.params;
}

checkLVal(
expr: N.Expression,
contextDescription: string,
...args: [
BindingTypes | void,
?Set<string>,
boolean | void,
boolean | void,
]
): void {
switch (expr.type) {
case "ObjectPattern":
expr.properties.forEach(prop => {
this.checkLVal(
prop.type === "Property" ? prop.value : prop,
"object destructuring pattern",
...args,
);
});
break;
default:
super.checkLVal(expr, contextDescription, ...args);
}
}

checkProto(
prop: N.ObjectMember | N.SpreadElement,
isRecord: boolean,
protoRef: { used: boolean },
refExpressionErrors: ?ExpressionErrors,
): void {
// $FlowIgnore: check prop.method and fallback to super method
if (prop.method) {
return;
}
super.checkProto(prop, isRecord, protoRef, refExpressionErrors);
}

isValidDirective(stmt: N.Statement): boolean {
return (
stmt.type === "ExpressionStatement" &&
Expand All @@ -170,11 +127,9 @@ export default (superClass: Class<Parser>): Class<Parser> =>

parseBlockBody(
node: N.BlockStatementLike,
allowDirectives: ?boolean,
topLevel: boolean,
end: TokenType,
...args: [?boolean, boolean, TokenType, void | (boolean => void)]
): void {
super.parseBlockBody(node, allowDirectives, topLevel, end);
super.parseBlockBody(node, ...args);

const directiveStatements = node.directives.map(d =>
this.directiveToStmt(d),
Expand Down Expand Up @@ -337,8 +292,8 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}

toAssignable(node: N.Node, isLHS: boolean = false): N.Node {
if (isSimpleProperty(node)) {
this.toAssignable(node.value);
if (node != null && this.isObjectProperty(node)) {
this.toAssignable(node.value, isLHS);

return node;
}
Expand All @@ -348,9 +303,9 @@ export default (superClass: Class<Parser>): Class<Parser> =>

toAssignableObjectExpressionProp(prop: N.Node, ...args) {
if (prop.kind === "get" || prop.kind === "set") {
throw this.raise(prop.key.start, Errors.PatternHasAccessor);
this.raise(prop.key.start, Errors.PatternHasAccessor);
} else if (prop.method) {
throw this.raise(prop.key.start, Errors.PatternHasMethod);
this.raise(prop.key.start, Errors.PatternHasMethod);
} else {
super.toAssignableObjectExpressionProp(prop, ...args);
}
Expand Down Expand Up @@ -450,4 +405,23 @@ export default (superClass: Class<Parser>): Class<Parser> =>
return node;
}
hasPropertyAsPrivateName(node: N.Node): boolean {
if (node.type === "ChainExpression") {
node = node.expression;
}
return super.hasPropertyAsPrivateName(node);
}
isOptionalChain(node: N.Node): boolean {
return node.type === "ChainExpression";
}
isObjectProperty(node: N.Node): boolean {
return node.type === "Property" && node.kind === "init" && !node.method;
}
isObjectMethod(node: N.Node): boolean {
return node.method || node.kind === "get" || node.kind === "set";
}
};

0 comments on commit 8478027

Please sign in to comment.