[parser] Disallow duplicate and undeclared private names (#10456)

* [parser] Add private names tracking to Scope - Disallow duplicate private names - Disallow undeclared private names * Update tests * Test all possible duplications * Test undeclared private names * Better error message for top-level private names * Fix flow * Update test262 whitelist * Update fixtures * Update flow whitelist * Remove old output.json * Move ClassScopeHandler to a separate class * Make the code readable
babel · Jan 10, 2020 · 771c730 · 771c730
1 parent 9f148a1
commit 771c730
Show file tree

Hide file tree

Showing 223 changed files with 16,948 additions and 259 deletions.
diff --git a/packages/babel-generator/test/fixtures/types/ClassBody-ClassProperty/input.js b/packages/babel-generator/test/fixtures/types/ClassBody-ClassProperty/input.js
@@ -28,9 +28,9 @@ class Foo {
   foo = 0; bar = 1;
 
   #foo;
-  #foo = 1;
-  static #foo;
-  static #foo = Foo.#foo;
+  #foo2 = 1;
+  static #foo3;
+  static #foo4 = Foo.#foo;
 }
 
 class A1 {

diff --git a/packages/babel-generator/test/fixtures/types/ClassBody-ClassProperty/output.js b/packages/babel-generator/test/fixtures/types/ClassBody-ClassProperty/output.js
@@ -27,9 +27,9 @@ class Foo {
   foo = 0;
   bar = 1;
   #foo;
-  #foo = 1;
-  static #foo;
-  static #foo = Foo.#foo;
+  #foo2 = 1;
+  static #foo3;
+  static #foo4 = Foo.#foo;
 }
 
 class A1 {
@@ -69,4 +69,4 @@ class A6 {
 class A7 {
   static get static() {}
 
-}
+}
diff --git a/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/input.js b/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/input.js
@@ -6,13 +6,13 @@ class Foo {
   set foo(bar) {}
 
   async #foo() {}
-  #foo() {}
-  get #foo() {}
-  set #foo(bar) {}
-  * #foo() {}
-  async * #foo() {}
-  get #bar() {}
-  set #baz(taz) {}
+  #foo2() {}
+  get #foo3() {}
+  set #foo3(bar) {}
+  * #foo4() {}
+  async * #foo5() {}
+  get #bar6() {}
+  set #baz6(taz) {}
 
   static async foo() {}
   static foo() {}
@@ -23,13 +23,13 @@ class Foo {
   static * foo() {}
   static async * foo() {}
 
-  static #foo() {}
-  static async #foo() {}
+  static #foo7() {}
+  static async #foo8() {}
   static ["foo"]() {}
-  static get #foo() {}
-  static set #foo(taz) {}
-  static * #foo() {}
-  static async * #foo() {}
+  static get #foo9() {}
+  static set #foo9(taz) {}
+  static * #foo10() {}
+  static async * #foo11() {}
 
   get
   () {}

diff --git a/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/output.js b/packages/babel-generator/test/fixtures/types/ClassBody-MethodDefinition/output.js
@@ -11,19 +11,19 @@ class Foo {
 
   async #foo() {}
 
-  #foo() {}
+  #foo2() {}
 
-  get #foo() {}
+  get #foo3() {}
 
-  set #foo(bar) {}
+  set #foo3(bar) {}
 
-  *#foo() {}
+  *#foo4() {}
 
-  async *#foo() {}
+  async *#foo5() {}
 
-  get #bar() {}
+  get #bar6() {}
 
-  set #baz(taz) {}
+  set #baz6(taz) {}
 
   static async foo() {}
 
@@ -41,19 +41,19 @@ class Foo {
 
   static async *foo() {}
 
-  static #foo() {}
+  static #foo7() {}
 
-  static async #foo() {}
+  static async #foo8() {}
 
   static ["foo"]() {}
 
-  static get #foo() {}
+  static get #foo9() {}
 
-  static set #foo(taz) {}
+  static set #foo9(taz) {}
 
-  static *#foo() {}
+  static *#foo10() {}
 
-  static async *#foo() {}
+  static async *#foo11() {}
 
   get() {}
 

diff --git a/packages/babel-parser/src/parser/base.js b/packages/babel-parser/src/parser/base.js
@@ -4,12 +4,14 @@ import type { Options } from "../options";
 import type State from "../tokenizer/state";
 import type { PluginsMap } from "./index";
 import type ScopeHandler from "../util/scope";
+import type ClassScopeHandler from "../util/class-scope";
 
 export default class BaseParser {
   // Properties set by constructor in index.js
   options: Options;
   inModule: boolean;
   scope: ScopeHandler<*>;
+  classScope: ClassScopeHandler;
   plugins: PluginsMap;
   filename: ?string;
   sawUnambiguousESM: boolean = false;

diff --git a/packages/babel-parser/src/parser/expression.js b/packages/babel-parser/src/parser/expression.js
@@ -612,15 +612,21 @@ export default class ExpressionParser extends LValParser {
         ? this.parseIdentifier(true)
         : this.parseMaybePrivateName();
       node.computed = computed;
-      if (
-        node.property.type === "PrivateName" &&
-        node.object.type === "Super"
-      ) {
-        this.raise(startPos, "Private fields can't be accessed on super");
+
+      if (node.property.type === "PrivateName") {
+        if (node.object.type === "Super") {
+          this.raise(startPos, "Private fields can't be accessed on super");
+        }
+        this.classScope.usePrivateName(
+          node.property.id.name,
+          node.property.start,
+        );
       }
+
       if (computed) {
         this.expect(tt.bracketR);
       }
+
       if (state.optionalChainMember) {
         node.optional = optional;
         return this.finishNode(node, "OptionalMemberExpression");

diff --git a/packages/babel-parser/src/parser/index.js b/packages/babel-parser/src/parser/index.js
@@ -7,6 +7,7 @@ import { getOptions } from "../options";
 import StatementParser from "./statement";
 import { SCOPE_ASYNC, SCOPE_PROGRAM } from "../util/scopeflags";
 import ScopeHandler from "../util/scope";
+import ClassScopeHandler from "../util/class-scope";
 
 export type PluginsMap = Map<string, { [string]: any }>;
 
@@ -25,6 +26,7 @@ export default class Parser extends StatementParser {
     this.options = options;
     this.inModule = this.options.sourceType === "module";
     this.scope = new ScopeHandler(this.raise.bind(this), this.inModule);
+    this.classScope = new ClassScopeHandler(this.raise.bind(this));
     this.plugins = pluginsMap(this.options.plugins);
     this.filename = options.sourceFilename;
   }

diff --git a/packages/babel-parser/src/parser/statement.js b/packages/babel-parser/src/parser/statement.js
@@ -20,6 +20,11 @@ import {
   SCOPE_OTHER,
   SCOPE_SIMPLE_CATCH,
   SCOPE_SUPER,
+  CLASS_ELEMENT_OTHER,
+  CLASS_ELEMENT_INSTANCE_GETTER,
+  CLASS_ELEMENT_INSTANCE_SETTER,
+  CLASS_ELEMENT_STATIC_GETTER,
+  CLASS_ELEMENT_STATIC_SETTER,
   type BindingTypes,
 } from "../util/scopeflags";
 
@@ -1171,7 +1176,7 @@ export default class StatementParser extends ExpressionParser {
   }
 
   parseClassBody(constructorAllowsSuper: boolean): N.ClassBody {
-    this.state.classLevel++;
+    this.classScope.enter();
 
     const state = { hadConstructor: false };
     let decorators: N.Decorator[] = [];
@@ -1231,7 +1236,7 @@ export default class StatementParser extends ExpressionParser {
       );
     }
 
-    this.state.classLevel--;
+    this.classScope.exit();
 
     return this.finishNode(classBody, "ClassBody");
   }
@@ -1514,7 +1519,15 @@ export default class StatementParser extends ExpressionParser {
     prop: N.ClassPrivateProperty,
   ) {
     this.expectPlugin("classPrivateProperties", prop.key.start);
-    classBody.body.push(this.parseClassPrivateProperty(prop));
+
+    const node = this.parseClassPrivateProperty(prop);
+    classBody.body.push(node);
+
+    this.classScope.declarePrivateName(
+      node.key.id.name,
+      CLASS_ELEMENT_OTHER,
+      node.key.start,
+    );
   }
 
   pushClassMethod(
@@ -1545,17 +1558,29 @@ export default class StatementParser extends ExpressionParser {
     isAsync: boolean,
   ): void {
     this.expectPlugin("classPrivateMethods", method.key.start);
-    classBody.body.push(
-      this.parseMethod(
-        method,
-        isGenerator,
-        isAsync,
-        /* isConstructor */ false,
-        false,
-        "ClassPrivateMethod",
-        true,
-      ),
+
+    const node = this.parseMethod(
+      method,
+      isGenerator,
+      isAsync,
+      /* isConstructor */ false,
+      false,
+      "ClassPrivateMethod",
+      true,
     );
+    classBody.body.push(node);
+
+    const kind =
+      node.kind === "get"
+        ? node.static
+          ? CLASS_ELEMENT_STATIC_GETTER
+          : CLASS_ELEMENT_INSTANCE_GETTER
+        : node.kind === "set"
+        ? node.static
+          ? CLASS_ELEMENT_STATIC_SETTER
+          : CLASS_ELEMENT_INSTANCE_SETTER
+        : CLASS_ELEMENT_OTHER;
+    this.classScope.declarePrivateName(node.key.id.name, kind, node.key.start);
   }
 
   // Overridden in typescript.js

diff --git a/packages/babel-parser/src/tokenizer/index.js b/packages/babel-parser/src/tokenizer/index.js
@@ -391,14 +391,8 @@ export default class Tokenizer extends LocationParser {
     }
 
     if (
-      (this.hasPlugin("classPrivateProperties") ||
-        this.hasPlugin("classPrivateMethods")) &&
-      this.state.classLevel > 0
-    ) {
-      ++this.state.pos;
-      this.finishToken(tt.hash);
-      return;
-    } else if (
+      this.hasPlugin("classPrivateProperties") ||
+      this.hasPlugin("classPrivateMethods") ||
       this.getPluginOption("pipelineOperator", "proposal") === "smart"
     ) {
       this.finishOp(tt.hash, 1);

diff --git a/packages/babel-parser/src/tokenizer/state.js b/packages/babel-parser/src/tokenizer/state.js
@@ -77,9 +77,6 @@ export default class State {
   soloAwait: boolean = false;
   inFSharpPipelineDirectBody: boolean = false;
 
-  // Check whether we are in a (nested) class or not.
-  classLevel: number = 0;
-
   // Labels in scope.
   labels: Array<{
     kind: ?("loop" | "switch"),