Skip to content
This repository was archived by the owner on May 19, 2018. It is now read-only.

Commit a9a55fb

Browse files
nicolo-ribaudohzoo
nicolo-ribaudo
authored and
hzoo
committed
Distinguish between ternary's : and arrow fn's return type (#573)
* Distinguish between ternary's : and arrow fn's return type * Correctly parse nested arrow functions inside conditional expressions Defer the conversion of arrow function parameters to assignable nodes so that it is possible to use the (invalid) ast to get the exact position of the (wrong) arrow functions. * Check params of arrow fns w/ type params or w/o return type * Fix also async functions * Add test from prettier prettier/prettier#2194 * Don't check arrow params if they are valid at the first attemp * Use state instead of relying on the "noArrowParamsConversion" parameter * Remove noArrowParamsConversion
1 parent 39447b1 commit a9a55fb

15 files changed

Lines changed: 6840 additions & 10 deletions

File tree

‎src/parser/expression.js‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,11 +1081,15 @@ export default class ExpressionParser extends LValParser {
10811081

10821082
parseArrowExpression(node: N.ArrowFunctionExpression, params: N.Expression[], isAsync?: boolean): N.ArrowFunctionExpression {
10831083
this.initFunction(node, isAsync);
1084-
node.params = this.toAssignableList(params, true, "arrow function parameters");
1084+
this.setArrowFunctionParameters(node, params);
10851085
this.parseFunctionBody(node, true);
10861086
return this.finishNode(node, "ArrowFunctionExpression");
10871087
}
10881088

1089+
setArrowFunctionParameters(node: N.ArrowFunctionExpression, params: N.Expression[]): N.ArrowFunctionExpression {
1090+
node.params = this.toAssignableList(params, true, "arrow function parameters");
1091+
}
1092+
10891093
isStrictBody(node: { body: N.BlockStatement }, isExpression?: boolean): boolean {
10901094
if (!isExpression && node.body.directives.length) {
10911095
for (const directive of node.body.directives) {
@@ -1120,12 +1124,19 @@ export default class ExpressionParser extends LValParser {
11201124
}
11211125
this.state.inAsync = oldInAsync;
11221126

1127+
this.checkFunctionNameAndParams(node, allowExpression);
1128+
}
1129+
1130+
checkFunctionNameAndParams(
1131+
node: N.Function,
1132+
isArrowFunction?: boolean
1133+
): void {
11231134
// If this is a strict mode function, verify that argument names
11241135
// are not repeated, and it does not try to bind the words `eval`
11251136
// or `arguments`.
1126-
const isStrict = this.isStrictBody(node, isExpression);
1127-
// Also check when allowExpression === true for arrow functions
1128-
const checkLVal = this.state.strict || allowExpression || isStrict;
1137+
const isStrict = this.isStrictBody(node, node.expression);
1138+
// Also check for arrow functions
1139+
const checkLVal = this.state.strict || isStrict || isArrowFunction;
11291140

11301141
if (isStrict && node.id && node.id.type === "Identifier" && node.id.name === "yield") {
11311142
this.raise(node.id.start, "Binding yield in strict mode");

‎src/plugins/flow.js‎

Lines changed: 186 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ const exportSuggestions = {
3737
interface: "export interface",
3838
};
3939

40+
// Like Array#filter, but returns a tuple [ acceptedElements, discardedElements ]
41+
function partition<T>(list: T[], test: (T, number, T[]) => ?boolean): [ T[], T[] ] {
42+
const list1 = [];
43+
const list2 = [];
44+
for (let i = 0; i < list.length; i++) {
45+
(test(list[i], i, list) ? list1 : list2).push(list[i]);
46+
}
47+
return [ list1, list2 ];
48+
}
49+
4050
export default (superClass: Class<Parser>): Class<Parser> => class extends superClass {
4151
flowParseTypeInitialiser(tok?: TokenType): N.FlowType {
4252
const oldInType = this.state.inType;
@@ -1007,9 +1017,15 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
10071017
// Overrides
10081018
// ==================================
10091019

1010-
// plain function return types: function name(): string {}
10111020
parseFunctionBody(node: N.Function, allowExpression?: boolean): void {
1012-
if (this.match(tt.colon) && !allowExpression) {
1021+
if (allowExpression && this.state.noArrowParamsConversionAt.indexOf(node.start) !== -1) {
1022+
this.state.noArrowParamsConversionAt.push(this.state.start);
1023+
super.parseFunctionBody(node, allowExpression);
1024+
this.state.noArrowParamsConversionAt.pop();
1025+
1026+
return;
1027+
} else if (this.match(tt.colon)) {
1028+
// plain function return types: function name(): string {}
10131029
// if allowExpression is true then we're parsing an arrow function and if
10141030
// there's a return type then it's been handled elsewhere
10151031
const typeNode = this.startNode();
@@ -1074,9 +1090,11 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
10741090
}
10751091

10761092
parseConditional(expr: N.Expression, noIn: ?boolean, startPos: number, startLoc: Position, refNeedsArrowPos?: ?Pos): N.Expression {
1093+
if (!this.match(tt.question)) return expr;
1094+
10771095
// only do the expensive clone if there is a question mark
10781096
// and if we come from inside parens
1079-
if (refNeedsArrowPos && this.match(tt.question)) {
1097+
if (refNeedsArrowPos) {
10801098
const state = this.state.clone();
10811099
try {
10821100
return super.parseConditional(expr, noIn, startPos, startLoc);
@@ -1092,7 +1110,129 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
10921110
}
10931111
}
10941112

1095-
return super.parseConditional(expr, noIn, startPos, startLoc);
1113+
this.expect(tt.question);
1114+
const state = this.state.clone();
1115+
const originalNoArrowAt = this.state.noArrowAt;
1116+
const node = this.startNodeAt(startPos, startLoc);
1117+
let { consequent, failed } = this.tryParseConditionalConsequent();
1118+
let [ valid, invalid ] = this.getArrowLikeExpressions(consequent);
1119+
1120+
if (failed || invalid.length > 0) {
1121+
const noArrowAt = [ ...originalNoArrowAt ];
1122+
1123+
if (invalid.length > 0) {
1124+
this.state = state;
1125+
this.state.noArrowAt = noArrowAt;
1126+
1127+
for (let i = 0; i < invalid.length; i++) {
1128+
noArrowAt.push(invalid[i].start);
1129+
}
1130+
1131+
({ consequent, failed } = this.tryParseConditionalConsequent());
1132+
[ valid, invalid ] = this.getArrowLikeExpressions(consequent);
1133+
}
1134+
1135+
if (failed && valid.length > 1) {
1136+
// if there are two or more possible correct ways of parsing, throw an
1137+
// error.
1138+
// e.g. Source: a ? (b): c => (d): e => f
1139+
// Result 1: a ? b : (c => ((d): e => f))
1140+
// Result 2: a ? ((b): c => d) : (e => f)
1141+
this.raise(
1142+
state.start,
1143+
"Ambiguous expression: wrap the arrow functions in parentheses to disambiguate."
1144+
);
1145+
}
1146+
1147+
if (failed && valid.length === 1) {
1148+
this.state = state;
1149+
this.state.noArrowAt = noArrowAt.concat(valid[0].start);
1150+
({ consequent, failed } = this.tryParseConditionalConsequent());
1151+
}
1152+
1153+
this.getArrowLikeExpressions(consequent, true);
1154+
}
1155+
1156+
this.state.noArrowAt = originalNoArrowAt;
1157+
this.expect(tt.colon);
1158+
1159+
node.test = expr;
1160+
node.consequent = consequent;
1161+
node.alternate = this.forwardNoArrowParamsConversionAt(node, () => (
1162+
this.parseMaybeAssign(noIn, undefined, undefined, undefined)
1163+
));
1164+
1165+
return this.finishNode(node, "ConditionalExpression");
1166+
}
1167+
1168+
tryParseConditionalConsequent(): { consequent: N.Expression, failed: boolean } {
1169+
this.state.noArrowParamsConversionAt.push(this.state.start);
1170+
1171+
const consequent = this.parseMaybeAssign();
1172+
const failed = !this.match(tt.colon);
1173+
1174+
this.state.noArrowParamsConversionAt.pop();
1175+
1176+
return { consequent, failed };
1177+
}
1178+
1179+
// Given an expression, walks throught its arrow functions whose body is
1180+
// an expression and throught conditional expressions. It returns every
1181+
// function which has been parsed with a return type but could have been
1182+
// parenthesized expressions.
1183+
// These functions are separated into two arrays: one containing the ones
1184+
// whose parameters can be converted to assignable lists, one containing the
1185+
// others.
1186+
getArrowLikeExpressions(node: N.Expression, disallowInvalid?: boolean): [ N.ArrowFunctionExpression[], N.ArrowFunctionExpression[] ] {
1187+
const stack = [ node ];
1188+
const arrows: N.ArrowFunctionExpression[] = [];
1189+
1190+
while (stack.length !== 0) {
1191+
const node = stack.pop();
1192+
if (node.type === "ArrowFunctionExpression") {
1193+
if (node.typeParameters || !node.returnType) {
1194+
// This is an arrow expression without ambiguity, so check its parameters
1195+
this.toAssignableList(node.params, true, "arrow function parameters");
1196+
// Use super's method to force the parameters to be checked
1197+
super.checkFunctionNameAndParams(node, true);
1198+
} else {
1199+
arrows.push(node);
1200+
}
1201+
stack.push(node.body);
1202+
} else if (node.type === "ConditionalExpression") {
1203+
stack.push(node.consequent);
1204+
stack.push(node.alternate);
1205+
}
1206+
}
1207+
1208+
if (disallowInvalid) {
1209+
for (let i = 0; i < arrows.length; i++) {
1210+
this.toAssignableList(arrows[i].params, true, "arrow function parameters");
1211+
}
1212+
return [ arrows, [] ];
1213+
}
1214+
1215+
return partition(arrows, (node) => {
1216+
try {
1217+
this.toAssignableList(node.params, true, "arrow function parameters");
1218+
return true;
1219+
} catch (err) {
1220+
return false;
1221+
}
1222+
});
1223+
}
1224+
1225+
forwardNoArrowParamsConversionAt<T>(node: N.Node, parse: () => T): T {
1226+
let result: T;
1227+
if (this.state.noArrowParamsConversionAt.indexOf(node.start) !== -1) {
1228+
this.state.noArrowParamsConversionAt.push(this.state.start);
1229+
result = parse();
1230+
this.state.noArrowParamsConversionAt.pop(this.state.start);
1231+
} else {
1232+
result = parse();
1233+
}
1234+
1235+
return result;
10961236
}
10971237

10981238
parseParenItem(node: N.Expression, startPos: number, startLoc: Position): N.Expression {
@@ -1510,8 +1650,9 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
15101650
let typeParameters;
15111651
try {
15121652
typeParameters = this.flowParseTypeParameterDeclaration();
1513-
1514-
arrowExpression = super.parseMaybeAssign(noIn, refShorthandDefaultPos, afterLeftParse, refNeedsArrowPos);
1653+
arrowExpression = this.forwardNoArrowParamsConversionAt(typeParameters, () => (
1654+
super.parseMaybeAssign(noIn, refShorthandDefaultPos, afterLeftParse, refNeedsArrowPos)
1655+
));
15151656
arrowExpression.typeParameters = typeParameters;
15161657
this.resetStartLocationFromNode(arrowExpression, typeParameters);
15171658
} catch (err) {
@@ -1570,4 +1711,43 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
15701711
shouldParseArrow(): boolean {
15711712
return this.match(tt.colon) || super.shouldParseArrow();
15721713
}
1714+
1715+
setArrowFunctionParameters(node: N.ArrowFunctionExpression, params: N.Expression[]): N.ArrowFunctionExpression {
1716+
if (this.state.noArrowParamsConversionAt.indexOf(node.start) !== -1) {
1717+
node.params = params;
1718+
return node;
1719+
}
1720+
1721+
return super.setArrowFunctionParameters(node, params);
1722+
}
1723+
1724+
checkFunctionNameAndParams(node: N.Function, isArrowFunction?: boolean): void {
1725+
if (isArrowFunction && this.state.noArrowParamsConversionAt.indexOf(node.start) !== -1) {
1726+
return;
1727+
}
1728+
1729+
return super.checkFunctionNameAndParams(node, isArrowFunction);
1730+
}
1731+
1732+
parseParenAndDistinguishExpression(canBeArrow: boolean): N.Expression {
1733+
return super.parseParenAndDistinguishExpression(
1734+
canBeArrow && this.state.noArrowAt.indexOf(this.state.start) === -1
1735+
);
1736+
}
1737+
1738+
parseSubscripts(base: N.Expression, startPos: number, startLoc: Position, noCalls?: boolean): N.Expression {
1739+
if (
1740+
base.type === "Identifier" && base.name === "async" &&
1741+
this.state.noArrowAt.indexOf(startPos) !== -1
1742+
) {
1743+
this.next();
1744+
1745+
const node = this.startNodeAt(startPos, startLoc);
1746+
node.callee = base;
1747+
node.arguments = this.parseCallExpressionArguments(tt.parenR, false);
1748+
base = this.finishNode(node, "CallExpression");
1749+
}
1750+
1751+
return super.parseSubscripts(base, startPos, startLoc, noCalls);
1752+
}
15731753
};

‎src/tokenizer/state.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ export default class State {
1818

1919
this.potentialArrowAt = -1;
2020

21+
this.noArrowAt = [];
22+
this.noArrowParamsConversionAt = [];
23+
2124
this.inMethod =
2225
this.inFunction =
2326
this.inGenerator =
@@ -76,6 +79,20 @@ export default class State {
7679
// Used to signify the start of a potential arrow function
7780
potentialArrowAt: number;
7881

82+
// Used to signify the start of an expression which looks like a
83+
// typed arrow function, but it isn't
84+
// e.g. a ? (b) : c => d
85+
// ^
86+
noArrowAt: number[];
87+
88+
// Used to signify the start of an expression whose params, if it looks like
89+
// an arrow function, shouldn't be converted to assignable nodes.
90+
// This is used to defer the validation of typed arrow functions inside
91+
// conditional expressions.
92+
// e.g. a ? (b) : c => d
93+
// ^
94+
noArrowParamsConversionAt: number[];
95+
7996
// Flags to track whether we are in a function, a generator.
8097
inFunction: boolean;
8198
inGenerator: boolean;

‎test/fixtures/flow/regression/issue-58-ambiguous/actual.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// This can be parsed in two ways:
2+
// a ? b : (c => ((d): e => f))
3+
// a ? ((b): c => d) : (e => f)
4+
a ? (b) : c => (d) : e => f;

‎test/fixtures/flow/regression/issue-58-ambiguous/options.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"throws": "Ambiguous expression: wrap the arrow functions in parentheses to disambiguate. (4:4)"
3+
}

‎test/fixtures/flow/regression/issue-58-failing-1/actual.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// Function which looks like a return type
2+
a ? (b) : (c => d) => e;

‎test/fixtures/flow/regression/issue-58-failing-1/options.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"throws": "Invalid left-hand side in arrow function parameters (2:11)"
3+
}

‎test/fixtures/flow/regression/issue-58-failing-2/actual.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// Invalid LHS parameter after type parameters
2+
a ? <T>(b => c) : d => (e) : f => g;

‎test/fixtures/flow/regression/issue-58-failing-2/options.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"throws": "Invalid left-hand side in arrow function parameters (2:8)"
3+
}

‎test/fixtures/flow/regression/issue-58-failing-3/actual.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// Invalid LHS parameter after type parameters
2+
a ? (b => c) => (e) : f => g;

0 commit comments

Comments
 (0)