[private-fields] Fix ESTree translator output to match latest ESTree …

…spec Summary: As a first step to fix ESTree translator output for private properties, we should unify it with public properties according to the latest [estree spec](https://github.com/estree/estree/blob/master/es2022.md). Changes: - Both private and public fields have type `PropertyDefinition`. - `key` of private fields are now `PrivateIdentifier` instead of `Identifier` - `computed` field of private fields are always present and always false Reviewed By: gkz Differential Revision: D30306603 fbshipit-source-id: 9ea947a69c6578bf2b6fe527f137f44076b412d5
facebook · Aug 17, 2021 · 5de4ea5 · 5de4ea5
1 parent faec817
commit 5de4ea5
Show file tree

Hide file tree

Showing 81 changed files with 503 additions and 548 deletions.
diff --git a/packages/flow-parser/test/custom_ast_types.js b/packages/flow-parser/test/custom_ast_types.js
@@ -66,8 +66,15 @@ export default function (fork) {
  .field("bigint", String);
 
  /////////
- // es-proposal
+ // es2022
  /////////
- def("ClassPrivateProperty")
- .field("key", def("Identifier"))
+ def("PrivateIdentifier")
+ .bases("Expression", "Pattern")
+ .field("name", String);
+
+ def("PropertyDefinition")
+ .bases("ClassProperty");
+
+ def("ClassBody")
+ .field("body", [or(def("MethodDefinition"), def("PropertyDefinition"))]);
 }
diff --git a/packages/flow-parser/test/esprima_test_runner.js b/packages/flow-parser/test/esprima_test_runner.js
@@ -365,7 +365,7 @@ function handleSpecialObjectCompare(esprima, flow, env) {
  break;
  case 'ObjectTypeProperty':
  case 'ObjectTypeIndexer':
- case 'ClassProperty':
+ case 'PropertyDefinition':
  delete flow.variance;
  break;
  case 'DeclareModule':
@@ -470,6 +470,7 @@ function compare(esprima, flow, env) {
  if (!ast_types.namedTypes[flow.type]) {
  env.diff("Unknown AST type", "known type", flow.type);
  } else if (!ast_types.namedTypes[flow.type].check(flow, true)) {
+ throw new Error(`${ast_types.namedTypes[flow.type]} ${JSON.stringify(flow)}`);
  env.ast_types_error();
  }
  }

diff --git a/packages/flow-remove-types/index.js b/packages/flow-remove-types/index.js
@@ -197,7 +197,7 @@ var removeFlowVisitor = {
  }
  },
 
- ClassProperty: function(context, node) {
+ PropertyDefinition: function(context, node) {
  if (node.declare || (context.ignoreUninitializedFields && !node.value)) {
  return removeNode(context, node);
  }

diff --git a/src/parser/estree_translator.ml b/src/parser/estree_translator.ml
@@ -737,8 +737,16 @@ with type t = Impl.t = struct
  ?comments
  loc
  [("name", string name); ("typeAnnotation", null); ("optional", bool false)]
- and private_name (loc, { PrivateName.id; comments }) =
- node ?comments "PrivateName" loc [("id", identifier id)]
+ and private_identifier
+ ( loc,
+ { PrivateName.id = (_, { Identifier.name; comments = identifier_comments }); comments } )
+ =
+ let comments = Flow_ast_utils.merge_comments ~outer:comments ~inner:identifier_comments in
+ node
+ ?comments
+ "PrivateIdentifier"
+ loc
+ [("name", string name); ("typeAnnotation", null); ("optional", bool false)]
  and pattern_identifier
  loc { Pattern.Identifier.name = (_, { Identifier.name; comments }); annot; optional } =
  node
@@ -939,7 +947,7 @@ with type t = Impl.t = struct
  match key with
  | Literal lit -> (literal lit, false, comments)
  | Identifier id -> (identifier id, false, comments)
- | PrivateName name -> (private_name name, false, comments)
+ | PrivateName name -> (private_identifier name, false, comments)
  | Computed (_, { ComputedKey.expression = expr; comments = computed_comments }) ->
  ( expression expr,
  true,
@@ -966,27 +974,19 @@ with type t = Impl.t = struct
  ("decorators", array_of_list class_decorator decorators);
  ]
  and class_private_field
- ( loc,
- {
- Class.PrivateField.key = (_, { PrivateName.id; comments = key_comments });
- value;
- annot;
- static;
- variance = variance_;
- comments;
- } ) =
+ (loc, { Class.PrivateField.key; value; annot; static; variance = variance_; comments }) =
  let (value, declare) =
  match value with
  | Class.Property.Declared -> (None, true)
  | Class.Property.Uninitialized -> (None, false)
  | Class.Property.Initialized x -> (Some x, false)
  in
- let comments = Flow_ast_utils.merge_comments ~outer:comments ~inner:key_comments in
  let props =
  [
- ("key", identifier id);
+ ("key", private_identifier key);
  ("value", option expression value);
  ("typeAnnotation", hint type_annotation annot);
+ ("computed", bool false);
  ("static", bool static);
  ("variance", option variance variance_);
  ]
@@ -996,7 +996,7 @@ with type t = Impl.t = struct
  else
  []
  in
- node ?comments "ClassPrivateProperty" loc props
+ node ?comments "PropertyDefinition" loc props
  and class_property
  (loc, { Class.Property.key; value; annot; static; variance = variance_; comments }) =
  let (key, computed, comments) =
@@ -1030,7 +1030,7 @@ with type t = Impl.t = struct
  else
  []
  in
- node ?comments "ClassProperty" loc props
+ node ?comments "PropertyDefinition" loc props
  and enum_declaration (loc, { Statement.EnumDeclaration.id; body; comments }) =
  let open Statement.EnumDeclaration in
  let enum_body =
@@ -1887,7 +1887,7 @@ with type t = Impl.t = struct
  let (property, computed) =
  match property with
  | Expression.Member.PropertyIdentifier id -> (identifier id, false)
- | Expression.Member.PropertyPrivateName name -> (private_name name, false)
+ | Expression.Member.PropertyPrivateName name -> (private_identifier name, false)
  | Expression.Member.PropertyExpression expr -> (expression expr, true)
  in
  [("object", expression _object); ("property", property); ("computed", bool computed)]

diff --git a/src/parser/test/esprima_tests.js b/src/parser/test/esprima_tests.js
@@ -2569,8 +2569,28 @@ module.exports = {
  },
  },
  },
- 'class Foo { [1 + 1]: string; }',
- 'class Foo { 123:string; }',
+ {
+ content: 'class Foo { [1 + 1]: string; }',
+ explanation: 'Esprima-fb returns ClassProperty, but Flow returns PropertyDefinition',
+ expected_differences: {
+ 'root.body.0.body.body.0.type': {
+ type: 'Wrong string',
+ expected: 'ClassProperty',
+ actual: 'PropertyDefinition'
+ }
+ }
+ },
+ {
+ content: 'class Foo { 123:string; }',
+ explanation: 'Esprima-fb returns ClassProperty, but Flow returns PropertyDefinition',
+ expected_differences: {
+ 'root.body.0.body.body.0.type': {
+ type: 'Wrong string',
+ expected: 'ClassProperty',
+ actual: 'PropertyDefinition'
+ }
+ }
+ },
  {
  content: 'class Foo { "bar"<T>() { } }',
  explanation: "Esprima-fb doesn't include params in " +
@@ -2588,10 +2608,60 @@ module.exports = {
  },
  },
  },
