Disallow trailing comma after rest (#9311)

* Add new tests

* Use state instead of param and disallow comma in [...a,]=[]

* Unify error messages

* Object destructuring

* Update whitelist
This commit is contained in:
Nicolò Ribaudo 2019-01-11 13:08:38 +01:00 committed by GitHub
parent 28b70e5910
commit 9764718c32
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
31 changed files with 312 additions and 262 deletions

View File

@ -130,6 +130,9 @@ export default class ExpressionParser extends LValParser {
return left; return left;
} }
const oldCommaAfterSpreadAt = this.state.commaAfterSpreadAt;
this.state.commaAfterSpreadAt = -1;
let failOnShorthandAssign; let failOnShorthandAssign;
if (refShorthandDefaultPos) { if (refShorthandDefaultPos) {
failOnShorthandAssign = false; failOnShorthandAssign = false;
@ -169,21 +172,26 @@ export default class ExpressionParser extends LValParser {
this.checkLVal(left, undefined, undefined, "assignment expression"); this.checkLVal(left, undefined, undefined, "assignment expression");
if (left.extra && left.extra.parenthesized) { let patternErrorMsg;
let errorMsg; let elementName;
if (left.type === "ObjectPattern") { if (left.type === "ObjectPattern") {
errorMsg = "`({a}) = 0` use `({a} = 0)`"; patternErrorMsg = "`({a}) = 0` use `({a} = 0)`";
} else if (left.type === "ArrayPattern") { elementName = "property";
errorMsg = "`([a]) = 0` use `([a] = 0)`"; } else if (left.type === "ArrayPattern") {
} patternErrorMsg = "`([a]) = 0` use `([a] = 0)`";
if (errorMsg) { elementName = "element";
this.raise(
left.start,
`You're trying to assign to a parenthesized expression, eg. instead of ${errorMsg}`,
);
}
} }
if (patternErrorMsg && left.extra && left.extra.parenthesized) {
this.raise(
left.start,
`You're trying to assign to a parenthesized expression, eg. instead of ${patternErrorMsg}`,
);
}
if (elementName) this.checkCommaAfterRestFromSpread(elementName);
this.state.commaAfterSpreadAt = oldCommaAfterSpreadAt;
this.next(); this.next();
node.right = this.parseMaybeAssign(noIn); node.right = this.parseMaybeAssign(noIn);
return this.finishNode(node, "AssignmentExpression"); return this.finishNode(node, "AssignmentExpression");
@ -191,6 +199,8 @@ export default class ExpressionParser extends LValParser {
this.unexpected(refShorthandDefaultPos.start); this.unexpected(refShorthandDefaultPos.start);
} }
this.state.commaAfterSpreadAt = oldCommaAfterSpreadAt;
return left; return left;
} }
@ -580,15 +590,12 @@ export default class ExpressionParser extends LValParser {
let node = this.startNodeAt(startPos, startLoc); let node = this.startNodeAt(startPos, startLoc);
node.callee = base; node.callee = base;
// TODO: Clean up/merge this into `this.state` or a class like acorn's const oldCommaAfterSpreadAt = this.state.commaAfterSpreadAt;
// `DestructuringErrors` alongside refShorthandDefaultPos and this.state.commaAfterSpreadAt = -1;
// refNeedsArrowPos.
const refTrailingCommaPos: Pos = { start: -1 };
node.arguments = this.parseCallExpressionArguments( node.arguments = this.parseCallExpressionArguments(
tt.parenR, tt.parenR,
possibleAsync, possibleAsync,
refTrailingCommaPos,
); );
if (!state.optionalChainMember) { if (!state.optionalChainMember) {
this.finishCallExpression(node); this.finishCallExpression(node);
@ -599,12 +606,7 @@ export default class ExpressionParser extends LValParser {
if (possibleAsync && this.shouldParseAsyncArrow()) { if (possibleAsync && this.shouldParseAsyncArrow()) {
state.stop = true; state.stop = true;
if (refTrailingCommaPos.start > -1) { this.checkCommaAfterRestFromSpread("parameter");
this.raise(
refTrailingCommaPos.start,
"A trailing comma is not permitted after the rest element",
);
}
node = this.parseAsyncArrowFromCallExpression( node = this.parseAsyncArrowFromCallExpression(
this.startNodeAt(startPos, startLoc), this.startNodeAt(startPos, startLoc),
@ -621,6 +623,7 @@ export default class ExpressionParser extends LValParser {
} }
this.state.maybeInArrowParameters = oldMaybeInArrowParameters; this.state.maybeInArrowParameters = oldMaybeInArrowParameters;
this.state.commaAfterSpreadAt = oldCommaAfterSpreadAt;
return node; return node;
} else if (this.match(tt.backQuote)) { } else if (this.match(tt.backQuote)) {
@ -700,7 +703,6 @@ export default class ExpressionParser extends LValParser {
parseCallExpressionArguments( parseCallExpressionArguments(
close: TokenType, close: TokenType,
possibleAsyncArrow: boolean, possibleAsyncArrow: boolean,
refTrailingCommaPos?: Pos,
): $ReadOnlyArray<?N.Expression> { ): $ReadOnlyArray<?N.Expression> {
const elts = []; const elts = [];
let innerParenStart; let innerParenStart;
@ -725,7 +727,6 @@ export default class ExpressionParser extends LValParser {
false, false,
possibleAsyncArrow ? { start: 0 } : undefined, possibleAsyncArrow ? { start: 0 } : undefined,
possibleAsyncArrow ? { start: 0 } : undefined, possibleAsyncArrow ? { start: 0 } : undefined,
possibleAsyncArrow ? refTrailingCommaPos : undefined,
), ),
); );
} }
@ -1192,14 +1193,7 @@ export default class ExpressionParser extends LValParser {
), ),
); );
if (this.match(tt.comma)) { this.checkCommaAfterRest(tt.parenR, "parameter");
const nextTokenType = this.lookahead().type;
const errorMessage =
nextTokenType === tt.parenR
? "A trailing comma is not permitted after the rest element"
: "Rest parameter must be last formal parameter";
this.raise(this.state.start, errorMessage);
}
break; break;
} else { } else {
@ -1398,8 +1392,6 @@ export default class ExpressionParser extends LValParser {
node.properties = []; node.properties = [];
this.next(); this.next();
let firstRestLocation = null;
while (!this.eat(tt.braceR)) { while (!this.eat(tt.braceR)) {
if (first) { if (first) {
first = false; first = false;
@ -1435,34 +1427,14 @@ export default class ExpressionParser extends LValParser {
if (this.match(tt.ellipsis)) { if (this.match(tt.ellipsis)) {
prop = this.parseSpread(isPattern ? { start: 0 } : undefined); prop = this.parseSpread(isPattern ? { start: 0 } : undefined);
if (isPattern) {
this.toAssignable(prop, true, "object pattern");
}
node.properties.push(prop); node.properties.push(prop);
if (isPattern) { if (isPattern) {
const position = this.state.start; this.toAssignable(prop, true, "object pattern");
if (firstRestLocation !== null) { this.checkCommaAfterRest(tt.braceR, "property");
this.unexpected( this.expect(tt.braceR);
firstRestLocation, break;
"Cannot have multiple rest elements when destructuring",
);
} else if (this.eat(tt.braceR)) {
break;
} else if (
this.match(tt.comma) &&
this.lookahead().type === tt.braceR
) {
this.unexpected(
position,
"A trailing comma is not permitted after the rest element",
);
} else {
firstRestLocation = position;
continue;
}
} else {
continue;
} }
continue;
} }
prop.method = false; prop.method = false;
@ -1519,13 +1491,6 @@ export default class ExpressionParser extends LValParser {
node.properties.push(prop); node.properties.push(prop);
} }
if (firstRestLocation !== null) {
this.unexpected(
firstRestLocation,
"The rest element has to be the last element when destructuring",
);
}
if (decorators.length) { if (decorators.length) {
this.raise( this.raise(
this.state.start, this.state.start,
@ -1923,7 +1888,6 @@ export default class ExpressionParser extends LValParser {
allowEmpty: ?boolean, allowEmpty: ?boolean,
refShorthandDefaultPos: ?Pos, refShorthandDefaultPos: ?Pos,
refNeedsArrowPos: ?Pos, refNeedsArrowPos: ?Pos,
refTrailingCommaPos?: Pos,
): ?N.Expression { ): ?N.Expression {
let elt; let elt;
if (allowEmpty && this.match(tt.comma)) { if (allowEmpty && this.match(tt.comma)) {
@ -1936,10 +1900,6 @@ export default class ExpressionParser extends LValParser {
spreadNodeStartPos, spreadNodeStartPos,
spreadNodeStartLoc, spreadNodeStartLoc,
); );
if (refTrailingCommaPos && this.match(tt.comma)) {
refTrailingCommaPos.start = this.state.start;
}
} else { } else {
elt = this.parseMaybeAssign( elt = this.parseMaybeAssign(
false, false,

View File

@ -122,10 +122,7 @@ export default class LValParser extends NodeUtils {
this.raise(prop.key.start, error); this.raise(prop.key.start, error);
} else if (prop.type === "SpreadElement" && !isLast) { } else if (prop.type === "SpreadElement" && !isLast) {
this.raise( this.raiseRestNotLast(prop.start, "property");
prop.start,
"The rest element has to be the last element when destructuring",
);
} else { } else {
this.toAssignable(prop, isBinding, "object destructuring pattern"); this.toAssignable(prop, isBinding, "object destructuring pattern");
} }
@ -162,13 +159,12 @@ export default class LValParser extends NodeUtils {
} }
for (let i = 0; i < end; i++) { for (let i = 0; i < end; i++) {
const elt = exprList[i]; const elt = exprList[i];
if (elt && elt.type === "SpreadElement") { if (elt) {
this.raise( this.toAssignable(elt, isBinding, contextDescription);
elt.start, if (elt.type === "RestElement") {
"The rest element has to be the last element when destructuring", this.raiseRestNotLast(elt.start, "element");
); }
} }
if (elt) this.toAssignable(elt, isBinding, contextDescription);
} }
return exprList; return exprList;
} }
@ -199,10 +195,10 @@ export default class LValParser extends NodeUtils {
// Parses spread element. // Parses spread element.
parseSpread<T: RestElement | SpreadElement>( parseSpread(
refShorthandDefaultPos: ?Pos, refShorthandDefaultPos: ?Pos,
refNeedsArrowPos?: ?Pos, refNeedsArrowPos?: ?Pos,
): T { ): SpreadElement {
const node = this.startNode(); const node = this.startNode();
this.next(); this.next();
node.argument = this.parseMaybeAssign( node.argument = this.parseMaybeAssign(
@ -211,6 +207,11 @@ export default class LValParser extends NodeUtils {
undefined, undefined,
refNeedsArrowPos, refNeedsArrowPos,
); );
if (this.state.commaAfterSpreadAt === -1 && this.match(tt.comma)) {
this.state.commaAfterSpreadAt = this.state.start;
}
return this.finishNode(node, "SpreadElement"); return this.finishNode(node, "SpreadElement");
} }
@ -273,20 +274,13 @@ export default class LValParser extends NodeUtils {
break; break;
} else if (this.match(tt.ellipsis)) { } else if (this.match(tt.ellipsis)) {
elts.push(this.parseAssignableListItemTypes(this.parseRest())); elts.push(this.parseAssignableListItemTypes(this.parseRest()));
if ( this.checkCommaAfterRest(
this.state.inFunction && close,
this.state.inParameters && this.state.inFunction && this.state.inParameters
this.match(tt.comma) ? "parameter"
) { : "element",
const nextTokenType = this.lookahead().type; );
const errorMessage = this.expect(close);
nextTokenType === tt.parenR
? "A trailing comma is not permitted after the rest element"
: "Rest parameter must be last formal parameter";
this.raise(this.state.start, errorMessage);
} else {
this.expect(close);
}
break; break;
} else { } else {
const decorators = []; const decorators = [];
@ -440,4 +434,28 @@ export default class LValParser extends NodeUtils {
this.raise(node.argument.start, "Invalid rest operator's argument"); this.raise(node.argument.start, "Invalid rest operator's argument");
} }
checkCommaAfterRest(close: TokenType, kind: string): void {
if (this.match(tt.comma)) {
if (this.lookahead().type === close) {
this.raiseCommaAfterRest(this.state.start, kind);
} else {
this.raiseRestNotLast(this.state.start, kind);
}
}
}
checkCommaAfterRestFromSpread(kind: string): void {
if (this.state.commaAfterSpreadAt > -1) {
this.raiseCommaAfterRest(this.state.commaAfterSpreadAt, kind);
}
}
raiseCommaAfterRest(pos: number, kind: string) {
this.raise(pos, `A trailing comma is not permitted after the rest ${kind}`);
}
raiseRestNotLast(pos: number, kind: string) {
this.raise(pos, `The rest ${kind} must be the last ${kind}`);
}
} }

View File

@ -546,17 +546,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
// No mandatory elements may follow optional elements // No mandatory elements may follow optional elements
// If there's a rest element, it must be at the end of the tuple // If there's a rest element, it must be at the end of the tuple
let seenOptionalElement = false; let seenOptionalElement = false;
node.elementTypes.forEach((elementNode, i) => { node.elementTypes.forEach(elementNode => {
if (elementNode.type === "TSRestType") { if (elementNode.type === "TSOptionalType") {
if (i !== node.elementTypes.length - 1) {
this.raise(
elementNode.start,
"A rest element must be last in a tuple type.",
);
}
} else if (elementNode.type === "TSOptionalType") {
seenOptionalElement = true; seenOptionalElement = true;
} else if (seenOptionalElement) { } else if (seenOptionalElement && elementNode.type !== "TSRestType") {
this.raise( this.raise(
elementNode.start, elementNode.start,
"A required element cannot follow an optional element.", "A required element cannot follow an optional element.",
@ -573,6 +566,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
const restNode: N.TsRestType = this.startNode(); const restNode: N.TsRestType = this.startNode();
this.next(); // skips ellipsis this.next(); // skips ellipsis
restNode.typeAnnotation = this.tsParseType(); restNode.typeAnnotation = this.tsParseType();
this.checkCommaAfterRest(tt.bracketR, "type");
return this.finishNode(restNode, "TSRestType"); return this.finishNode(restNode, "TSRestType");
} }

View File

@ -15,6 +15,8 @@ export default class State {
this.input = input; this.input = input;
this.commaAfterSpreadAt = -1;
this.potentialArrowAt = -1; this.potentialArrowAt = -1;
this.noArrowAt = []; this.noArrowAt = [];
@ -87,6 +89,11 @@ export default class State {
// TODO // TODO
input: string; input: string;
// A comma after "...a" is only allowed in spread, but not in rest.
// Since we parse destructuring patterns as array/object literals
// and then convert them, we need to track it.
commaAfterSpreadAt: number;
// Used to signify the start of a potential arrow function // Used to signify the start of a potential arrow function
potentialArrowAt: number; potentialArrowAt: number;

View File

@ -1,3 +1,3 @@
{ {
"throws": "Rest parameter must be last formal parameter (1:18)" "throws": "The rest parameter must be the last parameter (1:18)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "Rest parameter must be last formal parameter (3:13)" "throws": "The rest parameter must be the last parameter (3:13)"
} }

View File

@ -0,0 +1 @@
[...a,] = [];

View File

@ -0,0 +1,99 @@
{
"type": "File",
"start": 0,
"end": 8,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 8
}
},
"program": {
"type": "Program",
"start": 0,
"end": 8,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 8
}
},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start": 0,
"end": 8,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 8
}
},
"expression": {
"type": "ArrayExpression",
"start": 0,
"end": 7,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 7
}
},
"elements": [
{
"type": "SpreadElement",
"start": 1,
"end": 5,
"loc": {
"start": {
"line": 1,
"column": 1
},
"end": {
"line": 1,
"column": 5
}
},
"argument": {
"type": "Identifier",
"start": 4,
"end": 5,
"loc": {
"start": {
"line": 1,
"column": 4
},
"end": {
"line": 1,
"column": 5
},
"identifierName": "a"
},
"name": "a"
}
}
]
}
}
],
"directives": []
}
}

View File

@ -1,3 +1,3 @@
{ {
"throws": "The rest element has to be the last element when destructuring (1:1)" "throws": "The rest element must be the last element (1:1)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "A trailing comma is not permitted after the rest element (1:8)" "throws": "A trailing comma is not permitted after the rest parameter (1:8)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "Rest parameter must be last formal parameter (3:13)" "throws": "The rest parameter must be the last parameter (3:13)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "Rest parameter must be last formal parameter (1:18)" "throws": "The rest parameter must be the last parameter (1:18)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "Rest parameter must be last formal parameter (1:5)" "throws": "The rest parameter must be the last parameter (1:5)"
} }

View File

@ -1 +0,0 @@
[...a, ] = b

View File

@ -1,132 +0,0 @@
{
"type": "File",
"start": 0,
"end": 12,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 12
}
},
"program": {
"type": "Program",
"start": 0,
"end": 12,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 12
}
},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start": 0,
"end": 12,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 12
}
},
"expression": {
"type": "AssignmentExpression",
"start": 0,
"end": 12,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 12
}
},
"operator": "=",
"left": {
"type": "ArrayPattern",
"start": 0,
"end": 8,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 8
}
},
"elements": [
{
"type": "RestElement",
"start": 1,
"end": 5,
"loc": {
"start": {
"line": 1,
"column": 1
},
"end": {
"line": 1,
"column": 5
}
},
"argument": {
"type": "Identifier",
"start": 4,
"end": 5,
"loc": {
"start": {
"line": 1,
"column": 4
},
"end": {
"line": 1,
"column": 5
},
"identifierName": "a"
},
"name": "a"
}
}
]
},
"right": {
"type": "Identifier",
"start": 11,
"end": 12,
"loc": {
"start": {
"line": 1,
"column": 11
},
"end": {
"line": 1,
"column": 12
},
"identifierName": "b"
},
"name": "b"
}
}
}
],
"directives": []
}
}

