Skip to content

Commit

Permalink
Reland "Fix: check if param is assignable when parsing arrow return t…
Browse files Browse the repository at this point in the history
…ype annotation" (#12183)

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

This reverts commit 91a7a64.

* Mark `RestElement` as assignable
  • Loading branch information
nicolo-ribaudo committed Oct 15, 2020
1 parent 6cb0056 commit 84987a0
Show file tree
Hide file tree
Showing 11 changed files with 474 additions and 47 deletions.
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"]
}

0 comments on commit 84987a0

Please sign in to comment.