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

Add support for flow type spread #418

Merged
merged 4 commits into from
Apr 3, 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
52 changes: 34 additions & 18 deletions src/plugins/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ pp.flowParseInterfaceish = function (node) {
} while (this.eat(tt.comma));
}

node.body = this.flowParseObjectType(true);
node.body = this.flowParseObjectType(true, false, false);
};

pp.flowParseInterfaceExtends = function () {
Expand Down Expand Up @@ -403,7 +403,7 @@ pp.flowParseObjectTypeCallProperty = function (node, isStatic) {
return this.finishNode(node, "ObjectTypeCallProperty");
};

pp.flowParseObjectType = function (allowStatic, allowExact) {
pp.flowParseObjectType = function (allowStatic, allowExact, allowSpread) {
const oldInType = this.state.inType;
this.state.inType = true;

Expand Down Expand Up @@ -450,24 +450,40 @@ pp.flowParseObjectType = function (allowStatic, allowExact) {
}
nodeStart.callProperties.push(this.flowParseObjectTypeCallProperty(node, isStatic));
} else {
propertyKey = this.flowParseObjectPropertyKey();
if (this.isRelational("<") || this.match(tt.parenL)) {
// This is a method property
if (this.match(tt.ellipsis)) {
if (!allowSpread) {
this.unexpected(
null,
"Spread operator cannnot appear in class or interface definitions"
);
}
if (variance) {
this.unexpected(variance.start);
this.unexpected(variance.start, "Spread properties cannot have variance");
}
nodeStart.properties.push(this.flowParseObjectTypeMethod(startPos, startLoc, isStatic, propertyKey));
this.expect(tt.ellipsis);
node.argument = this.flowParseType();
this.flowObjectTypeSemicolon();
nodeStart.properties.push(this.finishNode(node, "ObjectTypeSpreadProperty"));
} else {
if (this.eat(tt.question)) {
optional = true;
propertyKey = this.flowParseObjectPropertyKey();
if (this.isRelational("<") || this.match(tt.parenL)) {
// This is a method property
if (variance) {
this.unexpected(variance.start);
}
nodeStart.properties.push(this.flowParseObjectTypeMethod(startPos, startLoc, isStatic, propertyKey));
} else {
if (this.eat(tt.question)) {
optional = true;
}
node.key = propertyKey;
node.value = this.flowParseTypeInitialiser();
node.optional = optional;
node.static = isStatic;
node.variance = variance;
this.flowObjectTypeSemicolon();
nodeStart.properties.push(this.finishNode(node, "ObjectTypeProperty"));
}
node.key = propertyKey;
node.value = this.flowParseTypeInitialiser();
node.optional = optional;
node.static = isStatic;
node.variance = variance;
this.flowObjectTypeSemicolon();
nodeStart.properties.push(this.finishNode(node, "ObjectTypeProperty"));
}
}

Expand Down Expand Up @@ -629,10 +645,10 @@ pp.flowParsePrimaryType = function () {
return this.flowIdentToTypeAnnotation(startPos, startLoc, node, this.parseIdentifier());

case tt.braceL:
return this.flowParseObjectType(false, false);
return this.flowParseObjectType(false, false, true);

case tt.braceBarL:
return this.flowParseObjectType(false, true);
return this.flowParseObjectType(false, true, true);

case tt.bracketL:
return this.flowParseTupleType();
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/flow/type-annotations/135/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
type A = {
...any,
Copy link
Member

@danez danez Mar 17, 2017

Choose a reason for hiding this comment

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

any is also not allowed here it seems, flow is throwing an error. Seems to also happen for all primitive types: number, string, boolean, any, null, ...

Copy link
Contributor

Choose a reason for hiding this comment

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

This should parse. How are you testing?

Copy link
Member

@danez danez Mar 18, 2017

Choose a reason for hiding this comment

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

Okay was testing on astexplorer, which was a bad idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, no worries. Per your initial comment, the typing support is incomplete, but eventually we will also support spreading string (implicitly promotes to String instance, which has no own properties).

};
117 changes: 117 additions & 0 deletions test/fixtures/flow/type-annotations/135/expected.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
{
"type": "File",
"start": 0,
"end": 22,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 3,
"column": 2
}
},
"program": {
"type": "Program",
"start": 0,
"end": 22,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 3,
"column": 2
}
},
"sourceType": "module",
"body": [
{
"type": "TypeAlias",
"start": 0,
"end": 22,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 3,
"column": 2
}
},
"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": "ObjectTypeAnnotation",
"start": 9,
"end": 21,
"loc": {
"start": {
"line": 1,
"column": 9
},
"end": {
"line": 3,
"column": 1
}
},
"callProperties": [],
"properties": [
{
"type": "ObjectTypeSpreadProperty",
"start": 12,
"end": 19,
"loc": {
"start": {
"line": 2,
"column": 1
},
"end": {
"line": 2,
"column": 8
}
},
"argument": {
"type": "AnyTypeAnnotation",
"start": 15,
"end": 18,
"loc": {
"start": {
"line": 2,
"column": 4
},
"end": {
"line": 2,
"column": 7
}
}
}
}
],
"indexers": [],
"exact": false
}
}
],
"directives": []
}
}
4 changes: 4 additions & 0 deletions test/fixtures/flow/type-annotations/136/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type A = {
p: {},
...{},
};
Copy link
Member

