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

Commit

Permalink
Revert "Distinguish between ternary's : and arrow fn's return type (#573
Browse files Browse the repository at this point in the history
)"

This reverts commit a9a55fb.
  • Loading branch information
hzoo committed Jun 27, 2017
1 parent a9a55fb commit 8829853
Show file tree
Hide file tree
Showing 15 changed files with 10 additions and 6,840 deletions.
19 changes: 4 additions & 15 deletions src/parser/expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -1081,15 +1081,11 @@ export default class ExpressionParser extends LValParser {

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

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

isStrictBody(node: { body: N.BlockStatement }, isExpression?: boolean): boolean {
if (!isExpression && node.body.directives.length) {
for (const directive of node.body.directives) {
Expand Down Expand Up @@ -1124,19 +1120,12 @@ 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, node.expression);
// Also check for arrow functions
const checkLVal = this.state.strict || isStrict || isArrowFunction;
const isStrict = this.isStrictBody(node, isExpression);
// Also check when allowExpression === true for arrow functions
const checkLVal = this.state.strict || allowExpression || isStrict;

if (isStrict && node.id && node.id.type === "Identifier" && node.id.name === "yield") {
this.raise(node.id.start, "Binding yield in strict mode");
Expand Down
192 changes: 6 additions & 186 deletions src/plugins/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,6 @@ 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 {
const oldInType = this.state.inType;
Expand Down Expand Up @@ -1017,15 +1007,9 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
// Overrides
// ==================================

// plain function return types: function name(): string {}
parseFunctionBody(node: N.Function, allowExpression?: boolean): void {
if (allowExpression && this.state.noArrowParamsConversionAt.indexOf(node.start) !== -1) {
this.state.noArrowParamsConversionAt.push(this.state.start);
super.parseFunctionBody(node, allowExpression);
this.state.noArrowParamsConversionAt.pop();

return;
} else if (this.match(tt.colon)) {
// plain function return types: function name(): string {}
if (this.match(tt.colon) && !allowExpression) {
// if allowExpression is true then we're parsing an arrow function and if
// there's a return type then it's been handled elsewhere
const typeNode = this.startNode();
Expand Down Expand Up @@ -1090,11 +1074,9 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
}

parseConditional(expr: N.Expression, noIn: ?boolean, startPos: number, 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) {
if (refNeedsArrowPos && this.match(tt.question)) {
const state = this.state.clone();
try {
return super.parseConditional(expr, noIn, startPos, startLoc);
Expand All @@ -1110,129 +1092,7 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
}
}

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, 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(arrows[i].params, true, "arrow function parameters");
}
return [ arrows, [] ];
}

return partition(arrows, (node) => {
try {
this.toAssignableList(node.params, 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(this.state.start);
} else {
result = parse();
}

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

parseParenItem(node: N.Expression, startPos: number, startLoc: Position): N.Expression {
Expand Down Expand Up @@ -1650,9 +1510,8 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
let typeParameters;
try {
typeParameters = this.flowParseTypeParameterDeclaration();
arrowExpression = this.forwardNoArrowParamsConversionAt(typeParameters, () => (
super.parseMaybeAssign(noIn, refShorthandDefaultPos, afterLeftParse, refNeedsArrowPos)
));

arrowExpression = super.parseMaybeAssign(noIn, refShorthandDefaultPos, afterLeftParse, refNeedsArrowPos);
arrowExpression.typeParameters = typeParameters;
this.resetStartLocationFromNode(arrowExpression, typeParameters);
} catch (err) {
Expand Down Expand Up @@ -1711,43 +1570,4 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
shouldParseArrow(): boolean {
return this.match(tt.colon) || super.shouldParseArrow();
}

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

return 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: 0 additions & 17 deletions src/tokenizer/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ export default class State {

this.potentialArrowAt = -1;

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

this.inMethod =
this.inFunction =
this.inGenerator =
Expand Down Expand Up @@ -79,20 +76,6 @@ 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: 0 additions & 4 deletions test/fixtures/flow/regression/issue-58-ambiguous/actual.js

This file was deleted.

3 changes: 0 additions & 3 deletions test/fixtures/flow/regression/issue-58-ambiguous/options.json

This file was deleted.

2 changes: 0 additions & 2 deletions test/fixtures/flow/regression/issue-58-failing-1/actual.js

This file was deleted.

3 changes: 0 additions & 3 deletions test/fixtures/flow/regression/issue-58-failing-1/options.json

This file was deleted.

2 changes: 0 additions & 2 deletions test/fixtures/flow/regression/issue-58-failing-2/actual.js

This file was deleted.

3 changes: 0 additions & 3 deletions test/fixtures/flow/regression/issue-58-failing-2/options.json

This file was deleted.

2 changes: 0 additions & 2 deletions test/fixtures/flow/regression/issue-58-failing-3/actual.js

This file was deleted.

3 changes: 0 additions & 3 deletions test/fixtures/flow/regression/issue-58-failing-3/options.json

This file was deleted.

2 changes: 0 additions & 2 deletions test/fixtures/flow/regression/issue-58-failing-4/actual.js

This file was deleted.

3 changes: 0 additions & 3 deletions test/fixtures/flow/regression/issue-58-failing-4/options.json

This file was deleted.

44 changes: 0 additions & 44 deletions test/fixtures/flow/regression/issue-58/actual.js

This file was deleted.

Loading

0 comments on commit 8829853

Please sign in to comment.