From 30bb38c4bb0ee6c14b2f8d78850b349377402f22 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Tue, 1 Mar 2016 17:03:06 -0800 Subject: [PATCH] Update scope binding info after transforming block-scoped bindings When convert a const, let or any other block-bound binding to a var we forget to update the scope info. This confuses other transforms that may come after this as to which scope does the binding belongs to. This also uncovered an issue where duplicate block-scoped bindings were allowed to co-exist. --- .../src/index.js | 40 +++++++++++++++++-- .../test/fixtures/exec/scope-bindings.js | 39 ++++++++++++++++++ .../test/fixtures/general/switch/actual.js | 4 +- .../test/fixtures/general/switch/expected.js | 4 +- 4 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/exec/scope-bindings.js diff --git a/packages/babel-plugin-transform-es2015-block-scoping/src/index.js b/packages/babel-plugin-transform-es2015-block-scoping/src/index.js index 3ab7b74cfd..079a126478 100644 --- a/packages/babel-plugin-transform-es2015-block-scoping/src/index.js +++ b/packages/babel-plugin-transform-es2015-block-scoping/src/index.js @@ -16,7 +16,7 @@ export default function () { VariableDeclaration(path, file) { let { node, parent, scope } = path; if (!isBlockScoped(node)) return; - convertBlockScopedToVar(node, parent, scope); + convertBlockScopedToVar(path, parent, scope, true); if (node._tdzThis) { let nodes = [node]; @@ -72,7 +72,8 @@ function isBlockScoped(node) { return true; } -function convertBlockScopedToVar(node, parent, scope) { +function convertBlockScopedToVar(path, parent, scope, moveBindingsToParent = false) { + const { node } = path; // https://github.com/babel/babel/issues/255 if (!t.isFor(parent)) { for (let i = 0; i < node.declarations.length; i++) { @@ -83,6 +84,20 @@ function convertBlockScopedToVar(node, parent, scope) { node[t.BLOCK_SCOPED_SYMBOL] = true; node.kind = "var"; + + // Move bindings from current block scope to function scope. + if (moveBindingsToParent) { + const parentScope = scope.getFunctionParent(); + if (parentScope === scope) { + return; + } + + const ids = path.getBindingIdentifiers(); + for (let name in ids) { + scope.removeOwnBinding(name); + } + parentScope.registerBinding("var", path); + } } function isVar(node) { @@ -287,6 +302,7 @@ class BlockScoping { this.outsideLetReferences = Object.create(null); this.hasLetReferences = false; this.letReferences = Object.create(null); + this.letReferencesDeclars = []; this.body = []; if (loopPath) { @@ -330,6 +346,8 @@ class BlockScoping { let letRefs = this.letReferences; let scope = this.scope; + const parentScope = scope.getFunctionParent(); + // alright, so since we aren't wrapping this block in a closure // we have to check if any of our let variables collide with // those in upper scopes and then if they do, generate a uid @@ -352,6 +370,18 @@ class BlockScoping { uid: uid }; } + + // Remove binding from block scope so it's moved to function scope. + if (parentScope !== scope) { + scope.removeOwnBinding(ref.name); + } + } + + // Move bindings to parent scope. + if (parentScope !== scope) { + for (let declar of (this.letReferencesDeclars: Array)) { + parentScope.registerBinding("var", declar); + } } if (!hasRemaps) return; @@ -502,7 +532,11 @@ class BlockScoping { for (let i = 0; i < block.body.length; i++) { let declar = block.body[i]; if (t.isClassDeclaration(declar) || t.isFunctionDeclaration(declar) || isBlockScoped(declar)) { - if (isBlockScoped(declar)) convertBlockScopedToVar(declar, block, this.scope); + let declarPath = this.blockPath.get("body")[i]; + if (isBlockScoped(declar)) { + convertBlockScopedToVar(declarPath, block, this.scope); + this.letReferencesDeclars.push(declarPath); + } declarators = declarators.concat(declar.declarations || declar); } } diff --git a/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/exec/scope-bindings.js b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/exec/scope-bindings.js new file mode 100644 index 0000000000..31bb7936b9 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/exec/scope-bindings.js @@ -0,0 +1,39 @@ +var code = [ + 'var foo = 1;', + 'if (x) {', + ' const bar = 1;', + '}', +].join('\n'); + +var innerScope = true; +var res = transform(code, { + plugins: opts.plugins.concat([ + function (b) { + var t = b.types; + return { + visitor: { + Scope: { + exit: function(path) { + if (innerScope) { + assert(Object.keys(path.scope.bindings).length === 0, 'Inner scope should not have any bindings'); + innerScope = false; + return; + } + + assert(Object.keys(path.scope.bindings).length === 2, 'Outer scope subsume the inner-scope binding'); + } + } + } + } + } + ]), +}); + +var expected = [ + 'var foo = 1;', + 'if (x) {', + ' var bar = 1;', + '}', +].join('\n'); + +assert.equal(res.code, expected); diff --git a/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/switch/actual.js b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/switch/actual.js index ffb85d1812..32ec8488aa 100644 --- a/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/switch/actual.js +++ b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/switch/actual.js @@ -3,9 +3,9 @@ let b = false; switch (a) { case true: - let b = 2; + let c = 2; break; case false: - let c = 3; + let d = 3; break; } diff --git a/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/switch/expected.js b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/switch/expected.js index ec0210e749..6f3fd7fdfd 100644 --- a/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/switch/expected.js +++ b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/switch/expected.js @@ -3,9 +3,9 @@ var b = false; switch (a) { case true: - var b = 2; + var c = 2; break; case false: - var c = 3; + var d = 3; break; }