Skip to content

Commit

Permalink
Clean up traverse scope (#12797)
Browse files Browse the repository at this point in the history
  • Loading branch information
@JLHwung
JLHwung committed Feb 16, 2021
1 parent 4819ce7 commit 4462eea
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 35 deletions.
50 changes: 18 additions & 32 deletions packages/babel-traverse/src/scope/index.ts
View file
Expand Up @@ -203,10 +203,7 @@ const collectorVisitor: Visitor<CollectVisitorState> = {
if (path.isBlockScoped()) return;

// this will be hit again once we traverse into it after this iteration
// @ts-expect-error todo(flow->ts): might be not correct for export all declaration
if (path.isExportDeclaration() && path.get("declaration").isDeclaration()) {
return;
}
if (path.isExportDeclaration()) return;

// we've ran into a declaration!
const parent =
Expand All @@ -228,7 +225,8 @@ const collectorVisitor: Visitor<CollectVisitorState> = {
ExportDeclaration: {
exit(path) {
const { node, scope } = path;
// @ts-expect-error todo(flow->ts) declaration is not present on ExportAllDeclaration
// ExportAllDeclaration does not have `declaration`
if (t.isExportAllDeclaration(node)) return;
const declar = node.declaration;
if (t.isClassDeclaration(declar) || t.isFunctionDeclaration(declar)) {
const id = declar.id;
Expand All @@ -248,8 +246,6 @@ const collectorVisitor: Visitor<CollectVisitorState> = {
},

LabeledStatement(path) {
// @ts-expect-error todo(flow->ts): possible bug - statement might not have name and so should not be added as global
path.scope.getProgramParent().addGlobal(path.node);
path.scope.getBlockParent().registerDeclaration(path);
},

Expand Down Expand Up @@ -283,15 +279,6 @@ const collectorVisitor: Visitor<CollectVisitorState> = {
}
},

Block(path) {
const paths = path.get("body");
for (const bodyPath of paths) {
if (bodyPath.isFunctionDeclaration()) {
path.scope.getBlockParent().registerDeclaration(bodyPath);
}
}
},

CatchClause(path) {
path.scope.registerBinding("let", path);
},
Expand Down Expand Up @@ -873,22 +860,6 @@ export default class Scope {
this.uids = Object.create(null);
this.data = Object.create(null);

// TODO: explore removing this as it should be covered by collectorVisitor
if (path.isFunction()) {
if (
path.isFunctionExpression() &&
path.has("id") &&
!path.get("id").node[t.NOT_LOCAL_BINDING]
) {
this.registerBinding("local", path.get("id"), path);
}

const params: Array<NodePath> = path.get("params");
for (const param of params) {
this.registerBinding("param", param);
}
}

const programParent = this.getProgramParent();
if (programParent.crawling) return;

Expand All @@ -899,6 +870,21 @@ export default class Scope {
};

this.crawling = true;
// traverse does not visit the root node, here we explicitly collect
// root node binding info when the root is not a Program.
if (path.type !== "Program" && collectorVisitor._exploded) {
// @ts-expect-error when collectorVisitor is exploded, `enter` always exists
for (const visit of collectorVisitor.enter) {
visit(path, state);
}
const typeVisitors = collectorVisitor[path.type];
if (typeVisitors) {
// @ts-expect-error when collectorVisitor is exploded, `enter` always exists
for (const visit of typeVisitors.enter) {
visit(path, state);
}
}
}
path.traverse(collectorVisitor, state);
this.crawling = false;

Expand Down
6 changes: 6 additions & 0 deletions packages/babel-traverse/test/__snapshots__/scope.js.snap
View file
Expand Up @@ -4,12 +4,16 @@ exports[`scope duplicate bindings catch const 1`] = `"Duplicate declaration \\"e

exports[`scope duplicate bindings catch let 1`] = `"Duplicate declaration \\"e\\""`;

exports[`scope duplicate bindings global class/class 1`] = `"Duplicate declaration \\"foo\\""`;

exports[`scope duplicate bindings global class/const 1`] = `"Duplicate declaration \\"foo\\""`;

exports[`scope duplicate bindings global class/function 1`] = `"Duplicate declaration \\"foo\\""`;

exports[`scope duplicate bindings global class/let 1`] = `"Duplicate declaration \\"foo\\""`;

exports[`scope duplicate bindings global class/var 1`] = `"Duplicate declaration \\"foo\\""`;

exports[`scope duplicate bindings global const/class 1`] = `"Duplicate declaration \\"foo\\""`;

exports[`scope duplicate bindings global const/const 1`] = `"Duplicate declaration \\"foo\\""`;
Expand All @@ -34,4 +38,6 @@ exports[`scope duplicate bindings global let/let 1`] = `"Duplicate declaration \

exports[`scope duplicate bindings global let/var 1`] = `"Duplicate declaration \\"foo\\""`;

exports[`scope duplicate bindings global var/class 1`] = `"Duplicate declaration \\"foo\\""`;

exports[`scope duplicate bindings global var/let 1`] = `"Duplicate declaration \\"foo\\""`;
119 changes: 116 additions & 3 deletions packages/babel-traverse/test/scope.js
View file
Expand Up @@ -2,7 +2,7 @@ import traverse, { NodePath } from "../lib";
import { parse } from "@babel/parser";
import * as t from "@babel/types";

function getPath(code, options) {
function getPath(code, options): NodePath<t.Program> {
const ast =
typeof code === "string" ? parse(code, options) : createNode(code);
let path;
Expand Down Expand Up @@ -92,6 +92,82 @@ describe("scope", () => {
});
});

describe("import declaration", () => {
it.each([
[
"import default",
"import foo from 'foo';(foo)=>{}",
"foo",
"ImportDefaultSpecifier",
],
[
"import named default",
"import { default as foo } from 'foo';(foo)=>{}",
"foo",
"ImportSpecifier",
],
[
"import named",
"import { foo } from 'foo';(foo)=>{}",
"foo",
"ImportSpecifier",
],
[
"import named aliased",
"import { _foo as foo } from 'foo';(foo)=>{}",
"foo",
"ImportSpecifier",
],
[
"import namespace",
"import * as foo from 'foo';(foo)=>{}",
"foo",
"ImportNamespaceSpecifier",
],
])("%s", (testTitle, source, bindingName, bindingNodeType) => {
expect(
getPath(source, { sourceType: "module" }).scope.getBinding(
bindingName,
).path.type,
).toBe(bindingNodeType);
});
});

describe("export declaration", () => {
it.each([
[
"export default function",
"export default function foo(foo) {}",
"foo",
"FunctionDeclaration",
],
[
"export default class",
"export default class foo extends function foo () {} {}",
"foo",
"ClassDeclaration",
],
[
"export named default",
"export const foo = function foo(foo) {};",
"foo",
"VariableDeclarator",
],
[
"export named default",
"export const [ { foo } ] = function foo(foo) {};",
"foo",
"VariableDeclarator",
],
])("%s", (testTitle, source, bindingName, bindingNodeType) => {
expect(
getPath(source, { sourceType: "module" }).scope.getBinding(
bindingName,
).path.type,
).toBe(bindingNodeType);
});
});

it("variable declaration", function () {
expect(getPath("var foo = null;").scope.getBinding("foo").path.type).toBe(
"VariableDeclarator",
Expand Down Expand Up @@ -284,6 +360,41 @@ describe("scope", () => {
});
});

describe("after crawl", () => {
it("modified function identifier available in function scope", () => {
const path = getPath("(function f(f) {})")
.get("body")[0]
.get("expression");
path.get("id").replaceWith(t.identifier("g"));
path.scope.crawl();
const binding = path.scope.getBinding("g");
expect(binding.kind).toBe("local");
});
it("modified function param available in function scope", () => {
const path = getPath("(function f(f) {})")
.get("body")[0]
.get("expression");
path.get("params")[0].replaceWith(t.identifier("g"));
path.scope.crawl();
const binding = path.scope.getBinding("g");
expect(binding.kind).toBe("param");
});
it("modified class identifier available in class expression scope", () => {
const path = getPath("(class c {})").get("body")[0].get("expression");
path.get("id").replaceWith(t.identifier("g"));
path.scope.crawl();
const binding = path.scope.getBinding("g");
expect(binding.kind).toBe("local");
});
it("modified class identifier available in class declaration scope", () => {
const path = getPath("class c {}").get("body")[0];
path.get("id").replaceWith(t.identifier("g"));
path.scope.crawl();
const binding = path.scope.getBinding("g");
expect(binding.kind).toBe("let");
});
});

it("class identifier available in class scope after crawl", function () {
const path = getPath("class a { build() { return new a(); } }");

Expand Down Expand Up @@ -421,7 +532,6 @@ describe("scope", () => {
// unless node1 === node2
const cases = [
["const", "let", false],

["const", "const", false],
["const", "function", false],
["const", "class", false],
Expand All @@ -432,11 +542,14 @@ describe("scope", () => {
["let", "function", false],
["let", "var", false],

//["var", "class", true],
["var", "class", false],
["var", "function", true],
["var", "var", true],

["class", "function", false],
["class", "class", false],

["function", "function", true],
];

const createNode = function (kind) {
Expand Down

0 comments on commit 4462eea

Please sign in to comment.