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

Make sure babel parser throws exactly same recoverable errors when estree plugin is enabled #12375

Merged
merged 13 commits into from Dec 3, 2020
Merged
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";
}
};