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

Reland "Fix: check if param is assignable when parsing arrow return type annotation" #12183

Merged
merged 2 commits into from Oct 15, 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
7 changes: 5 additions & 2 deletions packages/babel-parser/src/parser/expression.js
Expand Up @@ -1453,7 +1453,7 @@ export default class ExpressionParser extends LValParser {
if (
canBeArrow &&
this.shouldParseArrow() &&
(arrowNode = this.parseArrow(arrowNode))
(arrowNode = this.parseArrow(arrowNode, exprList))
) {
if (!this.isAwaitAllowed() && !this.state.maybeInAsyncArrowHead) {
this.state.awaitPos = oldAwaitPos;
Expand Down Expand Up @@ -1509,7 +1509,10 @@ export default class ExpressionParser extends LValParser {
return !this.canInsertSemicolon();
}

parseArrow(node: N.ArrowFunctionExpression): ?N.ArrowFunctionExpression {
parseArrow(
node: N.ArrowFunctionExpression,
exprList: N.Node[], // eslint-disable-line no-unused-vars
): ?N.ArrowFunctionExpression {
if (this.eat(tt.arrow)) {
return node;
}
Expand Down
54 changes: 52 additions & 2 deletions packages/babel-parser/src/parser/lval.js
Expand Up @@ -51,10 +51,60 @@ export default class LValParser extends NodeUtils {
+parseDecorator: () => Decorator;
*/

/**
* Check if a node can be converted to a binding identifier or binding pattern.
* https://tc39.es/ecma262/#prod-BindingIdentifier
* https://tc39.es/ecma262/#prod-BindingPattern
* Note that although a mebmer expression can serve as a LHS in the init of for loop,
* i.e. `for (a.b of []);`, it is not a binding pattern
*
* @param {Node} node
* @returns {boolean}
* @memberof LValParser
*/
isAssignable(node: Node): boolean {
switch (node.type) {
case "Identifier":
case "ObjectPattern":
case "ArrayPattern":
case "AssignmentPattern":
case "RestElement":
return true;

case "ObjectExpression": {
const last = node.properties.length - 1;
return node.properties.every((prop, i) => {
return (
prop.type !== "ObjectMethod" &&
(i === last || prop.type === "SpreadElement") &&
this.isAssignable(prop)
);
});
}

case "ObjectProperty":
return this.isAssignable(node.value);

case "SpreadElement":
return this.isAssignable(node.argument);

case "ArrayExpression":
return node.elements.every(element => this.isAssignable(element));

case "AssignmentExpression":
return node.operator === "=";

case "ParenthesizedExpression":
return this.isAssignable(node.expression);

default:
return false;
}
}

// Convert existing expression atom to assignable pattern
// if possible.
// NOTE: There is a corresponding "isAssignable" method in flow.js.
// When this one is updated, please check if also that one needs to be updated.
// When this one is updated, please check if `isAssignable` also needs to be updated.

toAssignable(node: Node): Node {
let parenthesized = undefined;
Expand Down
51 changes: 10 additions & 41 deletions packages/babel-parser/src/plugins/flow.js
Expand Up @@ -1943,7 +1943,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}

return partition(arrows, node =>
node.params.every(param => this.isAssignable(param, true)),
node.params.every(param => this.isAssignable(param)),
);
}

Expand Down Expand Up @@ -2146,47 +2146,12 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}
}

isAssignable(node: N.Node, isBinding?: boolean): boolean {
isAssignable(node: N.Node): boolean {
switch (node.type) {
case "Identifier":
case "ObjectPattern":
case "ArrayPattern":
case "AssignmentPattern":
return true;

case "ObjectExpression": {
const last = node.properties.length - 1;
return node.properties.every((prop, i) => {
return (
prop.type !== "ObjectMethod" &&
(i === last || prop.type === "SpreadElement") &&
this.isAssignable(prop)
);
});
}

case "ObjectProperty":
return this.isAssignable(node.value);

case "SpreadElement":
return this.isAssignable(node.argument);

case "ArrayExpression":
return node.elements.every(element => this.isAssignable(element));

case "AssignmentExpression":
return node.operator === "=";

case "ParenthesizedExpression":
case "TypeCastExpression":
return this.isAssignable(node.expression);

case "MemberExpression":
case "OptionalMemberExpression":
return !isBinding;

default:
return false;
return super.isAssignable(node);
}
}

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

// handle return types for arrow functions
parseArrow(node: N.ArrowFunctionExpression): ?N.ArrowFunctionExpression {
parseArrow(
node: N.ArrowFunctionExpression,
exprList: N.Node[],
): ?N.ArrowFunctionExpression {
if (this.match(tt.colon)) {
const result = this.tryParse(() => {
const oldNoAnonFunctionType = this.state.noAnonFunctionType;
Expand Down Expand Up @@ -2806,7 +2774,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
: null;
}

return super.parseArrow(node);
return super.parseArrow(node, exprList);
}

shouldParseArrow(): boolean {
Expand Down Expand Up @@ -2977,7 +2945,8 @@ export default (superClass: Class<Parser>): Class<Parser> =>
): ?N.ArrowFunctionExpression {
const node = this.startNodeAt(startPos, startLoc);
this.parseFunctionParams(node);
if (!this.parseArrow(node)) return;
// set exprList to `[]` as the parameters has been validated in `parseFunctionParams`
if (!this.parseArrow(node, [])) return;
return this.parseArrowExpression(
node,
/* params */ undefined,
Expand Down
23 changes: 21 additions & 2 deletions packages/babel-parser/src/plugins/typescript/index.js
Expand Up @@ -2540,7 +2540,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}
}

parseArrow(node: N.ArrowFunctionExpression): ?N.ArrowFunctionExpression {
parseArrow(
node: N.ArrowFunctionExpression,
exprList,
): ?N.ArrowFunctionExpression {
if (this.match(tt.colon)) {
// This is different from how the TS parser does it.
// TS uses lookahead. The Babel Parser parses it as a parenthesized expression and converts.
Expand All @@ -2550,6 +2553,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
tt.colon,
);
if (this.canInsertSemicolon() || !this.match(tt.arrow)) abort();
// check if the exprList is assignable because `: TSType` can be part of conditional expression
// i.e. we can only know `: v` is not a return type by checking that `sum(v)` can not be a pattern.
// 0 ? v => (sum(v)) : v => 0
if (exprList.some(param => !this.isAssignable(param))) abort();
return returnType;
});

Expand All @@ -2561,7 +2568,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}
}

return super.parseArrow(node);
return super.parseArrow(node, exprList);
}

// Allow type annotations inside of a parameter list.
Expand All @@ -2580,6 +2587,18 @@ export default (superClass: Class<Parser>): Class<Parser> =>
return param;
}

isAssignable(node: N.Node): boolean {
switch (node.type) {
case "TSAsExpression":
case "TSNonNullExpression":
case "TSTypeAssertion":
case "TSTypeCastExpression":
return this.isAssignable(node.expression);
default:
return super.isAssignable(node);
}
}

toAssignable(node: N.Node): N.Node {
switch (node.type) {
case "TSTypeCastExpression":
Expand Down
@@ -0,0 +1 @@
0 ? v => (v) : v => 0;
@@ -0,0 +1,7 @@
{
"sourceType": "module",
"plugins": [
"typescript"
],
"throws": "Unexpected token, expected \":\" (1:21)"
}
@@ -0,0 +1,4 @@
0 ? v => (sum += v) : v => 0;
0 ? v => (sum(v)) : v => 0;
0 ? v => (v.w) : v => 0;
0 ? v => ([ v.w ]) : v => {};
@@ -0,0 +1,3 @@
{
"plugins": ["typescript"]
}