View File

@ -1,3 +1,3 @@
{ {
"throws": "A trailing comma is not permitted after the rest element (1:11)" "throws": "A trailing comma is not permitted after the rest parameter (1:11)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "The rest element has to be the last element when destructuring (1:10)" "throws": "The rest property must be the last property (1:10)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "A trailing comma is not permitted after the rest element (1:16)" "throws": "A trailing comma is not permitted after the rest property (1:16)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "Cannot have multiple rest elements when destructuring (1:13)" "throws": "The rest property must be the last property (1:13)"
} }

View File

@ -0,0 +1 @@
({...a,} = {});

View File

@ -0,0 +1,3 @@
{
"throws": "A trailing comma is not permitted after the rest property (1:6)"
}

View File

@ -0,0 +1 @@
({...a,});

View File

@ -0,0 +1,103 @@
{
"type": "File",
"start": 0,
"end": 10,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 10
}
},
"program": {
"type": "Program",
"start": 0,
"end": 10,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 10
}
},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start": 0,
"end": 10,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 10
}
},
"expression": {
"type": "ObjectExpression",
"start": 1,
"end": 8,
"loc": {
"start": {
"line": 1,
"column": 1
},
"end": {
"line": 1,
"column": 8
}
},
"properties": [
{
"type": "SpreadElement",
"start": 2,
"end": 6,
"loc": {
"start": {
"line": 1,
"column": 2
},
"end": {
"line": 1,
"column": 6
}
},
"argument": {
"type": "Identifier",
"start": 5,
"end": 6,
"loc": {
"start": {
"line": 1,
"column": 5
},
"end": {
"line": 1,
"column": 6
},
"identifierName": "a"
},
"name": "a"
}
}
],
"extra": {
"parenthesized": true,
"parenStart": 0
}
}
}
],
"directives": []
}
}

