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

Distinguish between ternary's : and arrow fn's return type #596

Merged
merged 4 commits into from
Sep 25, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions src/parser/expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -1405,13 +1405,20 @@ export default class ExpressionParser extends LValParser {
isAsync?: boolean,
): N.ArrowFunctionExpression {
this.initFunction(node, isAsync);
this.setArrowFunctionParameters(node, params);
this.parseFunctionBody(node, true);
return this.finishNode(node, "ArrowFunctionExpression");
}

setArrowFunctionParameters(
node: N.ArrowFunctionExpression,
params: N.Expression[],
): void {
node.params = this.toAssignableList(
params,
true,
"arrow function parameters",
);
this.parseFunctionBody(node, true);
return this.finishNode(node, "ArrowFunctionExpression");
}

isStrictBody(
Expand Down Expand Up @@ -1465,12 +1472,19 @@ export default class ExpressionParser extends LValParser {
}
this.state.inAsync = oldInAsync;

this.checkFunctionNameAndParams(node, allowExpression);
}

checkFunctionNameAndParams(
node: N.Function,
isArrowFunction: ?boolean,
): void {
// If this is a strict mode function, verify that argument names
// are not repeated, and it does not try to bind the words `eval`
// or `arguments`.
const isStrict = this.isStrictBody(node, isExpression);
// Also check when allowExpression === true for arrow functions
const checkLVal = this.state.strict || allowExpression || isStrict;
const isStrict = this.isStrictBody(node, node.expression);
// Also check for arrow functions
const checkLVal = this.state.strict || isStrict || isArrowFunction;

if (
isStrict &&
Expand Down
238 changes: 230 additions & 8 deletions src/plugins/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@ const exportSuggestions = {
interface: "export interface",
};

// Like Array#filter, but returns a tuple [ acceptedElements, discardedElements ]
function partition<T>(
list: T[],
test: (T, number, T[]) => ?boolean,
): [T[], T[]] {
const list1 = [];
const list2 = [];
for (let i = 0; i < list.length; i++) {
(test(list[i], i, list) ? list1 : list2).push(list[i]);
}
return [list1, list2];
}

export default (superClass: Class<Parser>): Class<Parser> =>
class extends superClass {
flowParseTypeInitialiser(tok?: TokenType): N.FlowType {
Expand Down Expand Up @@ -1144,6 +1157,16 @@ export default (superClass: Class<Parser>): Class<Parser> =>
// Overrides
// ==================================

parseFunctionBody(node: N.Function, allowExpressionBody: ?boolean): void {
if (allowExpressionBody) {
return this.forwardNoArrowParamsConversionAt(node, () =>
super.parseFunctionBody(node, true),
);
}

return super.parseFunctionBody(node, false);
}

parseFunctionBodyAndFinish(
node: N.BodilessFunctionOrMethodBase,
type: string,
Expand Down Expand Up @@ -1239,9 +1262,11 @@ export default (superClass: Class<Parser>): Class<Parser> =>
startLoc: Position,
refNeedsArrowPos?: ?Pos,
): N.Expression {
if (!this.match(tt.question)) return expr;

// only do the expensive clone if there is a question mark
// and if we come from inside parens
if (refNeedsArrowPos && this.match(tt.question)) {
if (refNeedsArrowPos) {
const state = this.state.clone();
try {
return super.parseConditional(expr, noIn, startPos, startLoc);
Expand All @@ -1256,8 +1281,149 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}
}
}
this.expect(tt.question);
const state = this.state.clone();
const originalNoArrowAt = this.state.noArrowAt;
const node = this.startNodeAt(startPos, startLoc);
let { consequent, failed } = this.tryParseConditionalConsequent();
let [valid, invalid] = this.getArrowLikeExpressions(consequent);

if (failed || invalid.length > 0) {
const noArrowAt = [...originalNoArrowAt];

if (invalid.length > 0) {
this.state = state;
this.state.noArrowAt = noArrowAt;

for (let i = 0; i < invalid.length; i++) {
noArrowAt.push(invalid[i].start);
}

({ consequent, failed } = this.tryParseConditionalConsequent());
[valid, invalid] = this.getArrowLikeExpressions(consequent);
}

if (failed && valid.length > 1) {
// if there are two or more possible correct ways of parsing, throw an
// error.
// e.g. Source: a ? (b): c => (d): e => f
// Result 1: a ? b : (c => ((d): e => f))
// Result 2: a ? ((b): c => d) : (e => f)
this.raise(
state.start,
"Ambiguous expression: wrap the arrow functions in parentheses to disambiguate.",
);
}

if (failed && valid.length === 1) {
this.state = state;
this.state.noArrowAt = noArrowAt.concat(valid[0].start);
({ consequent, failed } = this.tryParseConditionalConsequent());
}

this.getArrowLikeExpressions(consequent, true);
}

this.state.noArrowAt = originalNoArrowAt;
this.expect(tt.colon);

node.test = expr;
node.consequent = consequent;
node.alternate = this.forwardNoArrowParamsConversionAt(node, () =>
this.parseMaybeAssign(noIn, undefined, undefined, undefined),
);

return this.finishNode(node, "ConditionalExpression");
}

tryParseConditionalConsequent(): {
consequent: N.Expression,
failed: boolean,
} {
this.state.noArrowParamsConversionAt.push(this.state.start);

const consequent = this.parseMaybeAssign();
const failed = !this.match(tt.colon);

this.state.noArrowParamsConversionAt.pop();

return { consequent, failed };
}

// Given an expression, walks throught its arrow functions whose body is
// an expression and throught conditional expressions. It returns every
// function which has been parsed with a return type but could have been
// parenthesized expressions.
// These functions are separated into two arrays: one containing the ones
// whose parameters can be converted to assignable lists, one containing the
// others.
getArrowLikeExpressions(
node: N.Expression,
disallowInvalid?: boolean,
): [N.ArrowFunctionExpression[], N.ArrowFunctionExpression[]] {
const stack = [node];
const arrows: N.ArrowFunctionExpression[] = [];

while (stack.length !== 0) {
const node = stack.pop();
if (node.type === "ArrowFunctionExpression") {
if (node.typeParameters || !node.returnType) {
// This is an arrow expression without ambiguity, so check its parameters
this.toAssignableList(
// node.params is Expression[] instead of $ReadOnlyArray<Pattern> because it
// has not been converted yet.
((node.params: any): N.Expression[]),
true,
"arrow function parameters",
);
// Use super's method to force the parameters to be checked
super.checkFunctionNameAndParams(node, true);
} else {
arrows.push(node);
}
stack.push(node.body);
} else if (node.type === "ConditionalExpression") {
stack.push(node.consequent);
stack.push(node.alternate);
}
}

if (disallowInvalid) {
for (let i = 0; i < arrows.length; i++) {
this.toAssignableList(
((node.params: any): N.Expression[]),
true,
"arrow function parameters",
);
}
return [arrows, []];
}

return partition(arrows, node => {
try {
this.toAssignableList(
((node.params: any): N.Expression[]),
true,
"arrow function parameters",
);
return true;
} catch (err) {
return false;
}
});
}

forwardNoArrowParamsConversionAt<T>(node: N.Node, parse: () => T): T {
let result: T;
if (this.state.noArrowParamsConversionAt.indexOf(node.start) !== -1) {
this.state.noArrowParamsConversionAt.push(this.state.start);
result = parse();
this.state.noArrowParamsConversionAt.pop();
} else {
result = parse();
}

return super.parseConditional(expr, noIn, startPos, startLoc);
return result;
}

parseParenItem(
Expand Down Expand Up @@ -1767,12 +1933,15 @@ export default (superClass: Class<Parser>): Class<Parser> =>
let typeParameters;
try {
typeParameters = this.flowParseTypeParameterDeclaration();

arrowExpression = super.parseMaybeAssign(
noIn,
refShorthandDefaultPos,
afterLeftParse,
refNeedsArrowPos,
arrowExpression = this.forwardNoArrowParamsConversionAt(
typeParameters,
() =>
super.parseMaybeAssign(
noIn,
refShorthandDefaultPos,
afterLeftParse,
refNeedsArrowPos,
),
);
arrowExpression.typeParameters = typeParameters;
this.resetStartLocationFromNode(arrowExpression, typeParameters);
Expand Down Expand Up @@ -1842,4 +2011,57 @@ export default (superClass: Class<Parser>): Class<Parser> =>
shouldParseArrow(): boolean {
return this.match(tt.colon) || super.shouldParseArrow();
}

setArrowFunctionParameters(
node: N.ArrowFunctionExpression,
params: N.Expression[],
): void {
if (this.state.noArrowParamsConversionAt.indexOf(node.start) !== -1) {
node.params = params;
} else {
super.setArrowFunctionParameters(node, params);
}
}

checkFunctionNameAndParams(
node: N.Function,
isArrowFunction: ?boolean,
): void {
if (
isArrowFunction &&
this.state.noArrowParamsConversionAt.indexOf(node.start) !== -1
) {
return;
}

return super.checkFunctionNameAndParams(node, isArrowFunction);
}

parseParenAndDistinguishExpression(canBeArrow: boolean): N.Expression {
return super.parseParenAndDistinguishExpression(
canBeArrow && this.state.noArrowAt.indexOf(this.state.start) === -1,
);
}

parseSubscripts(
base: N.Expression,
startPos: number,
startLoc: Position,
noCalls?: ?boolean,
): N.Expression {
if (
base.type === "Identifier" &&
base.name === "async" &&
this.state.noArrowAt.indexOf(startPos) !== -1
) {
this.next();

const node = this.startNodeAt(startPos, startLoc);
node.callee = base;
node.arguments = this.parseCallExpressionArguments(tt.parenR, false);
base = this.finishNode(node, "CallExpression");
}

return super.parseSubscripts(base, startPos, startLoc, noCalls);
}
};
17 changes: 17 additions & 0 deletions src/tokenizer/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ export default class State {

this.potentialArrowAt = -1;

this.noArrowAt = [];
this.noArrowParamsConversionAt = [];

// eslint-disable-next-line max-len
this.inMethod = this.inFunction = this.inGenerator = this.inAsync = this.inPropertyName = this.inType = this.inClassProperty = this.noAnonFunctionType = false;

Expand Down Expand Up @@ -68,6 +71,20 @@ export default class State {
// Used to signify the start of a potential arrow function
potentialArrowAt: number;

// Used to signify the start of an expression which looks like a
// typed arrow function, but it isn't
// e.g. a ? (b) : c => d
// ^
noArrowAt: number[];

// Used to signify the start of an expression whose params, if it looks like
// an arrow function, shouldn't be converted to assignable nodes.
// This is used to defer the validation of typed arrow functions inside
// conditional expressions.
// e.g. a ? (b) : c => d
// ^
noArrowParamsConversionAt: number[];

// Flags to track whether we are in a function, a generator.
inFunction: boolean;
inGenerator: boolean;
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/flow/regression/issue-58-ambiguous/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This can be parsed in two ways:
// a ? b : (c => ((d): e => f))
// a ? ((b): c => d) : (e => f)
a ? (b) : c => (d) : e => f;
3 changes: 3 additions & 0 deletions test/fixtures/flow/regression/issue-58-ambiguous/options.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "Ambiguous expression: wrap the arrow functions in parentheses to disambiguate. (4:4)"
}
2 changes: 2 additions & 0 deletions test/fixtures/flow/regression/issue-58-failing-1/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Function which looks like a return type
a ? (b) : (c => d) => e;
3 changes: 3 additions & 0 deletions test/fixtures/flow/regression/issue-58-failing-1/options.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "Invalid left-hand side in arrow function parameters (2:11)"
}
2 changes: 2 additions & 0 deletions test/fixtures/flow/regression/issue-58-failing-2/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Invalid LHS parameter after type parameters
a ? <T>(b => c) : d => (e) : f => g;
3 changes: 3 additions & 0 deletions test/fixtures/flow/regression/issue-58-failing-2/options.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "Invalid left-hand side in arrow function parameters (2:8)"
}
2 changes: 2 additions & 0 deletions test/fixtures/flow/regression/issue-58-failing-3/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Invalid LHS parameter after type parameters
a ? (b => c) => (e) : f => g;
3 changes: 3 additions & 0 deletions test/fixtures/flow/regression/issue-58-failing-3/options.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "Invalid left-hand side in arrow function parameters (2:5)"
}
2 changes: 2 additions & 0 deletions test/fixtures/flow/regression/issue-58-failing-4/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Invalid LHS parameter
a ? async (b => c) => (d) : f => g;
Loading