diff --git a/codemods/babel-plugin-codemod-optional-catch-binding/test/fixtures/codemod-optional-catch-binding/try-catch-block-duplicate-variable-declaration/input.js b/codemods/babel-plugin-codemod-optional-catch-binding/test/fixtures/codemod-optional-catch-binding/try-catch-block-duplicate-variable-declaration/input.js deleted file mode 100644 index 520eab559f..0000000000 --- a/codemods/babel-plugin-codemod-optional-catch-binding/test/fixtures/codemod-optional-catch-binding/try-catch-block-duplicate-variable-declaration/input.js +++ /dev/null @@ -1,5 +0,0 @@ -try { - throw 0; -} catch (e) { - let e = new TypeError('Duplicate variable declaration; will throw an error.'); -} diff --git a/codemods/babel-plugin-codemod-optional-catch-binding/test/fixtures/codemod-optional-catch-binding/try-catch-block-duplicate-variable-declaration/options.json b/codemods/babel-plugin-codemod-optional-catch-binding/test/fixtures/codemod-optional-catch-binding/try-catch-block-duplicate-variable-declaration/options.json deleted file mode 100644 index ab01b7471a..0000000000 --- a/codemods/babel-plugin-codemod-optional-catch-binding/test/fixtures/codemod-optional-catch-binding/try-catch-block-duplicate-variable-declaration/options.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "plugins": ["../../../../lib"], - "throws": "Duplicate declaration \"e\"" -} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-classes/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-classes/input.js deleted file mode 100644 index 4d0266bd50..0000000000 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-classes/input.js +++ /dev/null @@ -1,5 +0,0 @@ -const MULTIPLIER = 5; - -class MULTIPLIER { - -} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-classes/options.json b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-classes/options.json deleted file mode 100644 index 8901cbf81b..0000000000 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-classes/options.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "throws": "Duplicate declaration \"MULTIPLIER\"" -} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-declaration/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-declaration/input.js deleted file mode 100644 index ccb0ca0d05..0000000000 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-declaration/input.js +++ /dev/null @@ -1,3 +0,0 @@ -const MULTIPLIER = 5; - -var MULTIPLIER = "overwrite"; diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-declaration/options.json b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-declaration/options.json deleted file mode 100644 index 8901cbf81b..0000000000 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-declaration/options.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "throws": "Duplicate declaration \"MULTIPLIER\"" -} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-functions/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-functions/input.js deleted file mode 100644 index 8b3451172f..0000000000 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-functions/input.js +++ /dev/null @@ -1,5 +0,0 @@ -const MULTIPLIER = 5; - -function MULTIPLIER() { - -} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-functions/options.json b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-functions/options.json deleted file mode 100644 index 8901cbf81b..0000000000 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-functions/options.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "throws": "Duplicate declaration \"MULTIPLIER\"" -} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-5979/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-5979/input.js deleted file mode 100644 index 301ed2e804..0000000000 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-5979/input.js +++ /dev/null @@ -1 +0,0 @@ -try {} catch (a) { let a } diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-5979/options.json b/packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-5979/options.json deleted file mode 100644 index ca26076ba7..0000000000 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/general/issue-5979/options.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "throws": "Duplicate declaration \"a\"" -} diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index ed81f09158..88383fa66c 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/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" || diff --git a/packages/babel-traverse/test/__snapshots__/scope.js.snap b/packages/babel-traverse/test/__snapshots__/scope.js.snap new file mode 100644 index 0000000000..631ca61d5a --- /dev/null +++ b/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\\""`; diff --git a/packages/babel-traverse/test/scope.js b/packages/babel-traverse/test/scope.js index c602c97b06..aa7c0dba27 100644 --- a/packages/babel-traverse/test/scope.js +++ b/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() { - describe("binding paths", function() { +function createNode(node) { + 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(); + } + }); + }*/ + } + }); + }); });