Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getBindingIdentifiers should return params for private methods #13723

Merged
merged 6 commits into from Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions packages/babel-traverse/src/scope/binding.ts
Expand Up @@ -3,14 +3,14 @@ import type * as t from "@babel/types";
import type Scope from "./index";

type BindingKind =
| "var"
| "let"
| "const"
| "module"
| "hoisted"
| "param"
| "local"
| "unknown";
| "var" /* var declarator */
| "let" /* let declarator, class declaration id, catch clause parameters */
| "const" /* const declarator */
| "module" /* import specifiers */
| "hoisted" /* function declaration id */
| "param" /* function declaration parameters */
| "local" /* function expression id, class expression id */
| "unknown"; /* export specifiers */
/**
* This class is responsible for a binding inside of a scope.
*
Expand Down
28 changes: 15 additions & 13 deletions packages/babel-traverse/src/scope/index.ts
Expand Up @@ -5,7 +5,6 @@ import type { TraverseOptions } from "../index";
import Binding from "./binding";
import globals from "globals";
import {
FOR_INIT_KEYS,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice refactor here!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FOR_INIT_KEYS is only used here. After this PR we may even remove it in Babel 8 if nobody else is using.

NOT_LOCAL_BINDING,
callExpression,
cloneNode,
Expand Down Expand Up @@ -224,16 +223,13 @@ interface CollectVisitorState {
}

const collectorVisitor: Visitor<CollectVisitorState> = {
For(path) {
for (const key of FOR_INIT_KEYS) {
// todo: might be not needed with improvement to babel-types
const declar = path.get(key) as NodePath;
// delegate block scope handling to the `BlockScoped` method
if (declar.isVar()) {
const parentScope =
path.scope.getFunctionParent() || path.scope.getProgramParent();
parentScope.registerBinding("var", declar);
}
ForStatement(path) {
const declar = path.get("init");
// delegate block scope handling to the `BlockScoped` method
if (declar.isVar()) {
const { scope } = path;
const parentScope = scope.getFunctionParent() || scope.getProgramParent();
parentScope.registerBinding("var", declar);
}
},

Expand Down Expand Up @@ -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: {
Expand All @@ -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);
}
}
}
Expand Down
Expand Up @@ -121,6 +121,7 @@ getBindingIdentifiers.keys = {
ArrowFunctionExpression: ["params"],
ObjectMethod: ["params"],
ClassMethod: ["params"],
ClassPrivateMethod: ["params"],

ForInStatement: ["left"],
ForOfStatement: ["left"],
Expand Down
81 changes: 67 additions & 14 deletions packages/babel-types/test/retrievers.js
Expand Up @@ -7,20 +7,73 @@ function getBody(program) {

describe("retrievers", function () {
describe("getBindingIdentifiers", function () {
it("variable declarations", function () {
const program = "var a = 1; let b = 2; const c = 3;";
const ids = t.getBindingIdentifiers(getBody(program));
expect(Object.keys(ids)).toEqual(["a", "b", "c"]);
});
it("function declarations", function () {
const program = "var foo = 1; function bar() { var baz = 2; }";
const ids = t.getBindingIdentifiers(getBody(program));
expect(Object.keys(ids)).toEqual(["bar", "foo"]);
});
it("export named declarations", function () {
const program = "export const foo = 'foo';";
const ids = t.getBindingIdentifiers(getBody(program));
expect(Object.keys(ids)).toEqual(["foo"]);
it.each([
[
"variable declarations",
getBody("var a = 1; let b = 2; const c = 3;"),
["a", "b", "c"],
],
[
"function declarations",
getBody("var foo = 1; function bar() { var baz = 2; }"),
["bar", "foo"],
],
[
"object methods",
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);
});
});
});