address review comments

babel · Dec 23, 2019 · 63dd253 · 63dd253
1 parent 349e57d
commit 63dd253
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 8 deletions.
diff --git a/packages/babel-traverse/src/path/lib/virtual-types.js b/packages/babel-traverse/src/path/lib/virtual-types.js
@@ -64,7 +64,8 @@ export const Expression = {
 };
 
 export const Scope = {
-  types: ["Scopable"],
+  // When pattern is inside the function params, it is a scope
+  types: ["Scopable", "Pattern"],
   checkPath(path) {
     return t.isScope(path.node, path.parent);
   },

diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js
@@ -902,14 +902,14 @@ export default class Scope {
     do {
       const binding = scope.getOwnBinding(name);
       if (binding) {
-        // When a pattern is a Scope, it is a part of parameter expressions.
+        // Check if a pattern is a part of parameter expressions.
         // 9.2.10.28: The closure created by this expression should not have visibility of
         // declarations in the function body. If the binding is not a `param`-kind,
         // then it must be defined inside the function body, thus it should be skipped
         if (
           previousPath &&
           previousPath.isPattern() &&
-          previousPath.isScope() &&
+          previousPath.parentPath.isFunction() &&
           binding.kind !== "param"
         ) {
           // do nothing

diff --git a/packages/babel-traverse/src/scope/lib/renamer.js b/packages/babel-traverse/src/scope/lib/renamer.js
@@ -9,9 +9,8 @@ const renameVisitor = {
     }
   },
 
-  "Pattern|Scope"(path, state) {
+  Scope(path, state) {
     if (
-      path.isScope() &&
       !path.scope.bindingIdentifierEquals(
         state.oldName,
         state.binding.identifier,

diff --git a/packages/babel-traverse/test/scope.js b/packages/babel-traverse/test/scope.js
@@ -73,8 +73,8 @@ describe("scope", () => {
       ).toBe("Identifier");
     });
 
-    describe("function paramater expression", function() {
-      it("should not has visibility of declarations inside function body", () => {
+    describe("function parameter expression", function() {
+      it("should not have visibility of declarations inside function body", () => {
         expect(
           getPath(
             `var a = "outside"; (function foo(b = a) { let a = "inside" })`,
@@ -83,7 +83,7 @@ describe("scope", () => {
             .scope.getBinding("a").path.node.init.value,
         ).toBe("outside");
       });
-      it("should has visibility on paramater bindings", () => {
+      it("should have visibility on parameter bindings", () => {
         expect(
           getPath(`var a = "outside"; (function foo(b = a, a = "inside") {})`)
             .get("body.1.expression.params.0")

diff --git a/packages/babel-types/src/validators/isScope.js b/packages/babel-types/src/validators/isScope.js
@@ -19,6 +19,8 @@ export default function isScope(node: Object, parent: Object): boolean {
     return false;
   }
 
+  // If a Pattern is an immediate descendent of a Function, it must be in the params.
+  // Hence we skipped the parentKey === "params" check
   if (isPattern(node) && isFunction(parent)) {
     return true;
   }