- 'class Foo { "prop1":string; }',
- 'class Foo { [prop1]: string; }' ,
- 'class Foo { prop1:string; prop2:number; }',
- 'class Foo { static prop1:string; prop2:number; }',
+ {
+ content: 'class Foo { "prop1":string; }',
+ explanation: 'Esprima-fb returns ClassProperty, but Flow returns PropertyDefinition',
+ expected_differences: {
+ 'root.body.0.body.body.0.type': {
+ type: 'Wrong string',
+ expected: 'ClassProperty',
+ actual: 'PropertyDefinition'
+ }
+ }
+ },
+ {
+ content: 'class Foo { [prop1]: string; }',
+ explanation: 'Esprima-fb returns ClassProperty, but Flow returns PropertyDefinition',
+ expected_differences: {
+ 'root.body.0.body.body.0.type': {
+ type: 'Wrong string',
+ expected: 'ClassProperty',
+ actual: 'PropertyDefinition'
+ }
+ }
+ },
+ {
+ content: 'class Foo { prop1:string; prop2:number; }',
+ explanation: 'Esprima-fb returns ClassProperty, but Flow returns PropertyDefinition',
+ expected_differences: {
+ 'root.body.0.body.body.0.type': {
+ type: 'Wrong string',
+ expected: 'ClassProperty',
+ actual: 'PropertyDefinition'
+ },
+ 'root.body.0.body.body.1.type': {
+ type: 'Wrong string',
+ expected: 'ClassProperty',
+ actual: 'PropertyDefinition'
+ }
+ }
+ },
+ {
+ content: 'class Foo { static prop1:string; prop2:number; }',
+ explanation: 'Esprima-fb returns ClassProperty, but Flow returns PropertyDefinition',
+ expected_differences: {
+ 'root.body.0.body.body.0.type': {
+ type: 'Wrong string',
+ expected: 'ClassProperty',
+ actual: 'PropertyDefinition'
+ },
+ 'root.body.0.body.body.1.type': {
+ type: 'Wrong string',
+ expected: 'ClassProperty',
+ actual: 'PropertyDefinition'
+ }
+ },
+ },
  {
  content: 'class Foo {set fooProp(value:number){}}',
  explanation: "Esprima-fb doesn't include params in " +

diff --git a/src/parser/test/flow/async_await/migrated_0005.tree.json b/src/parser/test/flow/async_await/migrated_0005.tree.json
@@ -21,7 +21,7 @@
  "range":[12,30],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":14},"end":{"line":1,"column":28}},
  "range":[14,28],
  "key":{

diff --git a/src/parser/test/flow/class_method_kinds/multiple_constructors.tree.json b/src/parser/test/flow/class_method_kinds/multiple_constructors.tree.json
@@ -136,7 +136,7 @@
  "range":[74,115],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":7,"column":2},"end":{"line":7,"column":28}},
  "range":[78,104],
  "key":{
@@ -188,7 +188,7 @@
  "range":[127,161],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":11,"column":2},"end":{"line":11,"column":21}},
  "range":[131,150],
  "key":{
@@ -337,7 +337,7 @@
  "range":[255,286],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":20,"column":2},"end":{"line":20,"column":19}},
  "range":[259,276],
  "key":{
@@ -389,7 +389,7 @@
  "range":[303,342],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":24,"column":2},"end":{"line":24,"column":26}},
  "range":[307,331],
  "key":{

diff --git a/src/parser/test/flow/class_properties/async.tree.json b/src/parser/test/flow/class_properties/async.tree.json
@@ -21,7 +21,7 @@
  "range":[8,19],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":2,"column":2},"end":{"line":2,"column":7}},
  "range":[12,17],
  "key":{

diff --git a/src/parser/test/flow/class_properties/async_annotated.tree.json b/src/parser/test/flow/class_properties/async_annotated.tree.json
@@ -21,7 +21,7 @@
  "range":[8,28],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":2,"column":2},"end":{"line":2,"column":16}},
  "range":[12,26],
  "key":{

diff --git a/src/parser/test/flow/class_properties/async_asi.tree.json b/src/parser/test/flow/class_properties/async_asi.tree.json
@@ -21,7 +21,7 @@
  "range":[8,43],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":2,"column":2},"end":{"line":2,"column":7}},
  "range":[12,17],
  "key":{

diff --git a/src/parser/test/flow/class_properties/async_initializer.tree.json b/src/parser/test/flow/class_properties/async_initializer.tree.json
@@ -21,7 +21,7 @@
  "range":[8,27],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":2,"column":2},"end":{"line":2,"column":15}},
  "range":[12,25],
  "key":{

diff --git a/src/parser/test/flow/class_properties/computed.tree.json b/src/parser/test/flow/class_properties/computed.tree.json
@@ -21,7 +21,7 @@
  "range":[8,21],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":2,"column":2},"end":{"line":2,"column":9}},
  "range":[12,19],
  "key":{

diff --git a/src/parser/test/flow/class_properties/get.tree.json b/src/parser/test/flow/class_properties/get.tree.json
@@ -21,7 +21,7 @@
  "range":[8,17],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":2,"column":2},"end":{"line":2,"column":5}},
  "range":[12,15],
  "key":{

diff --git a/src/parser/test/flow/class_properties/migrated_0000.tree.json b/src/parser/test/flow/class_properties/migrated_0000.tree.json
@@ -21,7 +21,7 @@
  "range":[17,23],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":21}},
  "range":[19,21],
  "key":{

diff --git a/src/parser/test/flow/class_properties/migrated_0001.tree.json b/src/parser/test/flow/class_properties/migrated_0001.tree.json
@@ -21,7 +21,7 @@
  "range":[17,31],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":29}},
  "range":[19,29],
  "key":{

diff --git a/src/parser/test/flow/class_properties/migrated_0002.tree.json b/src/parser/test/flow/class_properties/migrated_0002.tree.json
@@ -21,7 +21,7 @@
  "range":[17,33],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":31}},
  "range":[19,31],
  "key":{

diff --git a/src/parser/test/flow/class_properties/migrated_0003.tree.json b/src/parser/test/flow/class_properties/migrated_0003.tree.json
@@ -55,7 +55,7 @@
  "range":[17,35],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":20}},
  "range":[19,20],
  "key":{

diff --git a/src/parser/test/flow/class_properties/migrated_0004.tree.json b/src/parser/test/flow/class_properties/migrated_0004.tree.json
@@ -21,7 +21,7 @@
  "range":[17,41],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":39}},
  "range":[19,39],
  "key":{

diff --git a/src/parser/test/flow/class_properties/migrated_0005.tree.json b/src/parser/test/flow/class_properties/migrated_0005.tree.json
@@ -21,7 +21,7 @@
  "range":[17,30],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":28}},
  "range":[19,28],
  "key":{

diff --git a/src/parser/test/flow/class_properties/migrated_0006.tree.json b/src/parser/test/flow/class_properties/migrated_0006.tree.json
@@ -21,7 +21,7 @@
  "range":[17,38],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":36}},
  "range":[19,36],
  "key":{

diff --git a/src/parser/test/flow/class_properties/migrated_0007.tree.json b/src/parser/test/flow/class_properties/migrated_0007.tree.json
@@ -21,7 +21,7 @@
  "range":[17,40],
  "body":[
  {
- "type":"ClassProperty",
+ "type":"PropertyDefinition",
  "loc":{"source":null,"start":{"line":1,"column":19},"end":{"line":1,"column":38}},
  "range":[19,38],
  "key":{