From d8429111305491e57f8afee705b20647cad65995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Mon, 19 Dec 2022 19:20:10 +0100 Subject: [PATCH] Fix `.parentPath` after rename in `SwitchCase` (#15287) --- packages/babel-traverse/src/scope/index.ts | 8 +++++++ .../babel-traverse/src/scope/lib/renamer.ts | 22 +++++++++++-------- packages/babel-traverse/test/scope.js | 20 +++++++++++++++++ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/packages/babel-traverse/src/scope/index.ts b/packages/babel-traverse/src/scope/index.ts index ac957a3f1cdb..8a4a8aa823f1 100644 --- a/packages/babel-traverse/src/scope/index.ts +++ b/packages/babel-traverse/src/scope/index.ts @@ -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(node: any, opts: any, state?: S) { traverse(node, opts, this, state, this.path); diff --git a/packages/babel-traverse/src/scope/lib/renamer.ts b/packages/babel-traverse/src/scope/lib/renamer.ts index c3ba79706d01..b2390d3a1400 100644 --- a/packages/babel-traverse/src/scope/lib/renamer.ts +++ b/packages/babel-traverse/src/scope/lib/renamer.ts @@ -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 = { ReferencedIdentifier({ node }, state) { @@ -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; @@ -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); diff --git a/packages/babel-traverse/test/scope.js b/packages/babel-traverse/test/scope.js index db12f7924da1..5a15dfae581f 100644 --- a/packages/babel-traverse/test/scope.js +++ b/packages/babel-traverse/test/scope.js @@ -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"); + }, + }); + }); + }); });