getBindingIdentifiers should return params for private methods (#13723)

* docs: add comments on binding kind

* refactor: simplify For collector visitor

* getBinding does not return falsy values

* chore: table style tests

* add more test cases

* fix: return params for private methods

packages/babel-traverse/src/scope/binding.ts

@@ -3,14 +3,14 @@ import type * as t from "@babel/types";
 import type Scope from "./index";
 
 type BindingKind =
-  | "var"
+  | "var" /* var declarator */
-  | "let"
+  | "let" /* let declarator, class declaration id, catch clause parameters */
-  | "const"
+  | "const" /* const declarator */
-  | "module"
+  | "module" /* import specifiers */
-  | "hoisted"
+  | "hoisted" /* function declaration id */
-  | "param"
+  | "param" /* function declaration parameters */
-  | "local"
+  | "local" /* function expression id, class expression id */
-  | "unknown";
+  | "unknown"; /* export specifiers */
 /**
  * This class is responsible for a binding inside of a scope.
  *

packages/babel-traverse/src/scope/index.ts

@@ -5,7 +5,6 @@ import type { TraverseOptions } from "../index";
 import Binding from "./binding";
 import globals from "globals";
 import {
-  FOR_INIT_KEYS,
   NOT_LOCAL_BINDING,
   callExpression,
   cloneNode,
@@ -224,16 +223,13 @@ interface CollectVisitorState {
 }
 
 const collectorVisitor: Visitor<CollectVisitorState> = {
-  For(path) {
+  ForStatement(path) {
-    for (const key of FOR_INIT_KEYS) {
+    const declar = path.get("init");
-      // todo: might be not needed with improvement to babel-types
+    // delegate block scope handling to the `BlockScoped` method
-      const declar = path.get(key) as NodePath;
+    if (declar.isVar()) {
-      // delegate block scope handling to the `BlockScoped` method
+      const { scope } = path;
-      if (declar.isVar()) {
+      const parentScope = scope.getFunctionParent() || scope.getProgramParent();
-        const parentScope =
+      parentScope.registerBinding("var", declar);
-          path.scope.getFunctionParent() || path.scope.getProgramParent();
-        parentScope.registerBinding("var", declar);
-      }
     }
   },
 
@@ -269,6 +265,12 @@ const collectorVisitor: Visitor<CollectVisitorState> = {
     if (left.isPattern() || left.isIdentifier()) {
       state.constantViolations.push(path);
     }
+    // delegate block scope handling to the `BlockScoped` method
+    else if (left.isVar()) {
+      const { scope } = path;
+      const parentScope = scope.getFunctionParent() || scope.getProgramParent();
+      parentScope.registerBinding("var", left);
+    }
   },
 
   ExportDeclaration: {
@@ -282,12 +284,12 @@ const collectorVisitor: Visitor<CollectVisitorState> = {
         if (!id) return;
 
         const binding = scope.getBinding(id.name);
-        if (binding) binding.reference(path);
+        binding?.reference(path);
       } else if (isVariableDeclaration(declar)) {
         for (const decl of declar.declarations) {
           for (const name of Object.keys(getBindingIdentifiers(decl))) {
             const binding = scope.getBinding(name);
-            if (binding) binding.reference(path);
+            binding?.reference(path);
           }
         }
       }

packages/babel-types/src/retrievers/getBindingIdentifiers.ts

@@ -121,6 +121,7 @@ getBindingIdentifiers.keys = {
   ArrowFunctionExpression: ["params"],
   ObjectMethod: ["params"],
   ClassMethod: ["params"],
+  ClassPrivateMethod: ["params"],
 
   ForInStatement: ["left"],
   ForOfStatement: ["left"],

packages/babel-types/test/retrievers.js

@@ -7,20 +7,73 @@ function getBody(program) {
 
 describe("retrievers", function () {
   describe("getBindingIdentifiers", function () {
-    it("variable declarations", function () {
+    it.each([
-      const program = "var a = 1; let b = 2; const c = 3;";
+      [
-      const ids = t.getBindingIdentifiers(getBody(program));
+        "variable declarations",
-      expect(Object.keys(ids)).toEqual(["a", "b", "c"]);
+        getBody("var a = 1; let b = 2; const c = 3;"),
-    });
+        ["a", "b", "c"],
-    it("function declarations", function () {
+      ],
-      const program = "var foo = 1; function bar() { var baz = 2; }";
+      [
-      const ids = t.getBindingIdentifiers(getBody(program));
+        "function declarations",
-      expect(Object.keys(ids)).toEqual(["bar", "foo"]);
+        getBody("var foo = 1; function bar() { var baz = 2; }"),
-    });
+        ["bar", "foo"],
-    it("export named declarations", function () {
+      ],
-      const program = "export const foo = 'foo';";
+      [
-      const ids = t.getBindingIdentifiers(getBody(program));
+        "object methods",
-      expect(Object.keys(ids)).toEqual(["foo"]);
+        getBody("({ a(b) { let c } })")[0].expression.properties[0],
+        ["b"],
+      ],
+      [
+        "class methods",
+        getBody("(class { a(b) { let c } })")[0].expression.body.body,
+        ["b"],
+      ],
+      [
+        "class private methods",
+        getBody("(class { #a(b) { let c } })")[0].expression.body.body,
+        ["b"],
+      ],
+      [
+        "export named declarations",
+        getBody("export const foo = 'foo';"),
+        ["foo"],
+      ],
+      [
+        "export default class declarations",
+        getBody("export default class foo {}"),
+        ["foo"],
+      ],
+      [
+        "export default referenced identifiers",
+        getBody("export default foo"),
+        [],
+      ],
+      ["export all declarations", getBody("export * from 'x'"), []],
+      [
+        "export all as namespace declarations",
+        getBody("export * as ns from 'x'"),
+        [], // exported bindings are not associated with declarations
+      ],
+      [
+        "export namespace specifiers",
+        getBody("export * as ns from 'x'")[0].specifiers,
+        ["ns"],
+      ],
+      [
+        "object patterns",
+        getBody("const { a, b: { ...c } = { d } } = {}"),
+        ["a", "c"],
+      ],
+      [
+        "array patterns",
+        getBody("var [ a, ...{ b, ...c } ] = {}"),
+        ["a", "b", "c"],
+      ],
+      ["update expression", getBody("++x")[0].expression, ["x"]],
+      ["assignment expression", getBody("x ??= 1")[0].expression, ["x"]],
+    ])("%s", (_, program, bindingNames) => {
+      const ids = t.getBindingIdentifiers(program);
+      expect(Object.keys(ids)).toEqual(bindingNames);
     });
   });
 });