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

Commit

Permalink
Fix flow plugin when flow+arrow+spread used together
Browse files Browse the repository at this point in the history
The fix includes creating a new method on the parser called `parseArrow`.
This new function by default only checks if current position matches an
arrow. If it does returns the `node` otherwise `undefined`.
The flow plugin can then extend this function and correctly parse the typeAnnotation
and add it to the node.

With this change, in the flow plugin there is no need anymore to extend
`parseParenAndDistinguishExpression` and the arrow handling in `parseParenItem`
could also be removed, because it is all handled now in `parseArrow`.

Some existing tests were failing, because `extra->parentesized` is now missing,
but this is correct as it is now inline with parsing without flow annotation. No extra
is added for arrow function without type annotations.

In the expression-parser `this.next()` was replaced by a more specific
`this.expect(tt.parenL)`.
  • Loading branch information
danez committed Apr 4, 2016
1 parent 8b15081 commit d15a231
Show file tree
Hide file tree
Showing 13 changed files with 793 additions and 74 deletions.
17 changes: 12 additions & 5 deletions src/parser/expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -531,12 +531,12 @@ pp.parseParenExpression = function () {
return val;
};

pp.parseParenAndDistinguishExpression = function (startPos, startLoc, canBeArrow, isAsync, allowOptionalCommaStart) {
pp.parseParenAndDistinguishExpression = function (startPos, startLoc, canBeArrow, isAsync) {
startPos = startPos || this.state.start;
startLoc = startLoc || this.state.startLoc;

let val;
this.next();
this.expect(tt.parenL);

let innerStartPos = this.state.start, innerStartLoc = this.state.startLoc;
let exprList = [], first = true;
Expand Down Expand Up @@ -566,12 +566,13 @@ pp.parseParenAndDistinguishExpression = function (startPos, startLoc, canBeArrow
let innerEndLoc = this.state.startLoc;
this.expect(tt.parenR);

if (canBeArrow && !this.canInsertSemicolon() && this.eat(tt.arrow)) {
let arrowNode = this.startNodeAt(startPos, startLoc);
if (canBeArrow && !this.canInsertSemicolon() && (arrowNode = this.parseArrow(arrowNode))) {
for (let param of exprList) {
if (param.extra && param.extra.parenthesized) this.unexpected(param.extra.parenStart);
}

return this.parseArrowExpression(this.startNodeAt(startPos, startLoc), exprList, isAsync);
return this.parseArrowExpression(arrowNode, exprList, isAsync);
}

if (!exprList.length) {
Expand All @@ -581,7 +582,7 @@ pp.parseParenAndDistinguishExpression = function (startPos, startLoc, canBeArrow
this.unexpected(this.state.lastTokStart);
}
}
if (optionalCommaStart && !allowOptionalCommaStart) this.unexpected(optionalCommaStart);
if (optionalCommaStart) this.unexpected(optionalCommaStart);
if (spreadStart) this.unexpected(spreadStart);
if (refShorthandDefaultPos.start) this.unexpected(refShorthandDefaultPos.start);

Expand All @@ -601,6 +602,12 @@ pp.parseParenAndDistinguishExpression = function (startPos, startLoc, canBeArrow
return val;
};

pp.parseArrow = function (node) {
if (this.eat(tt.arrow)) {
return node;
}
};

pp.parseParenItem = function (node) {
return node;
};
Expand Down
72 changes: 23 additions & 49 deletions src/plugins/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -717,30 +717,19 @@ export default function (instance) {
};
});

instance.extend("parseParenItem", function () {
return function (node, startLoc, startPos, forceArrow?) {
let canBeArrow = this.state.potentialArrowAt = startPos;
instance.extend("parseParenItem", function (inner) {
return function (node, startLoc, startPos) {
node = inner.call(this, node, startLoc, startPos);

if (this.match(tt.colon)) {
let typeCastNode = this.startNodeAt(startLoc, startPos);
typeCastNode.expression = node;
typeCastNode.typeAnnotation = this.flowParseTypeAnnotation();

if (forceArrow && !this.match(tt.arrow)) {
this.unexpected();
}

if (canBeArrow && this.eat(tt.arrow)) {
// ((lol): number => {});
let params = node.type === "SequenceExpression" ? node.expressions : [node];
let func = this.parseArrowExpression(this.startNodeAt(startLoc, startPos), params);
func.returnType = typeCastNode.typeAnnotation;
return func;
} else {
return this.finishNode(typeCastNode, "TypeCastExpression");
}
} else {
return node;
return this.finishNode(typeCastNode, "TypeCastExpression");
}

return node;
};
});

Expand Down Expand Up @@ -1038,40 +1027,25 @@ export default function (instance) {
});

// handle return types for arrow functions
instance.extend("parseParenAndDistinguishExpression", function (inner) {
return function (startPos, startLoc, canBeArrow, isAsync) {
startPos = startPos || this.state.start;
startLoc = startLoc || this.state.startLoc;

if (canBeArrow && this.lookahead().type === tt.parenR) {
// let foo = (): number => {};
this.expect(tt.parenL);
this.expect(tt.parenR);

let node = this.startNodeAt(startPos, startLoc);
if (this.match(tt.colon)) node.returnType = this.flowParseTypeAnnotation();
this.expect(tt.arrow);
return this.parseArrowExpression(node, [], isAsync);
} else {
// let foo = (foo): number => {};
let node = inner.call(this, startPos, startLoc, canBeArrow, isAsync, this.hasPlugin("trailingFunctionCommas"));

if (this.match(tt.colon)) {
let state = this.state.clone();
try {
return this.parseParenItem(node, startPos, startLoc, true);
} catch (err) {
if (err instanceof SyntaxError) {
this.state = state;
return node;
} else {
throw err;
}
instance.extend("parseArrow", function (inner) {
return function (node) {
if (this.match(tt.colon)) {
let state = this.state.clone();
try {
let returnType = this.flowParseTypeAnnotation();
if (!this.match(tt.arrow)) this.unexpected();
// assign after it is clear it is an arrow
node.returnType = returnType;
} catch (err) {
if (err instanceof SyntaxError) {
this.state = state;
} else {
throw err;
}
} else {
return node;
}
}

return inner.call(this, node);
};
});
}
4 changes: 0 additions & 4 deletions test/fixtures/flow/regression/issue-2493/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@
"raw": "' world'"
},
"value": " world"
},
"extra": {
"parenthesized": true,
"parenStart": 12
}
}
],
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/flow/type-annotations/101/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
( ...props: SomeType ) : ?ReturnType => ( 3 );
232 changes: 232 additions & 0 deletions test/fixtures/flow/type-annotations/101/expected.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
{
"type": "File",
"start": 0,
"end": 46,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 46
}
},
"program": {
"type": "Program",
"start": 0,
"end": 46,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 46
}
},
"sourceType": "module",
"body": [
{
"type": "ExpressionStatement",
"start": 0,
"end": 46,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 46
}
},
"expression": {
"type": "ArrowFunctionExpression",
"start": 0,
"end": 45,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 45
}
},
"returnType": {
"type": "TypeAnnotation",
"start": 23,
"end": 36,
"loc": {
"start": {
"line": 1,
"column": 23
},
"end": {
"line": 1,
"column": 36
}
},
"typeAnnotation": {
"type": "NullableTypeAnnotation",
"start": 25,
"end": 36,
"loc": {
"start": {
"line": 1,
"column": 25
},
"end": {
"line": 1,
"column": 36
}
},
"typeAnnotation": {
"type": "GenericTypeAnnotation",
"start": 26,
"end": 36,
"loc": {
"start": {
"line": 1,
"column": 26
},
"end": {
"line": 1,
"column": 36
}
},
"typeParameters": null,
"id": {
"type": "Identifier",
"start": 26,
"end": 36,
"loc": {
"start": {
"line": 1,
"column": 26
},
"end": {
"line": 1,
"column": 36
}
},
"name": "ReturnType"
}
}
}
},
"id": null,
"generator": false,
"expression": true,
"async": false,
"params": [
{
"type": "RestElement",
"start": 2,
"end": 10,
"loc": {
"start": {
"line": 1,
"column": 2
},
"end": {
"line": 1,
"column": 10
}
},
"argument": {
"type": "Identifier",
"start": 5,
"end": 10,
"loc": {
"start": {
"line": 1,
"column": 5
},
"end": {
"line": 1,
"column": 10
}
},
"name": "props"
},
"typeAnnotation": {
"type": "TypeAnnotation",
"start": 10,
"end": 20,
"loc": {
"start": {
"line": 1,
"column": 10
},
"end": {
"line": 1,
"column": 20
}
},
"typeAnnotation": {
"type": "GenericTypeAnnotation",
"start": 12,
"end": 20,
"loc": {
"start": {
"line": 1,
"column": 12
},
"end": {
"line": 1,
"column": 20
}
},
"typeParameters": null,
"id": {
"type": "Identifier",
"start": 12,
"end": 20,
"loc": {
"start": {
"line": 1,
"column": 12
},
"end": {
"line": 1,
"column": 20
}
},
"name": "SomeType"
}
}
}
}
],
"body": {
"type": "NumericLiteral",
"start": 42,
"end": 43,
"loc": {
"start": {
"line": 1,
"column": 42
},
"end": {
"line": 1,
"column": 43
}
},
"extra": {
"rawValue": 3,
"raw": "3",
"parenthesized": true,
"parenStart": 40
},
"value": 3
}
}
}
],
"directives": []
}
}
1 change: 1 addition & 0 deletions test/fixtures/flow/type-annotations/102/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default (...modifiers): Array<string> => {};
Loading

0 comments on commit d15a231

Please sign in to comment.