Fix .parentPath after rename in SwitchCase (#15287)

nicolo-ribaudo · web-flow · commit d84291113054 · 2022-12-19T19:20:10.000+01:00
diff --git 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<S>(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
@@ -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) {
@@ -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
@@ -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");
+        },
+      });
+    });
+  });
 });
