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

Optional names for function types and object type indexers #197

Merged
merged 6 commits into from
Nov 9, 2016
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
* text eol=lf
test/fixtures/es2015/uncategorised/22/actual.js binary
test/fixtures/es2015/uncategorised/23/actual.js binary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the issue you had with these two files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they have weird characters that the .gitattribute doesn't like if it treats them as text

$ git checkout master .gitattributes
$ git commit -m "revert gitattributes"
$ git rm --cached -r .
$ git reset --hard
$ g st
On branch flow
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js
    modified:   test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js

no changes added to commit (use "git add" and/or "git commit -a")
$ g diff
warning: CRLF will be replaced by LF in test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js.
The file will have its original line endings in your working directory.
diff --git a/test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js b/test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js
index dd8e38b..ed49ef3 100644
--- a/test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js
+++ b/test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js
@@ -1,2 +1,2 @@
-//
+//
 ]
diff --git a/test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js b/test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js
index 583aa13..e85031c 100644
--- a/test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js
+++ b/test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js
@@ -1,2 +1,2 @@
-/*
+/*
 */]

89 changes: 76 additions & 13 deletions src/plugins/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,13 @@ pp.flowParseObjectTypeIndexer = function (node, isStatic, variance) {
node.static = isStatic;

this.expect(tt.bracketL);
node.id = this.flowParseObjectPropertyKey();
node.key = this.flowParseTypeInitialiser();
if (this.lookahead().type === tt.colon) {
node.id = this.flowParseObjectPropertyKey();
node.key = this.flowParseTypeInitialiser();
} else {
node.id = null;
node.key = this.flowParseType();
}
this.expect(tt.bracketR);
node.value = this.flowParseTypeInitialiser();
node.variance = variance;
Expand Down Expand Up @@ -466,19 +471,37 @@ pp.flowParseTupleType = function () {
};

pp.flowParseFunctionTypeParam = function () {
let name = null;
let optional = false;
let typeAnnotation = null;
let node = this.startNode();
node.name = this.parseIdentifier();
if (this.eat(tt.question)) {
optional = true;
const lh = this.lookahead();
if (lh.type === tt.colon ||
lh.type === tt.question) {
name = this.parseIdentifier();
if (this.eat(tt.question)) {
optional = true;
}
typeAnnotation = this.flowParseTypeInitialiser();
} else {
typeAnnotation = this.flowParseType();
}
node.name = name;
node.optional = optional;
node.typeAnnotation = this.flowParseTypeInitialiser();
node.typeAnnotation = typeAnnotation;
return this.finishNode(node, "FunctionTypeParam");
};

pp.flowParseFunctionTypeParams = function () {
let ret = { params: [], rest: null };
pp.reinterpretTypeAsFunctionTypeParam = function (type) {
let node = this.startNodeAt(type.start, type.loc);
node.name = null;
node.optional = false;
node.typeAnnotation = type;
return this.finishNode(node, "FunctionTypeParam");
};

pp.flowParseFunctionTypeParams = function (params = []) {
let ret = { params, rest: null };
while (this.match(tt.name)) {
ret.params.push(this.flowParseFunctionTypeParam());
if (!this.match(tt.parenR)) {
Expand Down Expand Up @@ -529,6 +552,7 @@ pp.flowParsePrimaryType = function () {
let tmp;
let type;
let isGroupedType = false;
let oldNoAnonFunctionType = this.state.noAnonFunctionType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add noAnonFunctionType to the state so that v8 can better optimize the usage of the state.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do!


switch (this.state.type) {
case tt.name:
Expand Down Expand Up @@ -574,12 +598,30 @@ pp.flowParsePrimaryType = function () {
}

if (isGroupedType) {
this.state.noAnonFunctionType = false;
type = this.flowParseType();
this.expect(tt.parenR);
return type;
this.state.noAnonFunctionType = oldNoAnonFunctionType;

// A `,` or a `) =>` means this is an anonymous function type
if (this.state.noAnonFunctionType ||
!(this.match(tt.comma) ||
(this.match(tt.parenR) && this.lookahead().type === tt.arrow))) {
this.expect(tt.parenR);
return type;
} else {
// Eat a comma if there is one
this.eat(tt.comma);
}
}

if (type) {
tmp = this.flowParseFunctionTypeParams(
[this.reinterpretTypeAsFunctionTypeParam(type)],
);
} else {
tmp = this.flowParseFunctionTypeParams();
}

tmp = this.flowParseFunctionTypeParams();
node.params = tmp.params;
node.rest = tmp.rest;

Expand All @@ -588,6 +630,7 @@ pp.flowParsePrimaryType = function () {
this.expect(tt.arrow);

node.returnType = this.flowParseType();

node.typeParameters = null;

return this.finishNode(node, "FunctionTypeAnnotation");
Expand Down Expand Up @@ -668,12 +711,25 @@ pp.flowParsePrefixType = function () {
}
};

pp.flowParseAnonFunctionWithoutParens = function () {
const param = this.flowParsePrefixType();
if (!this.state.noAnonFunctionType && this.eat(tt.arrow)) {
const node = this.startNodeAt(param.start, param.loc);
node.params = [this.reinterpretTypeAsFunctionTypeParam(param)];
node.rest = null;
node.returnType = this.flowParseType();
node.typeParameters = null;
return this.finishNode(node, "FunctionTypeAnnotation");
}
return param;
};

pp.flowParseIntersectionType = function () {
let node = this.startNode();
let type = this.flowParsePrefixType();
let type = this.flowParseAnonFunctionWithoutParens();
node.types = [type];
while (this.eat(tt.bitwiseAND)) {
node.types.push(this.flowParsePrefixType());
node.types.push(this.flowParseAnonFunctionWithoutParens());
}
return node.types.length === 1 ? type : this.finishNode(node, "IntersectionTypeAnnotation");
};
Expand Down Expand Up @@ -1156,7 +1212,10 @@ export default function (instance) {
instance.extend("parseAsyncArrowFromCallExpression", function (inner) {
return function (node, call) {
if (this.match(tt.colon)) {
const oldNoAnonFunctionType = this.state.noAnonFunctionType;
this.state.noAnonFunctionType = true;
node.returnType = this.flowParseTypeAnnotation();
this.state.noAnonFunctionType = oldNoAnonFunctionType;
}

return inner.call(this, node, call);
Expand Down Expand Up @@ -1238,7 +1297,11 @@ export default function (instance) {
if (this.match(tt.colon)) {
let state = this.state.clone();
try {
const oldNoAnonFunctionType = this.state.noAnonFunctionType;
this.state.noAnonFunctionType = true;
let returnType = this.flowParseTypeAnnotation();
this.state.noAnonFunctionType = oldNoAnonFunctionType;

if (this.canInsertSemicolon()) this.unexpected();
if (!this.match(tt.arrow)) this.unexpected();
// assign after it is clear it is an arrow
Expand Down
8 changes: 7 additions & 1 deletion src/tokenizer/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@ export default class State {

this.potentialArrowAt = -1;

this.inMethod = this.inFunction = this.inGenerator = this.inAsync = this.inType = false;
this.inMethod =
this.inFunction =
this.inGenerator =
this.inAsync =
this.inType =
this.noAnonFunctionType =
false;

this.labels = [];

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
var f = (x): number => 123 => 123;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a better error message that says something like return type annotation needs to be wrapped in parens or have named parameters. But not sure if this is easy to accomplish.
This should not be a blocker, just if you have an idea on how this could be done easy, otherwise we can come back to that in a separate story.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's not easy to do. The problem is that the first => is parsed as arrow function's =>. So we'd have to decide which parsing errors after the => would need this explanation and which are just normal arrow function parse errors.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "Unexpected token, expected ; (1:27)"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
var f = (x): string | number => 123 => 123;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "Unexpected token, expected ; (1:36)"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
type A = string => void
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
{
"type": "File",
"start": 0,
"end": 23,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 23
}
},
"program": {
"type": "Program",
"start": 0,
"end": 23,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 23
}
},
"sourceType": "module",
"body": [
{
"type": "TypeAlias",
"start": 0,
"end": 23,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 23
}
},
"id": {
"type": "Identifier",
"start": 5,
"end": 6,
"loc": {
"start": {
"line": 1,
"column": 5
},
"end": {
"line": 1,
"column": 6
},
"identifierName": "A"
},
"name": "A"
},
"typeParameters": null,
"right": {
"type": "FunctionTypeAnnotation",
"start": 9,
"end": 23,
"loc": {
"start": {
"start": {
"line": 1,
"column": 9
},
"end": {
"line": 1,
"column": 15
}
},
"end": {
"line": 1,
"column": 23
}
},
"params": [
{
"type": "FunctionTypeParam",
"start": 9,
"end": 18,
"loc": {
"start": {
"start": {
"line": 1,
"column": 9
},
"end": {
"line": 1,
"column": 15
}
},
"end": {
"line": 1,
"column": 18
}
},
"name": null,
"optional": false,
"typeAnnotation": {
"type": "StringTypeAnnotation",
"start": 9,
"end": 15,
"loc": {
"start": {
"line": 1,
"column": 9
},
"end": {
"line": 1,
"column": 15
}
}
}
}
],
"rest": null,
"returnType": {
"type": "VoidTypeAnnotation",
"start": 19,
"end": 23,
"loc": {
"start": {
"line": 1,
"column": 19
},
"end": {
"line": 1,
"column": 23
}
}
},
"typeParameters": null
}
}
],
"directives": []
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
type A = Array<string> => void
Loading