Fix parsing of private fields

The computed key is not part of the spec. key for ClassProperties is an Expression Do not parse computed and literal keys for PrivateClassProperties
babel · Jun 5, 2017 · 9026b8e · 9026b8e
1 parent e579109
commit 9026b8e
Show file tree

Hide file tree

Showing 16 changed files with 28 additions and 1,918 deletions.
diff --git a/ast/spec.md b/ast/spec.md
@@ -1051,7 +1051,7 @@ interface ClassMethod <: Function {
 ```js
 interface ClassProperty <: Node {
   type: "ClassProperty";
-  key: Identifier;
+  key: Expression;
   value: Expression;
   computed: boolean;
 }

diff --git a/src/parser/expression.js b/src/parser/expression.js
@@ -970,20 +970,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;
   }
 

diff --git a/src/parser/statement.js b/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");
     }

diff --git a/src/plugins/flow.js b/src/plugins/flow.js
@@ -1284,7 +1284,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;
   }

diff --git a/src/types.js b/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

diff --git a/test/fixtures/experimental/class-private-properties/asi-failure-computed/actual.js b/test/fixtures/experimental/class-private-properties/asi-failure-computed/actual.js
@@ -1,4 +1,4 @@
 class Foo {
   #p = x
-  [#m] () {}
+  #[m] = 1
 }
diff --git a/test/fixtures/experimental/class-private-properties/asi-failure-computed/options.json b/test/fixtures/experimental/class-private-properties/asi-failure-computed/options.json
@@ -1,5 +1,5 @@
 {
-  "throws": "Unexpected token, expected ; (3:10)",
+  "throws": "Unexpected token (3:3)",
   "plugins": [
     "classProperties",
     "classPrivateProperties"

diff --git a/test/fixtures/experimental/class-private-properties/asi-failure-literal/actual.js b/test/fixtures/experimental/class-private-properties/asi-failure-literal/actual.js
@@ -0,0 +1,4 @@
+class Foo {
+  #"p" = x
+  #2 = y
+}
diff --git a/test/fixtures/experimental/class-private-properties/asi-failure-literal/options.json b/test/fixtures/experimental/class-private-properties/asi-failure-literal/options.json
@@ -0,0 +1,7 @@
+{
+  "throws": "Unexpected token (2:3)",
+  "plugins": [
+    "classProperties",
+    "classPrivateProperties"
+    ]
+}
diff --git a/test/fixtures/experimental/class-private-properties/asi-failure-method/actual.js b/test/fixtures/experimental/class-private-properties/asi-failure-method/actual.js
@@ -0,0 +1,4 @@
+class Foo {
+  #p = x
+  #m () {}
+}
diff --git a/...ate-properties/pbn-success.1/options.json → ...roperties/asi-failure-method/options.json b/...ate-properties/pbn-success.1/options.json → ...roperties/asi-failure-method/options.json
@@ -1,3 +1,4 @@
 {
+  "throws": "Unexpected token, expected ; (3:5)",
   "plugins": ["classProperties", "classPrivateProperties"]
 }
diff --git a/test/fixtures/experimental/class-private-properties/asi-success/expected.json b/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": []
   }
-}
+}
diff --git a/test/fixtures/experimental/class-private-properties/inline/expected.json b/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": []
   }
-}
+}
diff --git a/test/fixtures/experimental/class-private-properties/pbn-success.1/actual.js b/test/fixtures/experimental/class-private-properties/pbn-success.1/actual.js