Migrate some duplicate binding tests to traverse (#9532)

* Migrate try-catch duplicate error

* Remove exception for functions and let in the same scope

* Migrate duplicate bindings tests to traverse

* Add test for subscope and let/const

* Add more test cases

codemods/babel-plugin-codemod-optional-catch-binding/test/fixtures/codemod-optional-catch-binding/try-catch-block-duplicate-variable-declaration/input.js

@@ -1,5 +0,0 @@
-try {
-  throw 0;
-} catch (e) {
-  let e = new TypeError('Duplicate variable declaration; will throw an error.');
-}

codemods/babel-plugin-codemod-optional-catch-binding/test/fixtures/codemod-optional-catch-binding/try-catch-block-duplicate-variable-declaration/options.json

@@ -1,4 +0,0 @@
-{
-  "plugins": ["../../../../lib"],
-  "throws": "Duplicate declaration \"e\""
-}

packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-classes/input.js

@@ -1,5 +0,0 @@
-const MULTIPLIER = 5;
-
-class MULTIPLIER {
-
-}

packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-classes/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "Duplicate declaration \"MULTIPLIER\""
-}

packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-declaration/input.js

@@ -1,3 +0,0 @@
-const MULTIPLIER = 5;
-
-var MULTIPLIER = "overwrite";

packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-declaration/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "Duplicate declaration \"MULTIPLIER\""
-}

packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-functions/input.js

@@ -1,5 +0,0 @@
-const MULTIPLIER = 5;
-
-function MULTIPLIER() {
-
-}

packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-functions/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "Duplicate declaration \"MULTIPLIER\""
-}

packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-5979/input.js

@@ -1 +0,0 @@
-try {} catch (a) { let a }

packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-5979/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "Duplicate declaration \"a\""
-}

packages/babel-traverse/src/scope/index.js

