[parser] Disallow duplicate private names

babel · Sep 17, 2019 · cee6b25 · cee6b25
1 parent 99f4f6c
commit cee6b25
Show file tree

Hide file tree

Showing 16 changed files with 156 additions and 92 deletions.
diff --git a/packages/babel-parser/src/parser/statement.js b/packages/babel-parser/src/parser/statement.js
@@ -1177,6 +1177,7 @@ export default class StatementParser extends ExpressionParser {
     this.state.classLevel++;
 
     const state = { hadConstructor: false };
+    const privateNames: Set<string> = new Set();
     let decorators: N.Decorator[] = [];
     const classBody: N.ClassBody = this.startNode();
     classBody.body = [];
@@ -1212,7 +1213,13 @@ export default class StatementParser extends ExpressionParser {
           decorators = [];
         }
 
-        this.parseClassMember(classBody, member, state, constructorAllowsSuper);
+        this.parseClassMember(
+          classBody,
+          member,
+          state,
+          constructorAllowsSuper,
+          privateNames,
+        );
 
         if (
           member.kind === "constructor" &&
@@ -1244,6 +1251,7 @@ export default class StatementParser extends ExpressionParser {
     member: N.ClassMember,
     state: { hadConstructor: boolean },
     constructorAllowsSuper: boolean,
+    privateNames: Set<string>,
   ): void {
     let isStatic = false;
     const containsEsc = this.state.containsEsc;
@@ -1291,6 +1299,7 @@ export default class StatementParser extends ExpressionParser {
       state,
       isStatic,
       constructorAllowsSuper,
+      privateNames,
     );
   }
 
@@ -1300,6 +1309,7 @@ export default class StatementParser extends ExpressionParser {
     state: { hadConstructor: boolean },
     isStatic: boolean,
     constructorAllowsSuper: boolean,
+    privateNames: Set<string>,
   ) {
     const publicMethod: $FlowSubtype<N.ClassMethod> = member;
     const privateMethod: $FlowSubtype<N.ClassPrivateMethod> = member;
@@ -1318,7 +1328,13 @@ export default class StatementParser extends ExpressionParser {
 
       if (method.key.type === "PrivateName") {
         // Private generator method
-        this.pushClassPrivateMethod(classBody, privateMethod, true, false);
+        this.pushClassPrivateMethod(
+          classBody,
+          privateMethod,
+          true,
+          false,
+          privateNames,
+        );
         return;
       }
 
@@ -1350,7 +1366,13 @@ export default class StatementParser extends ExpressionParser {
       method.kind = "method";
 
       if (isPrivate) {
-        this.pushClassPrivateMethod(classBody, privateMethod, false, false);
+        this.pushClassPrivateMethod(
+          classBody,
+          privateMethod,
+          false,
+          false,
+          privateNames,
+        );
         return;
       }
 
@@ -1385,7 +1407,7 @@ export default class StatementParser extends ExpressionParser {
       );
     } else if (this.isClassProperty()) {
       if (isPrivate) {
-        this.pushClassPrivateProperty(classBody, privateProp);
+        this.pushClassPrivateProperty(classBody, privateProp, privateNames);
       } else {
         this.pushClassProperty(classBody, publicProp);
       }
@@ -1409,6 +1431,7 @@ export default class StatementParser extends ExpressionParser {
           privateMethod,
           isGenerator,
           true,
+          privateNames,
         );
       } else {
         if (this.isNonstaticConstructor(publicMethod)) {
@@ -1441,7 +1464,13 @@ export default class StatementParser extends ExpressionParser {
 
       if (method.key.type === "PrivateName") {
         // private getter/setter
-        this.pushClassPrivateMethod(classBody, privateMethod, false, false);
+        this.pushClassPrivateMethod(
+          classBody,
+          privateMethod,
+          false,
+          false,
+          privateNames,
+        );
       } else {
         if (this.isNonstaticConstructor(publicMethod)) {
           this.raise(
@@ -1463,7 +1492,7 @@ export default class StatementParser extends ExpressionParser {
     } else if (this.isLineTerminator()) {
       // an uninitialized class property (due to ASI, since we don't otherwise recognize the next token)
       if (isPrivate) {
-        this.pushClassPrivateProperty(classBody, privateProp);
+        this.pushClassPrivateProperty(classBody, privateProp, privateNames);
       } else {
         this.pushClassProperty(classBody, publicProp);
       }
@@ -1511,9 +1540,15 @@ export default class StatementParser extends ExpressionParser {
   pushClassPrivateProperty(
     classBody: N.ClassBody,
     prop: N.ClassPrivateProperty,
+    privateNames: Set<string>,
   ) {
     this.expectPlugin("classPrivateProperties", prop.key.start);
-    classBody.body.push(this.parseClassPrivateProperty(prop));
+    classBody.body.push(
+      this.assertNoDuplicatePrivateName(
+        this.parseClassPrivateProperty(prop),
+        privateNames,
+      ),
+    );
   }
 
   pushClassMethod(
@@ -1542,21 +1577,58 @@ export default class StatementParser extends ExpressionParser {
     method: N.ClassPrivateMethod,
     isGenerator: boolean,
     isAsync: boolean,
+    privateNames: Set<string>,
   ): void {
     this.expectPlugin("classPrivateMethods", method.key.start);
     classBody.body.push(
-      this.parseMethod(
-        method,
-        isGenerator,
-        isAsync,
-        /* isConstructor */ false,
-        false,
-        "ClassPrivateMethod",
-        true,
+      this.assertNoDuplicatePrivateName(
+        this.parseMethod(
+          method,
+          isGenerator,
+          isAsync,
+          /* isConstructor */ false,
+          false,
+          "ClassPrivateMethod",
+          true,
+        ),
+        privateNames,
       ),
     );
   }
 
+  assertNoDuplicatePrivateName<
+    T: N.ClassPrivateMethod | N.ClassPrivateProperty,
+  >(methodOrProp: T, privateNames: Set<string>) {
+    const { name } = methodOrProp.key.id;
+
+    let isDuplicate = privateNames.has(name);
+    privateNames.add(name);
+
+    const kind =
+      methodOrProp.type === "ClassPrivateMethod" && methodOrProp.kind;
+
+    // Accessors can have the same name for get and set
+    if (kind === "get" || kind === "set") {
+      const otherKind = kind === "get" ? "set" : "get";
+
+      // If the current private name (e.g. #x) has already been seen,
+      // it's possible that was an accessor of the other kind.
+      // In this case, only throw if an accessor of the same kind has
+      // already been seen.
+      if (isDuplicate && privateNames.has(`${otherKind} ${name}`)) {
+        isDuplicate = privateNames.has(`${kind} ${name}`);
+      }
+
+      privateNames.add(`${kind} ${name}`);
+    }
+
+    if (isDuplicate) {
+      this.raise(methodOrProp.key.start, `Duplicate private name #${name}`);
+    }
+
+    return methodOrProp;
+  }
+
   // Overridden in typescript.js
   parsePostMemberNameModifiers(
     // eslint-disable-next-line no-unused-vars

diff --git a/packages/babel-parser/src/plugins/flow.js b/packages/babel-parser/src/plugins/flow.js
@@ -2107,8 +2107,6 @@ export default (superClass: Class<Parser>): Class<Parser> =>
     pushClassPrivateMethod(
       classBody: N.ClassBody,
       method: N.ClassPrivateMethod,
-      isGenerator: boolean,
-      isAsync: boolean,
     ): void {
       if ((method: $FlowFixMe).variance) {
         this.unexpected((method: $FlowFixMe).variance.start);
@@ -2118,7 +2116,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
         method.typeParameters = this.flowParseTypeParameterDeclaration();
       }
 
-      super.pushClassPrivateMethod(classBody, method, isGenerator, isAsync);
+      super.pushClassPrivateMethod(...arguments);
     }
 
     // parse a the super class type parameters and implements

diff --git a/packages/babel-parser/src/plugins/typescript/index.js b/packages/babel-parser/src/plugins/typescript/index.js
@@ -1795,24 +1795,18 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       return this.tsParseModifier(["public", "protected", "private"]);
     }
 
-    parseClassMember(
-      classBody: N.ClassBody,
-      member: any,
-      state: { hadConstructor: boolean },
-      constructorAllowsSuper: boolean,
-    ): void {
+    parseClassMember(classBody: N.ClassBody, member: any): void {
       const accessibility = this.parseAccessModifier();
       if (accessibility) member.accessibility = accessibility;
 
-      super.parseClassMember(classBody, member, state, constructorAllowsSuper);
+      super.parseClassMember(...arguments);
     }
 
     parseClassMemberWithIsStatic(
       classBody: N.ClassBody,
       member: any,
       state: { hadConstructor: boolean },
       isStatic: boolean,
-      constructorAllowsSuper: boolean,
     ): void {
       const methodOrProp: N.ClassMethod | N.ClassProperty = member;
       const prop: N.ClassProperty = member;
@@ -1853,13 +1847,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
         return;
       }
 
-      super.parseClassMemberWithIsStatic(
-        classBody,
-        member,
-        state,
-        isStatic,
-        constructorAllowsSuper,
-      );
+      super.parseClassMemberWithIsStatic(...arguments);
     }
 
     parsePostMemberNameModifiers(
@@ -2035,12 +2023,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
     pushClassPrivateMethod(
       classBody: N.ClassBody,
       method: N.ClassPrivateMethod,
-      isGenerator: boolean,
-      isAsync: boolean,
     ): void {
       const typeParameters = this.tsTryParseTypeParameters();
       if (typeParameters) method.typeParameters = typeParameters;
-      super.pushClassPrivateMethod(classBody, method, isGenerator, isAsync);
+      super.pushClassPrivateMethod(...arguments);
     }
 
     parseClassSuper(node: N.Class): void {

diff --git a/...rser/test/fixtures/experimental/class-private-methods/duplicated-accessor-method/input.js b/...rser/test/fixtures/experimental/class-private-methods/duplicated-accessor-method/input.js
@@ -0,0 +1,4 @@
+class A {
+  get #x() {}
+  #x() {}
+}
diff --git a/.../test/fixtures/experimental/class-private-methods/duplicated-accessor-method/options.json b/.../test/fixtures/experimental/class-private-methods/duplicated-accessor-method/options.json
@@ -0,0 +1,7 @@
+{
+  "plugins": [
+    "classPrivateMethods",
+    "classPrivateProperties"
+  ],
+  "throws": "Duplicate private name #x (3:2)"
+}
diff --git a/...-parser/test/fixtures/experimental/class-private-methods/duplicated-field-method/input.js b/...-parser/test/fixtures/experimental/class-private-methods/duplicated-field-method/input.js
@@ -0,0 +1,4 @@
+class A {
+  #x;
+  #x() {}
+}
diff --git a/...ser/test/fixtures/experimental/class-private-methods/duplicated-field-method/options.json b/...ser/test/fixtures/experimental/class-private-methods/duplicated-field-method/options.json
@@ -0,0 +1,7 @@
+{
+  "plugins": [
+    "classPrivateMethods",
+    "classPrivateProperties"
+  ],
+  "throws": "Duplicate private name #x (3:2)"
+}
diff --git a/...rser/test/fixtures/experimental/class-private-methods/duplicated-method-accessor/input.js b/...rser/test/fixtures/experimental/class-private-methods/duplicated-method-accessor/input.js
@@ -0,0 +1,4 @@
+class A {
+  #x() {}
+  get #x() {}
+}
diff --git a/.../test/fixtures/experimental/class-private-methods/duplicated-method-accessor/options.json b/.../test/fixtures/experimental/class-private-methods/duplicated-method-accessor/options.json
@@ -0,0 +1,7 @@
+{
+  "plugins": [
+    "classPrivateMethods",
+    "classPrivateProperties"
+  ],
+  "throws": "Duplicate private name #x (3:6)"
+}
diff --git a/...-parser/test/fixtures/experimental/class-private-methods/duplicated-method-field/input.js b/...-parser/test/fixtures/experimental/class-private-methods/duplicated-method-field/input.js
@@ -0,0 +1,4 @@
+class A {
+  #x() {}
+  #x;
+}
diff --git a/...ser/test/fixtures/experimental/class-private-methods/duplicated-method-field/options.json b/...ser/test/fixtures/experimental/class-private-methods/duplicated-method-field/options.json
@@ -0,0 +1,7 @@
+{
+  "plugins": [
+    "classPrivateMethods",
+    "classPrivateProperties"
+  ],
+  "throws": "Duplicate private name #x (3:2)"
+}
diff --git a/...st/fixtures/experimental/class-private-properties/duplicated-different-placement/input.js b/...st/fixtures/experimental/class-private-properties/duplicated-different-placement/input.js
@@ -0,0 +1,4 @@
+class A {
+  #x = 1;
+  static #x = 2;
+}
diff --git a/...ixtures/experimental/class-private-properties/duplicated-different-placement/options.json b/...ixtures/experimental/class-private-properties/duplicated-different-placement/options.json
@@ -0,0 +1,6 @@
+{
+  "plugins": [
+    "classPrivateProperties"
+  ],
+  "throws": "Duplicate private name #x (3:9)"
+}
diff --git a/...er/test/fixtures/experimental/class-private-properties/duplicated-same-placement/input.js b/...er/test/fixtures/experimental/class-private-properties/duplicated-same-placement/input.js
@@ -0,0 +1,4 @@
+class A {
+  #x = 1;
+  #x = 2;
+}
diff --git a/...est/fixtures/experimental/class-private-properties/duplicated-same-placement/options.json b/...est/fixtures/experimental/class-private-properties/duplicated-same-placement/options.json
@@ -0,0 +1,6 @@
+{
+  "plugins": [
+    "classPrivateProperties"
+  ],
+  "throws": "Duplicate private name #x (3:2)"
+}