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 #573

Merged
merged 8 commits into from
Jun 27, 2017
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eaef8f8
Distinguish between ternary's : and arrow fn's return type
nicolo-ribaudo Jun 8, 2017
9d6e62e
Correctly parse nested arrow functions inside conditional expressions
nicolo-ribaudo Jun 12, 2017
1db908a
Check params of arrow fns w/ type params or w/o return type
nicolo-ribaudo Jun 12, 2017
4d93bd7
Fix also async functions
nicolo-ribaudo Jun 12, 2017
b43240c
Add test from prettier
nicolo-ribaudo Jun 20, 2017
2f60733
Don't check arrow params if they are valid at the first attemp
nicolo-ribaudo Jun 20, 2017
74ef24e
Use state instead of relying on the "noArrowParamsConversion" parameter
nicolo-ribaudo Jun 25, 2017
8541f2c
Remove noArrowParamsConversion
nicolo-ribaudo Jun 25, 2017
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
91 changes: 54 additions & 37 deletions src/parser/expression.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export default class ExpressionParser extends LValParser {
// Parse an assignment expression. This includes applications of
// operators like `+=`.

parseMaybeAssign(noIn?: ?boolean, refShorthandDefaultPos?: ?Pos, afterLeftParse?: Function, refNeedsArrowPos?: ?Pos): N.Expression {
parseMaybeAssign(noIn?: ?boolean, refShorthandDefaultPos?: ?Pos, afterLeftParse?: Function, refNeedsArrowPos?: ?Pos, noArrowParamsConversion?: boolean): N.Expression {
const startPos = this.state.start;
const startLoc = this.state.startLoc;
if (this.match(tt._yield) && this.state.inGenerator) {
Expand All @@ -117,7 +117,7 @@ export default class ExpressionParser extends LValParser {
this.state.potentialArrowAt = this.state.start;
}

let left = this.parseMaybeConditional(noIn, refShorthandDefaultPos, refNeedsArrowPos);
let left = this.parseMaybeConditional(noIn, refShorthandDefaultPos, refNeedsArrowPos, noArrowParamsConversion);
if (afterLeftParse) left = afterLeftParse.call(this, left, startPos, startLoc);
if (this.state.type.isAssign) {
const node = this.startNodeAt(startPos, startLoc);
Expand Down Expand Up @@ -151,13 +151,13 @@ export default class ExpressionParser extends LValParser {

// Parse a ternary conditional (`?:`) operator.

parseMaybeConditional(noIn: ?boolean, refShorthandDefaultPos: Pos, refNeedsArrowPos?: ?Pos): N.Expression {
parseMaybeConditional(noIn: ?boolean, refShorthandDefaultPos: Pos, refNeedsArrowPos?: ?Pos, noArrowParamsConversion?: boolean): N.Expression {
const startPos = this.state.start;
const startLoc = this.state.startLoc;
const expr = this.parseExprOps(noIn, refShorthandDefaultPos);
const expr = this.parseExprOps(noIn, refShorthandDefaultPos, noArrowParamsConversion);
if (refShorthandDefaultPos && refShorthandDefaultPos.start) return expr;

return this.parseConditional(expr, noIn, startPos, startLoc, refNeedsArrowPos);
return this.parseConditional(expr, noIn, startPos, startLoc, refNeedsArrowPos, noArrowParamsConversion);
}

parseConditional(
Expand All @@ -167,7 +167,9 @@ export default class ExpressionParser extends LValParser {
startLoc: Position,
// FIXME: Disabling this for now since can't seem to get it to play nicely
// eslint-disable-next-line no-unused-vars
refNeedsArrowPos?: ?Pos
refNeedsArrowPos?: ?Pos,
// eslint-disable-next-line no-unused-vars
noArrowParamsConversion?: boolean
): N.Expression {
if (this.eat(tt.question)) {
const node = this.startNodeAt(startPos, startLoc);
Expand All @@ -182,10 +184,10 @@ export default class ExpressionParser extends LValParser {

// Start the precedence parser.

parseExprOps(noIn: ?boolean, refShorthandDefaultPos: Pos): N.Expression {
parseExprOps(noIn: ?boolean, refShorthandDefaultPos: Pos, noArrowParamsConversion?: boolean): N.Expression {
const startPos = this.state.start;
const startLoc = this.state.startLoc;
const expr = this.parseMaybeUnary(refShorthandDefaultPos);
const expr = this.parseMaybeUnary(refShorthandDefaultPos, noArrowParamsConversion);
if (refShorthandDefaultPos && refShorthandDefaultPos.start) {
return expr;
} else {
Expand Down Expand Up @@ -233,7 +235,7 @@ export default class ExpressionParser extends LValParser {

// Parse unary operators, both prefix and postfix.

parseMaybeUnary(refShorthandDefaultPos: ?Pos): N.Expression {
parseMaybeUnary(refShorthandDefaultPos: ?Pos, noArrowParamsConversion?: boolean): N.Expression {
if (this.state.type.prefix) {
const node = this.startNode();
const update = this.match(tt.incDec);
Expand Down Expand Up @@ -261,7 +263,7 @@ export default class ExpressionParser extends LValParser {

const startPos = this.state.start;
const startLoc = this.state.startLoc;
let expr = this.parseExprSubscripts(refShorthandDefaultPos);
let expr = this.parseExprSubscripts(refShorthandDefaultPos, noArrowParamsConversion);
if (refShorthandDefaultPos && refShorthandDefaultPos.start) return expr;
while (this.state.type.postfix && !this.canInsertSemicolon()) {
const node = this.startNodeAt(startPos, startLoc);
Expand All @@ -277,11 +279,11 @@ export default class ExpressionParser extends LValParser {

// Parse call, dot, and `[]`-subscript expressions.

parseExprSubscripts(refShorthandDefaultPos: ?Pos): N.Expression {
parseExprSubscripts(refShorthandDefaultPos: ?Pos, noArrowParamsConversion?: boolean): N.Expression {
const startPos = this.state.start;
const startLoc = this.state.startLoc;
const potentialArrowAt = this.state.potentialArrowAt;
const expr = this.parseExprAtom(refShorthandDefaultPos);
const expr = this.parseExprAtom(refShorthandDefaultPos, noArrowParamsConversion);

if (expr.type === "ArrowFunctionExpression" && expr.start === potentialArrowAt) {
return expr;
Expand Down Expand Up @@ -440,7 +442,7 @@ export default class ExpressionParser extends LValParser {
// `new`, or an expression wrapped in punctuation like `()`, `[]`,
// or `{}`.

parseExprAtom(refShorthandDefaultPos?: ?Pos): N.Expression {
parseExprAtom(refShorthandDefaultPos?: ?Pos, noArrowParamsConversion?: boolean): N.Expression {
const canBeArrow = this.state.potentialArrowAt === this.state.start;
let node;

Expand Down Expand Up @@ -507,7 +509,7 @@ export default class ExpressionParser extends LValParser {
}

if (canBeArrow && !this.canInsertSemicolon() && this.eat(tt.arrow)) {
return this.parseArrowExpression(node, [id]);
return this.parseArrowExpression(node, [id], undefined, noArrowParamsConversion);
}

return id;
Expand Down Expand Up @@ -551,7 +553,7 @@ export default class ExpressionParser extends LValParser {
return this.finishNode(node, "BooleanLiteral");

case tt.parenL:
return this.parseParenAndDistinguishExpression(canBeArrow);
return this.parseParenAndDistinguishExpression(canBeArrow, noArrowParamsConversion);

case tt.bracketL:
node = this.startNode();
Expand Down Expand Up @@ -665,7 +667,7 @@ export default class ExpressionParser extends LValParser {
return val;
}

parseParenAndDistinguishExpression(canBeArrow: boolean): N.Expression {
parseParenAndDistinguishExpression(canBeArrow: boolean, noArrowParamsConversion?: boolean): N.Expression {
const startPos = this.state.start;
const startLoc = this.state.startLoc;

Expand Down Expand Up @@ -713,7 +715,7 @@ export default class ExpressionParser extends LValParser {
if (param.extra && param.extra.parenthesized) this.unexpected(param.extra.parenStart);
}

return this.parseArrowExpression(arrowNode, exprList);
return this.parseArrowExpression(arrowNode, exprList, false, noArrowParamsConversion);
}

if (!exprList.length) {
Expand Down Expand Up @@ -1053,10 +1055,16 @@ export default class ExpressionParser extends LValParser {

// Parse arrow function expression with given parameters.

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

if (noArrowParamsConversion) {
node.params = params;
} else {
node.params = this.toAssignableList(params, true, "arrow function parameters");
}

this.parseFunctionBody(node, true, noArrowParamsConversion);
return this.finishNode(node, "ArrowFunctionExpression");
}

Expand All @@ -1073,13 +1081,13 @@ export default class ExpressionParser extends LValParser {
}

// Parse function body and check parameters.
parseFunctionBody(node: N.Function, allowExpression?: boolean): void {
parseFunctionBody(node: N.Function, allowExpression?: boolean, noArrowParamsConversion?: boolean): void {
const isExpression = allowExpression && !this.match(tt.braceL);

const oldInAsync = this.state.inAsync;
this.state.inAsync = node.async;
if (isExpression) {
node.body = this.parseMaybeAssign();
node.body = this.parseMaybeAssign(undefined, undefined, undefined, undefined, noArrowParamsConversion);
node.expression = true;
} else {
// Start a new scope with regard to labels and the `inFunction`
Expand All @@ -1098,28 +1106,37 @@ export default class ExpressionParser extends LValParser {
// 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;

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

if (checkLVal) {
const nameHash = Object.create(null);
const oldStrict = this.state.strict;
if (isStrict) this.state.strict = true;
if (node.id) {
this.checkLVal(node.id, true, undefined, "function name");
}
for (const param of node.params) {
if (isStrict && param.type !== "Identifier") {
this.raise(param.start, "Non-simple parameter in strict mode");
}
this.checkLVal(param, true, nameHash, "function parameter list");
// If this is an arrow function, check its parameter only if they are
// converted to assignable nodes
if (allowExpression ? !noArrowParamsConversion : this.state.strict || isStrict) {
this.checkFunctionLValues(node);
}
}

// Checks that functions parameters and function name (if it exists) are valid
// left hand side values.
checkFunctionLValues(node: N.Function): void {
const nameHash = Object.create(null);
const oldStrict = this.state.strict;
const isStrict = this.isStrictBody(node, node.expression);

if (isStrict) this.state.strict = true;
if (node.id) {
this.checkLVal(node.id, true, undefined, "function name");
}
for (const param of node.params) {
if (isStrict && param.type !== "Identifier") {
this.raise(param.start, "Non-simple parameter in strict mode");
}
this.state.strict = oldStrict;
this.checkLVal(param, true, nameHash, "function parameter list");
}

this.state.strict = oldStrict;
}

// Parses a comma-separated list of expressions, and returns them as
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/estree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
classBody.body.push(this.finishNode(method, "MethodDefinition"));
}

parseExprAtom(refShorthandDefaultPos?: ?Pos): N.Expression {
parseExprAtom(refShorthandDefaultPos?: ?Pos, noArrowParamsConversion?: boolean): N.Expression {
switch (this.state.type) {
case tt.regexp:
return this.estreeParseRegExpLiteral(this.state.value);
Expand All @@ -183,7 +183,7 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
return this.estreeParseLiteral(false);

default:
return super.parseExprAtom(refShorthandDefaultPos);
return super.parseExprAtom(refShorthandDefaultPos, noArrowParamsConversion);
}
}

Expand Down
Loading