Skip to content

Commit

Permalink
Fix .parentPath after rename in SwitchCase (#15287)
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolo-ribaudo committed Dec 19, 2022
1 parent 87e48e7 commit d842911
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 9 deletions.
8 changes: 8 additions & 0 deletions packages/babel-traverse/src/scope/index.ts
Expand Up @@ -457,6 +457,14 @@ export default class Scope {
traverse(node: t.Node | t.Node[], opts?: TraverseOptions, state?: any): void;
/**
* Traverse node with current scope and path.
*
* !!! WARNING !!!
* This method assumes that `this.path` is the NodePath representing `node`.
* After running the traversal, the `.parentPath` of the NodePaths
* corresponding to `node`'s children will be set to `this.path`.
*
* There is no good reason to use this method, since the only safe way to use
* it is equivalent to `scope.path.traverse(opts, state)`.
*/
traverse<S>(node: any, opts: any, state?: S) {
traverse(node, opts, this, state, this.path);
Expand Down
22 changes: 13 additions & 9 deletions packages/babel-traverse/src/scope/lib/renamer.ts
Expand Up @@ -3,6 +3,8 @@ import splitExportDeclaration from "@babel/helper-split-export-declaration";
import * as t from "@babel/types";
import type { NodePath, Visitor } from "../..";
import { requeueComputedKeyAndDecorators } from "@babel/helper-environment-visitor";
import { traverseNode } from "../../traverse-node";
import { explode } from "../../visitors";

const renameVisitor: Visitor<Renamer> = {
ReferencedIdentifier({ node }, state) {
Expand Down Expand Up @@ -111,6 +113,7 @@ export default class Renamer {
// );
}

// TODO(Babel 8): Remove this `block` parameter. It's not needed anywhere.
rename(block?: t.Pattern | t.Scopable) {
const { binding, oldName, newName } = this;
const { scope, path } = binding;
Expand All @@ -130,15 +133,16 @@ export default class Renamer {
}
}

const blockToTraverse = block || scope.block;
if (blockToTraverse?.type === "SwitchStatement") {
// discriminant is not part of current scope, should be skipped.
blockToTraverse.cases.forEach(c => {
scope.traverse(c, renameVisitor, this);
});
} else {
scope.traverse(blockToTraverse, renameVisitor, this);
}
traverseNode(
block || scope.block,
explode(renameVisitor),
scope,
this,
scope.path,
// When blockToTraverse is a SwitchStatement, the discriminant
// is not part of the current scope and thus should be skipped.
{ discriminant: true },
);

if (!block) {
scope.removeOwnBinding(oldName);
Expand Down
20 changes: 20 additions & 0 deletions packages/babel-traverse/test/scope.js
Expand Up @@ -1019,4 +1019,24 @@ describe("scope", () => {
expect(program.scope.hasOwnBinding("ref")).toBe(true);
});
});

describe("rename", () => {
it(".parentPath after renaming variable in switch", () => {
const program = getPath(`
switch (x) {
case y:
let a;
}
`);
program.traverse({
VariableDeclaration(path) {
if (path.node.declarations[0].id.name !== "a") return;

expect(path.parentPath.type).toBe("SwitchCase");
path.scope.rename("a");
expect(path.parentPath.type).toBe("SwitchCase");
},
});
});
});
});

0 comments on commit d842911

Please sign in to comment.