View File

@ -1,3 +1,3 @@
{ {
"throws": "The rest element has to be the last element when destructuring (1:2)" "throws": "The rest property must be the last property (1:2)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "Rest parameter must be last formal parameter (1:5)" "throws": "The rest parameter must be the last parameter (1:5)"
} }

View File

@ -1,3 +1,3 @@
{ {
"throws": "Rest parameter must be last formal parameter (1:18)" "throws": "The rest parameter must be the last parameter (1:18)"
} }

View File

@ -1,5 +1,5 @@
{ {
"sourceType": "module", "sourceType": "module",
"plugins": ["typescript"], "plugins": ["typescript"],
"throws": "A rest element must be last in a tuple type. (1:8)" "throws": "The rest type must be the last type (1:19)"
} }

View File

@ -649,10 +649,6 @@ language/block-scope/syntax/redeclaration/var-redeclaration-attempt-after-let.js
language/block-scope/syntax/redeclaration/var-redeclaration-attempt-after-let.js(strict mode) language/block-scope/syntax/redeclaration/var-redeclaration-attempt-after-let.js(strict mode)
language/expressions/assignment/destructuring/obj-prop-__proto__dup.js(default) language/expressions/assignment/destructuring/obj-prop-__proto__dup.js(default)
language/expressions/assignment/destructuring/obj-prop-__proto__dup.js(strict mode) language/expressions/assignment/destructuring/obj-prop-__proto__dup.js(strict mode)
language/expressions/assignment/dstr/array-rest-before-elision.js(default)
language/expressions/assignment/dstr/array-rest-before-elision.js(strict mode)
language/expressions/assignment/dstr/array-rest-elision-invalid.js(default)
language/expressions/assignment/dstr/array-rest-elision-invalid.js(strict mode)
language/expressions/assignment/dstr/obj-id-identifier-yield-ident-valid.js(default) language/expressions/assignment/dstr/obj-id-identifier-yield-ident-valid.js(default)
language/expressions/async-arrow-function/await-as-param-ident-nested-arrow-parameter-position.js(default) language/expressions/async-arrow-function/await-as-param-ident-nested-arrow-parameter-position.js(default)
language/expressions/async-arrow-function/await-as-param-ident-nested-arrow-parameter-position.js(strict mode) language/expressions/async-arrow-function/await-as-param-ident-nested-arrow-parameter-position.js(strict mode)