@danez danez Mar 17, 2017

Choose a reason for hiding this comment

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

This should not parse I think. Probably related to what @gabelevi wrote.

At least flow is also throwing errors: http://astexplorer.net/#/gist/7703a4fb36053ce507e10d396c5f7d71/c2f5ba5c1048c1e8bade34bebccc16efa44d7367

Copy link
Contributor

Choose a reason for hiding this comment

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

This should parse. How are you testing? Gabe's comment is that parsePrimaryType is actually too restrictive. We also parse non-primary types in this position, like unions. E.g., {...A|B}.

Copy link
Member

Choose a reason for hiding this comment

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

Oh okay sorry. I see astexplorer uses flow 0.40 which supports it spreading only for Identifiers?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, we "reserved" the syntax in 0.40, but 0.42 extends support to all type syntax and adds support.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, PR for astexplorer hasn't been merged yet fkling/astexplorer#208

Copy link
Contributor

Choose a reason for hiding this comment

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

But https://flowtype.org/try/ should work and it has an AST tab

Copy link
Member

Choose a reason for hiding this comment

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

We should see if we can setup greenkeeper or a bot to update the package + yarn @fkling

Copy link
Member

Choose a reason for hiding this comment

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

I think felix is working on this. fkling/astexplorer#197

175 changes: 175 additions & 0 deletions test/fixtures/flow/type-annotations/136/expected.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
{
"type": "File",
"start": 0,
"end": 29,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 4,
"column": 2
}
},
"program": {
"type": "Program",
"start": 0,
"end": 29,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 4,
"column": 2
}
},
"sourceType": "module",
"body": [
{
"type": "TypeAlias",
"start": 0,
"end": 29,
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 4,
"column": 2
}
},
"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": "ObjectTypeAnnotation",
"start": 9,
"end": 28,
"loc": {
"start": {
"line": 1,
"column": 9
},
"end": {
"line": 4,
"column": 1
}
},
"callProperties": [],
"properties": [
{
"type": "ObjectTypeProperty",
"start": 12,
"end": 18,
"loc": {
"start": {
"line": 2,
"column": 1
},
"end": {
"line": 2,
"column": 7
}
},
"key": {
"type": "Identifier",
"start": 12,
"end": 13,
"loc": {
"start": {
"line": 2,
"column": 1
},
"end": {
"line": 2,
"column": 2
},
"identifierName": "p"
},
"name": "p"
},
"value": {
"type": "ObjectTypeAnnotation",
"start": 15,
"end": 17,
"loc": {
"start": {
"line": 2,
"column": 4
},
"end": {
"line": 2,
"column": 6
}
},
"callProperties": [],
"properties": [],
"indexers": [],
"exact": false
},
"optional": false,
"static": false,
"variance": null
},
{
"type": "ObjectTypeSpreadProperty",
"start": 20,
"end": 26,
"loc": {
"start": {
"line": 3,
"column": 1
},
"end": {
"line": 3,
"column": 7
}
},
"argument": {
"type": "ObjectTypeAnnotation",
"start": 23,
"end": 25,
"loc": {
"start": {
"line": 3,
"column": 4
},
"end": {
"line": 3,
"column": 6
}
},
"callProperties": [],
"properties": [],
"indexers": [],
"exact": false
}
}
],
"indexers": [],
"exact": false
}
}
],
"directives": []
}
}
3 changes: 3 additions & 0 deletions test/fixtures/flow/type-annotations/137/actual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
interface A {
...any,
};
3 changes: 3 additions & 0 deletions test/fixtures/flow/type-annotations/137/options.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "Spread operator cannnot appear in class or interface definitions (2:1)"
}
Loading