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: implement early errors for record and tuple #11652

Merged
merged 5 commits into from Jun 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/babel-parser/src/parser/error-message.js
Expand Up @@ -70,6 +70,8 @@ export const ErrorMessages = Object.freeze({
InvalidParenthesizedAssignment: "Invalid parenthesized assignment pattern",
InvalidPrivateFieldResolution: "Private name #%0 is not defined",
InvalidPropertyBindingPattern: "Binding member expression",
InvalidRecordProperty:
"Only properties and spread elements are allowed in record definitions",
InvalidRestAssignmentPattern: "Invalid rest operator's argument",
LabelRedeclaration: "Label '%0' is already declared",
LetInLexicalBinding:
Expand Down Expand Up @@ -124,6 +126,7 @@ export const ErrorMessages = Object.freeze({
"Record expressions starting with '{|' are only allowed when the 'syntaxType' option of the 'recordAndTuple' plugin is set to 'bar'",
RecordExpressionHashIncorrectStartSyntaxType:
"Record expressions starting with '#{' are only allowed when the 'syntaxType' option of the 'recordAndTuple' plugin is set to 'hash'",
RecordNoProto: "'__proto__' is not allowed in Record expressions",
RestTrailingComma: "Unexpected trailing comma after rest element",
SloppyFunction:
"In non-strict mode code, functions can only be declared at top level, inside a block, or as the body of an if statement",
Expand Down Expand Up @@ -163,7 +166,7 @@ export const ErrorMessages = Object.freeze({
"Private names can only be used as the name of a class element (i.e. class C { #p = 42; #m() {} } )\n or a property of member expression (i.e. this.#p).",
UnexpectedReservedWord: "Unexpected reserved word '%0'",
UnexpectedSuper: "super is only allowed in object methods and classes",
UnexpectedToken: "Unexpected token '%'",
UnexpectedToken: "Unexpected token '%0'",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️

UnexpectedTokenUnaryExponentiation:
"Illegal expression. Wrap left hand side or entire exponentiation in parentheses.",
UnsupportedBind: "Binding should be performed on object property.",
Expand Down
32 changes: 25 additions & 7 deletions packages/babel-parser/src/parser/expression.js
Expand Up @@ -74,19 +74,22 @@ export default class ExpressionParser extends LValParser {
+takeDecorators: (node: N.HasDecorators) => void;
*/

// Check if property __proto__ has been used more than once.
// For object literal, check if property __proto__ has been used more than once.
// If the expression is a destructuring assignment, then __proto__ may appear
// multiple times. Otherwise, __proto__ is a duplicated key.

checkDuplicatedProto(
// For record expression, check if property __proto__ exists

checkProto(
prop: N.ObjectMember | N.SpreadElement,
isRecord: boolean,
protoRef: { used: boolean },
refExpressionErrors: ?ExpressionErrors,
): void {
if (
prop.type === "SpreadElement" ||
prop.type === "ObjectMethod" ||
prop.computed ||
prop.kind ||
// $FlowIgnore
prop.shorthand
) {
Expand All @@ -95,9 +98,13 @@ export default class ExpressionParser extends LValParser {

const key = prop.key;
// It is either an Identifier or a String/NumericLiteral
const name = key.type === "Identifier" ? key.name : String(key.value);
const name = key.type === "Identifier" ? key.name : key.value;

if (name === "__proto__") {
if (isRecord) {
this.raise(key.start, Errors.RecordNoProto);
return;
}
if (protoRef.used) {
if (refExpressionErrors) {
// Store the first redefinition's position, otherwise ignore because
Expand Down Expand Up @@ -1050,7 +1057,7 @@ export default class ExpressionParser extends LValParser {
this.next();
node.elements = this.parseExprList(
close,
true,
false,
refExpressionErrors,
node,
);
Expand Down Expand Up @@ -1554,7 +1561,15 @@ export default class ExpressionParser extends LValParser {
const prop = this.parseObjectMember(isPattern, refExpressionErrors);
if (!isPattern) {
// $FlowIgnore RestElement will never be returned if !isPattern
this.checkDuplicatedProto(prop, propHash, refExpressionErrors);
this.checkProto(prop, isRecord, propHash, refExpressionErrors);
}

if (
isRecord &&
prop.type !== "ObjectProperty" &&
prop.type !== "SpreadElement"
) {
this.raise(prop.start, Errors.InvalidRecordProperty);
}

// $FlowIgnore
Expand Down Expand Up @@ -2088,7 +2103,10 @@ export default class ExpressionParser extends LValParser {
allowPlaceholder: ?boolean,
): ?N.Expression {
let elt;
if (allowEmpty && this.match(tt.comma)) {
if (this.match(tt.comma)) {
if (!allowEmpty) {
this.raise(this.state.pos, Errors.UnexpectedToken, ",");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I resort to the most generic error message. I still feel awkward mentioning holes when throwing f(,). Any suggestions are definitely welcome.

At least the error is now recoverable.

}
elt = null;
} else if (this.match(tt.ellipsis)) {
const spreadNodeStartPos = this.state.start;
Expand Down
30 changes: 5 additions & 25 deletions packages/babel-parser/src/plugins/estree.js
Expand Up @@ -143,37 +143,17 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}
}

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

const key = prop.key;
// It is either an Identifier or a String/NumericLiteral
const name = key.type === "Identifier" ? key.name : String(key.value);

if (name === "__proto__" && prop.kind === "init") {
// Store the first redefinition's position
if (protoRef.used) {
if (refExpressionErrors?.doubleProto === -1) {
refExpressionErrors.doubleProto = key.start;
} else {
this.raise(key.start, Errors.DuplicateProto);
}
}

protoRef.used = true;
}
super.checkProto(prop, isRecord, protoRef, refExpressionErrors);
}

isValidDirective(stmt: N.Statement): boolean {
Expand Down

This file was deleted.

@@ -0,0 +1,35 @@
{
"type": "File",
"start":0,"end":7,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":7}},
"errors": [
"SyntaxError: Unexpected token ',' (1:5)"
],
"program": {
"type": "Program",
"start":0,"end":7,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":7}},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start":0,"end":7,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":7}},
"expression": {
"type": "CallExpression",
"start":0,"end":6,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":6}},
"callee": {
"type": "Identifier",
"start":0,"end":3,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":3},"identifierName":"log"},
"name": "log"
},
"extra": {
"trailingComma": 4
},
"arguments": [
null
]
}
}
],
"directives": []
}
}
@@ -0,0 +1,7 @@
#{
a() {},
async b() {},
get c() {},
set d(_) {},
*e() {}
}
@@ -0,0 +1,3 @@
{
"plugins": [["recordAndTuple", { "syntaxType": "hash" }]]
}
@@ -0,0 +1,146 @@
{
"type": "File",
"start":0,"end":69,"loc":{"start":{"line":1,"column":0},"end":{"line":7,"column":1}},
"errors": [
"SyntaxError: Only properties and spread elements are allowed in record definitions (2:2)",
"SyntaxError: Only properties and spread elements are allowed in record definitions (3:2)",
"SyntaxError: Only properties and spread elements are allowed in record definitions (4:2)",
"SyntaxError: Only properties and spread elements are allowed in record definitions (5:2)",
"SyntaxError: Only properties and spread elements are allowed in record definitions (6:2)"
],
"program": {
"type": "Program",
"start":0,"end":69,"loc":{"start":{"line":1,"column":0},"end":{"line":7,"column":1}},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start":0,"end":69,"loc":{"start":{"line":1,"column":0},"end":{"line":7,"column":1}},
"expression": {
"type": "RecordExpression",
"start":0,"end":69,"loc":{"start":{"line":1,"column":0},"end":{"line":7,"column":1}},
"properties": [
{
"type": "ObjectMethod",
"start":5,"end":11,"loc":{"start":{"line":2,"column":2},"end":{"line":2,"column":8}},
"method": true,
"key": {
"type": "Identifier",
"start":5,"end":6,"loc":{"start":{"line":2,"column":2},"end":{"line":2,"column":3},"identifierName":"a"},
"name": "a"
},
"computed": false,
"kind": "method",
"id": null,
"generator": false,
"async": false,
"params": [],
"body": {
"type": "BlockStatement",
"start":9,"end":11,"loc":{"start":{"line":2,"column":6},"end":{"line":2,"column":8}},
"body": [],
"directives": []
}
},
{
"type": "ObjectMethod",
"start":15,"end":27,"loc":{"start":{"line":3,"column":2},"end":{"line":3,"column":14}},
"method": true,
"key": {
"type": "Identifier",
"start":21,"end":22,"loc":{"start":{"line":3,"column":8},"end":{"line":3,"column":9},"identifierName":"b"},
"name": "b"
},
"computed": false,
"kind": "method",
"id": null,
"generator": false,
"async": true,
"params": [],
"body": {
"type": "BlockStatement",
"start":25,"end":27,"loc":{"start":{"line":3,"column":12},"end":{"line":3,"column":14}},
"body": [],
"directives": []
}
},
{
"type": "ObjectMethod",
"start":31,"end":41,"loc":{"start":{"line":4,"column":2},"end":{"line":4,"column":12}},
"method": false,
"key": {
"type": "Identifier",
"start":35,"end":36,"loc":{"start":{"line":4,"column":6},"end":{"line":4,"column":7},"identifierName":"c"},
"name": "c"
},
"computed": false,
"kind": "get",
"id": null,
"generator": false,
"async": false,
"params": [],
"body": {
"type": "BlockStatement",
"start":39,"end":41,"loc":{"start":{"line":4,"column":10},"end":{"line":4,"column":12}},
"body": [],
"directives": []
}
},
{
"type": "ObjectMethod",
"start":45,"end":56,"loc":{"start":{"line":5,"column":2},"end":{"line":5,"column":13}},
"method": false,
"key": {
"type": "Identifier",
"start":49,"end":50,"loc":{"start":{"line":5,"column":6},"end":{"line":5,"column":7},"identifierName":"d"},
"name": "d"
},
"computed": false,
"kind": "set",
"id": null,
"generator": false,
"async": false,
"params": [
{
"type": "Identifier",
"start":51,"end":52,"loc":{"start":{"line":5,"column":8},"end":{"line":5,"column":9},"identifierName":"_"},
"name": "_"
}
],
"body": {
"type": "BlockStatement",
"start":54,"end":56,"loc":{"start":{"line":5,"column":11},"end":{"line":5,"column":13}},
"body": [],
"directives": []
}
},
{
"type": "ObjectMethod",
"start":60,"end":67,"loc":{"start":{"line":6,"column":2},"end":{"line":6,"column":9}},
"method": true,
"key": {
"type": "Identifier",
"start":61,"end":62,"loc":{"start":{"line":6,"column":3},"end":{"line":6,"column":4},"identifierName":"e"},
"name": "e"
},
"computed": false,
"kind": "method",
"id": null,
"generator": true,
"async": false,
"params": [],
"body": {
"type": "BlockStatement",
"start":65,"end":67,"loc":{"start":{"line":6,"column":7},"end":{"line":6,"column":9}},
"body": [],
"directives": []
}
}
]
}
}
],
"directives": []
}
}
@@ -0,0 +1 @@
#[,]
@@ -0,0 +1,10 @@
{
"plugins": [
[
"recordAndTuple",
{
"syntaxType": "hash"
}
]
]
}
@@ -0,0 +1,30 @@
{
"type": "File",
"start":0,"end":4,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":4}},
"errors": [
"SyntaxError: Unexpected token ',' (1:3)"
],
"program": {
"type": "Program",
"start":0,"end":4,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":4}},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start":0,"end":4,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":4}},
"expression": {
"type": "TupleExpression",
"start":0,"end":4,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":4}},
"extra": {
"trailingComma": 2
},
"elements": [
null
]
}
}
],
"directives": []
}
}
@@ -0,0 +1 @@
#[1,]