Skip to content

Commit

Permalink
[parser] fix AST of nameless functions and classes in ExportDefaultDe…
Browse files Browse the repository at this point in the history
…claration

Summary:
`export default class {}` creates a `ClassDeclaration`, not a `ClassExpression`.

ESTree doesn't technically allow nullable ids on `FunctionDeclaration` and `ClassDeclaration` yet, but that will be fixed by estree/estree#98.

Reviewed By: samwgoldman

Differential Revision: D3939824

fbshipit-source-id: 9b671beeb936a1154cef7b54c812198ff2dc5ab0
  • Loading branch information
mroch authored and Facebook Github Bot committed Sep 28, 2016
1 parent 9ce80b1 commit f529a73
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 37 deletions.
62 changes: 27 additions & 35 deletions src/parser/estree_translator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -201,31 +201,7 @@ end with type t = Impl.t) = struct
| loc, ClassDeclaration c -> class_declaration (loc, c)
| loc, InterfaceDeclaration i -> interface_declaration (loc, i)
| loc, VariableDeclaration var -> variable_declaration (loc, var)
| loc, FunctionDeclaration fn -> Function.(
(* esprima/estree hasn't come around to the idea that function decls can
* have optional ids :( *)
let (node_type, node_value) = (
match fn.id with
| Some(id) -> "FunctionDeclaration", identifier id
| None -> "FunctionExpression", null
) in

let body = (match fn.body with
| BodyBlock b -> block b
| BodyExpression b -> expression b) in

node node_type loc [|
"id", node_value;
"params", function_params fn.params;
"body", body;
"async", bool fn.async;
"generator", bool fn.generator;
"predicate", option predicate fn.predicate;
"expression", bool fn.expression;
"returnType", option type_annotation fn.returnType;
"typeParameters", option type_parameter_declaration fn.typeParameters;
|]
)
| loc, FunctionDeclaration fn -> function_declaration (loc, fn)
| loc, DeclareVariable d -> declare_variable (loc, d)
| loc, DeclareFunction d -> declare_function (loc, d)
| loc, DeclareClass d -> declare_class (loc, d)
Expand Down Expand Up @@ -520,6 +496,27 @@ end with type t = Impl.t) = struct
|]
))

and function_declaration (loc, fn) = Function.(
let body = match fn.body with
| BodyBlock b -> block b
| BodyExpression b -> expression b in

node "FunctionDeclaration" loc [|
(* estree hasn't come around to the idea that function decls can have
optional ids, but acorn, babel, espree and esprima all have, so let's
do it too. see https://github.com/estree/estree/issues/98 *)
"id", option identifier fn.id;
"params", function_params fn.params;
"body", body;
"async", bool fn.async;
"generator", bool fn.generator;
"predicate", option predicate fn.predicate;
"expression", bool fn.expression;
"returnType", option type_annotation fn.returnType;
"typeParameters", option type_parameter_declaration fn.typeParameters;
|]
)

and function_expression (loc, _function) = Function.(
let body = match _function.body with
| BodyBlock b -> block b
Expand Down Expand Up @@ -620,16 +617,11 @@ end with type t = Impl.t) = struct
)

and class_declaration (loc, c) = Class.(
(* esprima/estree hasn't come around to the idea that class decls can have
* optional ids :( *)
let (node_type, node_value) = (
match c.id with
| Some(id) -> "ClassDeclaration", identifier id
| None -> "ClassExpression", null
) in

node node_type loc [|
"id", node_value;
node "ClassDeclaration" loc [|
(* estree hasn't come around to the idea that class decls can have
optional ids, but acorn, babel, espree and esprima all have, so let's
do it too. see https://github.com/estree/estree/issues/98 *)
"id", option identifier c.id;
"body", class_body c.body;
"superClass", option expression c.superClass;
"typeParameters", option type_parameter_declaration c.typeParameters;
Expand Down
24 changes: 23 additions & 1 deletion src/parser/test/custom_ast_types.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,33 @@ def("DeclareExportDeclaration")
null
))

// TODO: should be named NullableClassDeclaration. estree allows a nameless
// decl inside an `export default` (https://github.com/estree/estree/issues/98),
// but ast-types uses the `TypeName` in `def("TypeName")` as the expected `type`
// property, which isn't the case here. So, we incorrectly have to loosen all
// ClassDeclarations for now.
def("ClassDeclaration")
.field("id", or(def("Identifier"), null))

// TODO: should be named NullableFunctionDeclaration. estree allows a nameless
// decl inside an `export default` (https://github.com/estree/estree/issues/98),
// but ast-types uses the `TypeName` in `def("TypeName")` as the expected `type`
// property, which isn't the case here. So, we incorrectly have to loosen all
// FunctionDeclarations for now.
def("FunctionDeclaration")
.field("id", or(def("Identifier"), null))

// See https://github.com/benjamn/ast-types/issues/180
def("ExportDefaultDeclaration")
.bases("Declaration")
.build("declaration", "exportKind")
.field("declaration", or(def("Declaration"), def("Expression")))
.field("declaration", or(
def("ClassDeclaration"), // TODO: should be NullableClassDeclaration
def("FunctionDeclaration"), // TODO: should be NullableFunctionDeclaration
def("VariableDeclaration"),
def("InterfaceDeclaration"),
def("TypeAlias"),
def("Expression")))
.field("exportKind", or("type", "value"));

// See https://github.com/benjamn/ast-types/issues/180
Expand Down
5 changes: 5 additions & 0 deletions src/parser/test/esprima_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -4485,6 +4485,11 @@ module.exports = {
'root.body.0.source': {
type: 'Missing property',
},
'root.body.0.declaration.type': {
type: 'Wrong string',
expected: 'FunctionExpression',
actual: 'FunctionDeclaration',
},
},
},
/* Esprima parses default exports wrong
Expand Down
12 changes: 11 additions & 1 deletion src/parser/test/hardcoded_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -1229,14 +1229,24 @@ module.exports = {
},
'export default class {}': {
'body.0.declaration': {
'type': 'ClassExpression',
'type': 'ClassDeclaration',
}
},
'export default class A {}': {
'body.0.declaration': {
'type': 'ClassDeclaration',
}
},
'export default function() {}': {
'body.0.declaration': {
'type': 'FunctionDeclaration',
}
},
'export default function a() {}': {
'body.0.declaration': {
'type': 'FunctionDeclaration',
}
},
'export type A = number': {
'body.0.declaration': {
'type': 'TypeAlias',
Expand Down

0 comments on commit f529a73

Please sign in to comment.