@@ -349,9 +349,6 @@ export default class Scope {
     // class expression
     if (local.kind === "local") return;
 
-    // ignore hoisted functions if there's also a local let
-    if (kind === "hoisted" && local.kind === "let") return;
-
     const duplicate =
       // don't allow duplicate bindings to exist alongside
       kind === "let" ||

packages/babel-traverse/test/__snapshots__/scope.js.snap

@@ -0,0 +1,25 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+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/function 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\\""`;
+
+exports[`scope duplicate bindings global const/function 1`] = `"Duplicate declaration \\"foo\\""`;
+
+exports[`scope duplicate bindings global const/let 1`] = `"Duplicate declaration \\"foo\\""`;
+
+exports[`scope duplicate bindings global const/var 1`] = `"Duplicate declaration \\"foo\\""`;
+
+exports[`scope duplicate bindings global let/class 1`] = `"Duplicate declaration \\"foo\\""`;
+
+exports[`scope duplicate bindings global let/function 1`] = `"Duplicate declaration \\"foo\\""`;
+
+exports[`scope duplicate bindings global let/let 1`] = `"Duplicate declaration \\"foo\\""`;
+
+exports[`scope duplicate bindings global let/var 1`] = `"Duplicate declaration \\"foo\\""`;

packages/babel-traverse/test/scope.js

@@ -1,8 +1,10 @@
-import traverse from "../lib";
+import traverse, { NodePath } from "../lib";
 import { parse } from "@babel/parser";
+import * as t from "@babel/types";
 
 function getPath(code, options) {
-  const ast = parse(code, options);
+  const ast =
+    typeof code === "string" ? parse(code, options) : createNode(code);
   let path;
   traverse(ast, {
     Program: function(_path) {
@@ -26,8 +28,27 @@ function getIdentifierPath(code) {
   return nodePath;
 }
 
-describe("scope", function() {
+function createNode(node) {
-  describe("binding paths", function() {
+  const ast = t.file(t.program(Array.isArray(node) ? node : [node]));
+
+  // This puts the path into the cache internally
+  // We afterwards traverse ast, as we need to start traversing
+  // at the File node and not the Program node
+  NodePath.get({
+    hub: {
+      buildError: (_, msg) => new Error(msg),
+    },
+    parentPath: null,
+    parent: ast,
+    container: ast,
+    key: "program",
+  }).setContext();
+
+  return ast;
+}
+
+describe("scope", () => {
+  describe("binding paths", () => {
     it("function declaration id", function() {
       expect(
         getPath("function foo() {}").scope.getBinding("foo").path.type,
@@ -250,4 +271,136 @@ describe("scope", function() {
       });
     });
   });
+
+  describe("duplicate bindings", () => {
+    /*
+     * These tests do not use the parser as the parser has
+     * its own scope tracking and we want to test the scope tracking
+     * of traverse here and see if it handles duplicate bindings correctly
+     */
+    describe("catch", () => {
+      // try {} catch (e) { let e; }
+      const createTryCatch = function(kind) {
+        return t.tryStatement(
+          t.blockStatement([]),
+          t.catchClause(
+            t.identifier("e"),
+            t.blockStatement([
+              t.variableDeclaration(kind, [
+                t.variableDeclarator(t.identifier("e"), t.stringLiteral("1")),
+              ]),
+            ]),
+          ),
+        );
+      };
+      ["let", "const"].forEach(name => {
+        it(name, () => {
+          const ast = createTryCatch(name);
+
+          expect(() => getPath(ast)).toThrowErrorMatchingSnapshot();
+        });
+      });
+
+      it("var", () => {
+        const ast = createTryCatch("var");
+
+        expect(() => getPath(ast)).not.toThrow();
+      });
+    });
+
+    ["let", "const"].forEach(name => {
+      it(`${name} and function in sub scope`, () => {
+        const ast = [
+          t.variableDeclaration(name, [
+            t.variableDeclarator(t.identifier("foo")),
+          ]),
+          t.blockStatement([
+            t.functionDeclaration(
+              t.identifier("foo"),
+              [],
+              t.blockStatement([]),
+            ),
+          ]),
+        ];
+
+        expect(() => getPath(ast)).not.toThrow();
+      });
+    });
+
+    describe("global", () => {
+      // node1, node2, success
+      // every line will run 2 tests `node1;node2;` and `node2;node1;`
+      // unless node1 === node2
+      const cases = [
+        ["const", "let", false],
+
+        ["const", "const", false],
+        ["const", "function", false],
+        ["const", "class", false],
+        ["const", "var", false],
+
+        ["let", "let", false],
+        ["let", "class", false],
+        ["let", "function", false],
+        ["let", "var", false],
+
+        //["var", "class", true],
+        ["var", "function", true],
+        ["var", "var", true],
+
+        ["class", "function", false],
+      ];
+
+      const createNode = function(kind) {
+        switch (kind) {
+          case "let":
+          case "const":
+          case "var":
+            return t.variableDeclaration(kind, [
+              t.variableDeclarator(t.identifier("foo")),
+            ]);
+          case "class":
+            return t.classDeclaration(
+              t.identifier("foo"),
+              null,
+              t.classBody([]),
+            );
+          case "function":
+            return t.functionDeclaration(
+              t.identifier("foo"),
+              [],
+              t.blockStatement([]),
+            );
+        }
+      };
+
+      const createAST = function(kind1, kind2) {
+        return [createNode(kind1), createNode(kind2)];
+      };
+
+      for (const [kind1, kind2, success] of cases) {
+        it(`${kind1}/${kind2}`, () => {
+          const ast = createAST(kind1, kind2);
+
+          if (success) {
+            expect(() => getPath(ast)).not.toThrow();
+          } else {
+            expect(() => getPath(ast)).toThrowErrorMatchingSnapshot();
+          }
+        });
+
+        /*if (kind1 !== kind2) {
+          it(`${kind2}/${kind1}`, () => {
+            const ast = createAST(kind2, kind1);
+
+            if (success) {
+              expect(() => getPath(ast)).not.toThrow();
+            } else {
+              expect(() => getPath(ast)).toThrowErrorMatchingSnapshot();
+            }
+          });
+        }*/
+      }
+    });
+  });
 });