Fix parsing of private fields (#566)

The computed key is not part of the spec.
key for ClassProperties is an Expression
Do not parse computed and literal keys for PrivateClassProperties

ast/spec.md

@@ -1054,7 +1054,7 @@ interface ClassMethod <: Function {
 ```js
 interface ClassProperty <: Node {
   type: "ClassProperty";
-  key: Identifier;
+  key: Expression;
   value: Expression;
   computed: boolean;
 }

src/parser/expression.js

@@ -578,7 +578,7 @@ export default class ExpressionParser extends LValParser {
         if (this.hasPlugin("classPrivateProperties")) {
           return this.parseMaybePrivateName();
         } else {
-          this.unexpected();
+          throw this.unexpected();
         }
 
       case tt._new:
@@ -1011,20 +1011,19 @@ export default class ExpressionParser extends LValParser {
     if (!node) this.unexpected();
   }
 
-  parsePropertyName(prop: N.ObjectOrClassMember): N.Identifier {
+  parsePropertyName(prop: N.ObjectOrClassMember): N.Expression {
     if (this.eat(tt.bracketL)) {
-      // $FlowFixMe (ClassPrivateMember shouldn't be allowed to be computed!)
       prop.computed = true;
       prop.key = this.parseMaybeAssign();
       this.expect(tt.bracketR);
     } else {
-      // $FlowFixMe (ClassPrivateMember shouldn't be allowed to be computed!)
       prop.computed = false;
       const oldInPropertyName = this.state.inPropertyName;
       this.state.inPropertyName = true;
       prop.key = (this.match(tt.num) || this.match(tt.string)) ? this.parseExprAtom() : this.parseIdentifier(true);
       this.state.inPropertyName = oldInPropertyName;
     }
+
     return prop.key;
   }
 

src/parser/statement.js

@@ -642,9 +642,7 @@ export default class StatementParser extends ExpressionParser {
 
   isNonstaticConstructor(method: N.ClassMethod | N.ClassProperty): boolean {
     return !method.computed && !method.static && (
-      // $FlowFixMe ('key' downcasting)
       (method.key.name === "constructor") || // Identifier
-      // $FlowFixMe ('key' downcasting)
       (method.key.value === "constructor")   // Literal
     );
   }
@@ -707,7 +705,7 @@ export default class StatementParser extends ExpressionParser {
     if (this.hasPlugin("classPrivateProperties") && this.match(tt.hash)) { // Private property
       this.next();
       const privateProp: N.ClassPrivateProperty = memberAny;
-      this.parsePropertyName(privateProp);
+      privateProp.key = this.parseIdentifier(true);
       classBody.body.push(this.parsePrivateClassProperty(privateProp));
       return;
     }
@@ -749,7 +747,6 @@ export default class StatementParser extends ExpressionParser {
 
     const isSimple = this.match(tt.name);
     const key = this.parsePropertyName(methodOrProp);
-    // $FlowFixMe ('key' downcasting)
     if (!methodOrProp.computed && methodOrProp.static && (methodOrProp.key.name === "prototype" || methodOrProp.key.value === "prototype")) {
       this.raise(methodOrProp.key.start, "Classes may not have static property named prototype");
     }

src/plugins/flow.js

@@ -1294,7 +1294,6 @@ export default (superClass: Class<Parser>): Class<Parser> => class extends super
   parsePropertyName(node: N.ObjectOrClassMember): N.Identifier {
     const variance = this.flowParseVariance();
     const key = super.parsePropertyName(node);
-    // $FlowFixMe (variance not defined on ClassPrivateProperty)
     node.variance = variance;
     return key;
   }

src/types.js

@@ -339,7 +339,7 @@ export type ObjectExpression = NodeBase & {
   properties: $ReadOnlyArray<ObjectProperty | ObjectMethod | SpreadElement>;
 };
 
-export type ObjectOrClassMember = ClassMethod | ClassProperty | ClassPrivateProperty | ObjectMember;
+export type ObjectOrClassMember = ClassMethod | ClassProperty | ObjectMember;
 
 export type ObjectMember = ObjectProperty | ObjectMethod;
 
@@ -359,7 +359,7 @@ export type ObjectProperty = ObjectMemberBase & {
   shorthand: boolean;
 };
 
-export type ObjectMethod = ObjectMemberBase & MethodBase &  {
+export type ObjectMethod = ObjectMemberBase & MethodBase & {
   type: "ObjectMethod";
   kind: "get" | "set" | "method"; // Never "constructor"
 };
@@ -579,7 +579,7 @@ export type ClassMethod = MethodBase & ClassMemberBase & {
 
 export type ClassProperty = ClassMemberBase & {
   type: "ClassProperty";
-  key: Identifier;
+  key: Expression;
   value: ?Expression; // TODO: Not in spec that this is nullable.
 
   typeAnnotation?: ?FlowTypeAnnotation; // TODO: Not in spec

test/fixtures/experimental/class-private-properties/asi-success/expected.json

@@ -89,7 +89,6 @@
                   "column": 4
                 }
               },
-              "computed": false,
               "key": {
                 "type": "Identifier",
                 "start": 15,
@@ -123,7 +122,6 @@
                   "column": 4
                 }
               },
-              "computed": false,
               "key": {
                 "type": "Identifier",
                 "start": 20,
@@ -149,4 +147,4 @@
     ],
     "directives": []
   }
-}
+}

test/fixtures/experimental/class-private-properties/failure-computed/actual.js

@@ -1,4 +1,4 @@
 class Foo {
   #p = x
-  [#m] () {}
+  #[m] = 1
 }

test/fixtures/experimental/class-private-properties/failure-computed/options.json

@@ -1,5 +1,5 @@
 {
-  "throws": "Unexpected token, expected ; (3:10)",
+  "throws": "Unexpected token (3:3)",
   "plugins": [
     "classProperties",
     "classPrivateProperties"

test/fixtures/experimental/class-private-properties/failure-method/actual.js

@@ -0,0 +1,4 @@
+class Foo {
+  #p = x
+  #m () {}
+}

test/fixtures/experimental/class-private-properties/failure-method/options.json

@@ -1,3 +1,4 @@
 {
+  "throws": "Unexpected token, expected ; (3:5)",
   "plugins": ["classProperties", "classPrivateProperties"]
 }

test/fixtures/experimental/class-private-properties/failure-numeric-literal/actual.js

@@ -0,0 +1,3 @@
+class Foo {
+  #2 = y
+}

test/fixtures/experimental/class-private-properties/failure-numeric-literal/options.json

@@ -0,0 +1,7 @@
+{
+  "throws": "Unexpected token (2:3)",
+  "plugins": [
+    "classProperties",
+    "classPrivateProperties"
+    ]
+}

test/fixtures/experimental/class-private-properties/failure-string-literal/actual.js

@@ -0,0 +1,3 @@
+class Foo {
+  #"p" = x
+}

test/fixtures/experimental/class-private-properties/failure-string-literal/options.json

@@ -0,0 +1,7 @@
+{
+  "throws": "Unexpected token (2:3)",
+  "plugins": [
+    "classProperties",
+    "classPrivateProperties"
+    ]
+}

test/fixtures/experimental/class-private-properties/inline/expected.json

@@ -89,7 +89,6 @@
                   "column": 13
                 }
               },
-              "computed": false,
               "key": {
                 "type": "Identifier",
                 "start": 11,
@@ -123,7 +122,6 @@
                   "column": 17
                 }
               },
-              "computed": false,
               "key": {
                 "type": "Identifier",
                 "start": 15,
@@ -207,7 +205,6 @@
                   "column": 17
                 }
               },
-              "computed": false,
               "key": {
                 "type": "Identifier",
                 "start": 32,
@@ -260,7 +257,6 @@
                   "column": 25
                 }
               },
-              "computed": false,
               "key": {
                 "type": "Identifier",
                 "start": 40,
@@ -305,4 +301,4 @@
     ],
     "directives": []
   }
-}
+}

test/fixtures/experimental/class-private-properties/pbn-success.1/actual.js

@@ -1,19 +0,0 @@
-class Point {
-    #x;
-    #y;
-
-    constructor(x = 0, y = 0) {
-        #x = +x;
-        #y = +y;
-    }
-
-    get x() { return this.#x }
-    set x(value) { this.#x = +value }
-
-    get y() { return this.#y }
-    set y(value) { this.#y = +value }
-
-    equals(p) { return this.#x === p.#x && this.#y === p.#y }
-
-    toString() { return `Point<${ this.#x },${ this.#y }>` }
-}

test/fixtures/experimental/class-private-properties/pbn-success/expected.json

@@ -89,7 +89,6 @@
                   "column": 7
                 }
               },
-              "computed": false,
               "key": {
                 "type": "Identifier",
                 "start": 19,
@@ -123,7 +122,6 @@
                   "column": 7
                 }
               },
-              "computed": false,
               "key": {
                 "type": "Identifier",
                 "start": 27,
@@ -1623,4 +1621,4 @@
     ],
     "directives": []
   }
-}
+}