Skip to content

Commit

Permalink
Improve getBindingIdentifiers (#16544)
Browse files Browse the repository at this point in the history
* Add more getBindingIdentifiers tests

* fix: do not return class id from getOuterBindingId

* update expression should not introduce new bindings

* fix: register class expression binding from its id node

* defer the fix to Babel 8

* remove unused FlowIgnore comment
  • Loading branch information
JLHwung authored Jun 4, 2024
1 parent ec0c62a commit 07bd000
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 12 deletions.
2 changes: 1 addition & 1 deletion packages/babel-traverse/src/scope/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ const collectorVisitor: Visitor<CollectVisitorState> = {
// @ts-expect-error Fixme: document symbol ast properties
!path.get("id").node[NOT_LOCAL_BINDING]
) {
path.scope.registerBinding("local", path);
path.scope.registerBinding("local", path.get("id"), path);
}
},
TSTypeAnnotation(path) {
Expand Down
23 changes: 14 additions & 9 deletions packages/babel-types/src/retrievers/getBindingIdentifiers.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import {
isExportDeclaration,
isIdentifier,
isClassExpression,
isDeclaration,
isFunctionDeclaration,
isFunctionExpression,
isExportAllDeclaration,
isAssignmentExpression,
isUnaryExpression,
isUpdateExpression,
} from "../validators/generated/index.ts";
import type * as t from "../index.ts";

Expand Down Expand Up @@ -51,20 +53,18 @@ function getBindingIdentifiers(

if (
newBindingsOnly &&
// These two nodes do not introduce _new_ bindings, but they are included
// These nodes do not introduce _new_ bindings, but they are included
// in getBindingIdentifiers.keys for backwards compatibility.
// TODO(@nicolo-ribaudo): Check if we can remove them from .keys in a
// backward-compatible way, and if not what we need to do to remove them
// in Babel 8.
(isAssignmentExpression(id) || isUnaryExpression(id))
(isAssignmentExpression(id) ||
isUnaryExpression(id) ||
isUpdateExpression(id))
) {
continue;
}

const keys =
// @ts-expect-error getBindingIdentifiers.keys do not cover all AST types
getBindingIdentifiers.keys[id.type];

if (isIdentifier(id)) {
if (duplicates) {
const _ids = (ids[id.name] = ids[id.name] || []);
Expand All @@ -88,11 +88,18 @@ function getBindingIdentifiers(
continue;
}

if (isFunctionExpression(id)) {
if (
isFunctionExpression(id) ||
(process.env.BABEL_8_BREAKING && isClassExpression(id))
) {
continue;
}
}

const keys =
// @ts-expect-error getBindingIdentifiers.keys do not cover all AST types
getBindingIdentifiers.keys[id.type];

if (keys) {
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
Expand All @@ -105,8 +112,6 @@ function getBindingIdentifiers(
}
}
}

// $FlowIssue Object.create() seems broken
return ids;
}

Expand Down
199 changes: 197 additions & 2 deletions packages/babel-types/test/retrievers.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import {
describeBabel7,
describeBabel8,
} from "../../../scripts/repo-utils/index.cjs";
import * as t from "../lib/index.js";
import { parse } from "@babel/parser";

Expand All @@ -15,8 +19,28 @@ describe("retrievers", function () {
],
[
"function declarations",
getBody("var foo = 1; function bar() { var baz = 2; }"),
["bar", "foo"],
getBody("function bar() { var baz = 2; }"),
["bar"],
],
[
"function declarations with parameters",
getBody(
"function f(a, { b }, c = 1, [{ _: [ d ], ...e }]) { var baz = 2; }",
),
["f", "a", "c", "b", "e", "d"],
],
[
"function expressions with parameters",
getBody(
"(function f(a, { b }, c = 1, [{ _: [ d ], ...e }]) { var baz = 2; })",
)[0].expression,
["f", "a", "c", "b", "e", "d"],
],
["class declarations", getBody("class C { a(b) { let c } }")[0], ["C"]],
[
"class expressions",
getBody("(class C { a(b) { let c } })")[0].expression,
["C"],
],
[
"object methods",
Expand All @@ -33,11 +57,28 @@ describe("retrievers", function () {
getBody("(class { #a(b) { let c } })")[0].expression.body.body,
["b"],
],
[
"for-in statement",
getBody("for ([{ _: [ d ], ...e }] in rhs);"),
["e", "d"],
],
[
"for-of statement",
getBody("for ([{ _: [ d ], ...e }] of rhs);"),
["e", "d"],
],
["catch clause", getBody("try { } catch (e) {}")[0].handler, ["e"]],
["labeled statement", getBody("label: x"), ["label"]],
[
"export named declarations",
getBody("export const foo = 'foo';"),
["foo"],
],
[
"export function declarations",
getBody("export function foo() {}"),
["foo"],
],
[
"export default class declarations",
getBody("export default class foo {}"),
Expand Down Expand Up @@ -69,11 +110,165 @@ describe("retrievers", function () {
getBody("var [ a, ...{ b, ...c } ] = {}"),
["a", "b", "c"],
],
["unary expression", getBody("void x")[0].expression, ["x"]],
["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);
});
});
describe("getBindingIdentifiers(%, /* duplicates */ true)", function () {
it.each([
["variable declarations", getBody("var a = 1, a = 2"), { a: 2 }],
[
"function declarations with parameters",
getBody("function f(f) { var f = 1; }"),
{ f: 2 },
],
])("%s", (_, program, expected) => {
const ids = t.getBindingIdentifiers(program, true);
for (const name of Object.keys(ids)) {
ids[name] = Array.isArray(ids[name]) ? ids[name].length : 1;
}
expect(ids).toEqual(expected);
});
});
describe("getOuterBindingIdentifiers", function () {
it.each([
[
"variable declarations",
getBody("var a = 1; let b = 2; const c = 3;"),
["a", "b", "c"],
],
[
"function declarations",
getBody("function bar() { var baz = 2; }"),
["bar"],
],
[
"function declarations with parameters",
getBody(
"function f(a, { b }, c = 1, [{ _: [ d ], ...e }]) { var baz = 2; }",
),
["f"],
],
[
"function expressions with parameters",
getBody(
"(function f(a, { b }, c = 1, [{ _: [ d ], ...e }]) { var baz = 2; })",
)[0].expression,
[],
],
["class declarations", getBody("class C { a(b) { let c } }")[0], ["C"]],
[
"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"],
],
[
"for-in statement",
getBody("for ([{ _: [ d ], ...e }] in rhs);"),
["e", "d"],
],
[
"for-of statement",
getBody("for ([{ _: [ d ], ...e }] of rhs);"),
["e", "d"],
],
["catch clause", getBody("try { } catch (e) {}")[0].handler, ["e"]],
["labeled statement", getBody("label: x"), ["label"]],
[
"export named declarations",
getBody("export const foo = 'foo';"),
["foo"],
],
[
"export function declarations",
getBody("export function 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"],
],
["unary expression", getBody("void x")[0].expression, ["x"]],
["update expression", getBody("++x")[0].expression, ["x"]],
["assignment expression", getBody("x ??= 1")[0].expression, ["x"]],
])("%s", (_, program, bindingNames) => {
const ids = t.getOuterBindingIdentifiers(program);
expect(Object.keys(ids)).toEqual(bindingNames);
});
});
describeBabel7("getOuterBindingIdentifiers - Babel 7", function () {
it.each([
[
"class expressions",
getBody("(class C { a(b) { let c } })")[0].expression,
["C"],
],
])("%s", (_, program, bindingNames) => {
const ids = t.getOuterBindingIdentifiers(program);
expect(Object.keys(ids)).toEqual(bindingNames);
});
});
describeBabel8("getOuterBindingIdentifiers - Babel 8", function () {
it.each([
[
"class expressions",
getBody("(class C { a(b) { let c } })")[0].expression,
[],
],
])("%s", (_, program, bindingNames) => {
const ids = t.getOuterBindingIdentifiers(program);
expect(Object.keys(ids)).toEqual(bindingNames);
});
});
describe("getBindingIdentifiers(%, false, false, /* newBindingsOnly */ true)", function () {
it.each([
["unary expression", getBody("void x")[0].expression, []],
["update expression", getBody("++x")[0].expression, []],
["assignment expression", getBody("x ??= 1")[0].expression, []],
])("%s", (_, program, bindingNames) => {
const ids = t.getBindingIdentifiers(program, false, false, true);
expect(Object.keys(ids)).toEqual(bindingNames);
});
});
});

0 comments on commit 07bd000

Please sign in to comment.