diff --git a/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/issue-4363/actual.js b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/issue-4363/actual.js new file mode 100644 index 0000000000..5d1acc8ba8 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/issue-4363/actual.js @@ -0,0 +1,18 @@ +function WithoutCurlyBraces() { + if (true) + for (let k in kv) { + function foo() { return this } + function bar() { return foo.call(this) } + console.log(this, k) // => undefined + } +} + +function WithCurlyBraces() { + if (true) { + for (let k in kv) { + function foo() { return this } + function bar() { return foo.call(this) } + console.log(this, k) // => 777 + } + } +} diff --git a/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/issue-4363/expected.js b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/issue-4363/expected.js new file mode 100644 index 0000000000..030e0f7103 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-block-scoping/test/fixtures/general/issue-4363/expected.js @@ -0,0 +1,43 @@ +function WithoutCurlyBraces() { + var _this = this; + + if (true) { + var _loop = function (k) { + function foo() { + return this; + } + + function bar() { + return foo.call(this); + } + + console.log(_this, k); // => undefined + }; + + for (var k in kv) { + _loop(k); + } + } +} + +function WithCurlyBraces() { + var _this2 = this; + + if (true) { + var _loop2 = function (k) { + function foo() { + return this; + } + + function bar() { + return foo.call(this); + } + + console.log(_this2, k); // => 777 + }; + + for (var k in kv) { + _loop2(k); + } + } +} diff --git a/packages/babel-plugin-transform-es2015-classes/src/index.js b/packages/babel-plugin-transform-es2015-classes/src/index.js index 4b16a7e853..6bdac143f9 100644 --- a/packages/babel-plugin-transform-es2015-classes/src/index.js +++ b/packages/babel-plugin-transform-es2015-classes/src/index.js @@ -44,7 +44,10 @@ export default function({ types: t }) { if (node[VISITED]) return; const inferred = nameFunction(path); - if (inferred && inferred !== node) return path.replaceWith(inferred); + if (inferred && inferred !== node) { + path.replaceWith(inferred); + return; + } node[VISITED] = true; diff --git a/packages/babel-plugin-transform-es2015-for-of/src/index.js b/packages/babel-plugin-transform-es2015-for-of/src/index.js index a89e30b277..8490d952d8 100644 --- a/packages/babel-plugin-transform-es2015-for-of/src/index.js +++ b/packages/babel-plugin-transform-es2015-for-of/src/index.js @@ -109,7 +109,8 @@ export default function({ messages, template, types: t }) { right.isGenericType("Array") || t.isArrayTypeAnnotation(right.getTypeAnnotation()) ) { - return replaceWithArray(path); + replaceWithArray(path); + return; } let callback = spec; diff --git a/packages/babel-traverse/src/path/modification.js b/packages/babel-traverse/src/path/modification.js index a4a5c74767..790d7a6f97 100644 --- a/packages/babel-traverse/src/path/modification.js +++ b/packages/babel-traverse/src/path/modification.js @@ -24,20 +24,24 @@ export function insertBefore(nodes) { (this.parentPath.isForStatement() && this.key === "init") ) { if (this.node) nodes.push(this.node); - this.replaceExpressionWithStatements(nodes); + return this.replaceExpressionWithStatements(nodes); } else if (Array.isArray(this.container)) { return this._containerInsertBefore(nodes); } else if (this.isStatementOrBlock()) { - if (this.node) nodes.push(this.node); - this.replaceWith(t.blockStatement(nodes)); + const shouldInsertCurrentNode = + this.node && + (!this.isExpressionStatement() || this.node.expression != null); + + this.replaceWith( + t.blockStatement(shouldInsertCurrentNode ? [this.node] : []), + ); + return this.unshiftContainer("body", nodes); } else { throw new Error( "We don't know what to do with this node type. " + "We were previously a Statement but we can't fit in here?", ); } - - return [this]; } export function _containerInsert(from, nodes) { @@ -45,34 +49,14 @@ export function _containerInsert(from, nodes) { const paths = []; + this.container.splice(from, 0, ...nodes); for (let i = 0; i < nodes.length; i++) { const to = from + i; - const node = nodes[i]; - this.container.splice(to, 0, node); + const path = this.getSibling(`${to}`); + paths.push(path); - if (this.context) { - const path = this.context.create( - this.parent, - this.container, - to, - this.listKey, - ); - - // While this path may have a context, there is currently no guarantee that the context - // will be the active context, because `popContext` may leave a final context in place. - // We should remove this `if` and always push once #4145 has been resolved. - if (this.context.queue) path.pushContext(this.context); - paths.push(path); - } else { - paths.push( - NodePath.get({ - parentPath: this.parentPath, - parent: this.parent, - container: this.container, - listKey: this.listKey, - key: to, - }), - ); + if (this.context && this.context.queue) { + path.pushContext(this.context); } } @@ -124,26 +108,24 @@ export function insertAfter(nodes) { ); nodes.push(t.expressionStatement(temp)); } - this.replaceExpressionWithStatements(nodes); + return this.replaceExpressionWithStatements(nodes); } else if (Array.isArray(this.container)) { return this._containerInsertAfter(nodes); } else if (this.isStatementOrBlock()) { - // Unshift current node if it's not an empty expression - if ( + const shouldInsertCurrentNode = this.node && - (!this.isExpressionStatement() || this.node.expression != null) - ) { - nodes.unshift(this.node); - } - this.replaceWith(t.blockStatement(nodes)); + (!this.isExpressionStatement() || this.node.expression != null); + + this.replaceWith( + t.blockStatement(shouldInsertCurrentNode ? [this.node] : []), + ); + return this.pushContainer("body", nodes); } else { throw new Error( "We don't know what to do with this node type. " + "We were previously a Statement but we can't fit in here?", ); } - - return [this]; } /** diff --git a/packages/babel-traverse/src/path/replacement.js b/packages/babel-traverse/src/path/replacement.js index 390cf3e167..24d81fe3f6 100644 --- a/packages/babel-traverse/src/path/replacement.js +++ b/packages/babel-traverse/src/path/replacement.js @@ -50,13 +50,14 @@ export function replaceWithMultiple(nodes: Array) { t.inheritLeadingComments(nodes[0], this.node); t.inheritTrailingComments(nodes[nodes.length - 1], this.node); this.node = this.container[this.key] = null; - this.insertAfter(nodes); + const paths = this.insertAfter(nodes); if (this.node) { this.requeue(); } else { this.remove(); } + return paths; } /** @@ -115,7 +116,7 @@ export function replaceWith(replacement) { } if (this.node === replacement) { - return; + return [this]; } if (this.isProgram() && !t.isProgram(replacement)) { @@ -136,6 +137,8 @@ export function replaceWith(replacement) { ); } + let nodePath = ""; + if (this.isNodeType("Statement") && t.isExpression(replacement)) { if ( !this.canHaveVariableDeclarationOrExpression() && @@ -144,6 +147,7 @@ export function replaceWith(replacement) { ) { // replacing a statement with an expression so wrap it in an expression statement replacement = t.expressionStatement(replacement); + nodePath = "expression"; } } @@ -172,6 +176,8 @@ export function replaceWith(replacement) { // requeue for visiting this.requeue(); + + return [nodePath ? this.get(nodePath) : this]; } /** @@ -206,45 +212,45 @@ export function replaceExpressionWithStatements(nodes: Array) { const toSequenceExpression = t.toSequenceExpression(nodes, this.scope); if (toSequenceExpression) { - this.replaceWith(toSequenceExpression); - } else { - const container = t.arrowFunctionExpression([], t.blockStatement(nodes)); - - this.replaceWith(t.callExpression(container, [])); - this.traverse(hoistVariablesVisitor); - - // add implicit returns to all ending expression statements - const completionRecords: Array = this.get( - "callee", - ).getCompletionRecords(); - for (const path of completionRecords) { - if (!path.isExpressionStatement()) continue; - - const loop = path.findParent(path => path.isLoop()); - if (loop) { - let uid = loop.getData("expressionReplacementReturnUid"); - - if (!uid) { - const callee = this.get("callee"); - uid = callee.scope.generateDeclaredUidIdentifier("ret"); - callee.get("body").pushContainer("body", t.returnStatement(uid)); - loop.setData("expressionReplacementReturnUid", uid); - } else { - uid = t.identifier(uid.name); - } - - path - .get("expression") - .replaceWith(t.assignmentExpression("=", uid, path.node.expression)); - } else { - path.replaceWith(t.returnStatement(path.node.expression)); - } - } - - this.get("callee").arrowFunctionToExpression(); - - return this.node; + return this.replaceWith(toSequenceExpression)[0].get("expressions"); } + const container = t.arrowFunctionExpression([], t.blockStatement(nodes)); + + this.replaceWith(t.callExpression(container, [])); + this.traverse(hoistVariablesVisitor); + + // add implicit returns to all ending expression statements + const completionRecords: Array = this.get( + "callee", + ).getCompletionRecords(); + for (const path of completionRecords) { + if (!path.isExpressionStatement()) continue; + + const loop = path.findParent(path => path.isLoop()); + if (loop) { + let uid = loop.getData("expressionReplacementReturnUid"); + + if (!uid) { + const callee = this.get("callee"); + uid = callee.scope.generateDeclaredUidIdentifier("ret"); + callee.get("body").pushContainer("body", t.returnStatement(uid)); + loop.setData("expressionReplacementReturnUid", uid); + } else { + uid = t.identifier(uid.name); + } + + path + .get("expression") + .replaceWith(t.assignmentExpression("=", uid, path.node.expression)); + } else { + path.replaceWith(t.returnStatement(path.node.expression)); + } + } + + const callee = this.get("callee"); + callee.arrowFunctionToExpression(); + + return callee.get("body.body"); } export function replaceInline(nodes: Object | Array) { @@ -253,8 +259,9 @@ export function replaceInline(nodes: Object | Array) { if (Array.isArray(nodes)) { if (Array.isArray(this.container)) { nodes = this._verifyNodeList(nodes); - this._containerInsertAfter(nodes); - return this.remove(); + const paths = this._containerInsertAfter(nodes); + this.remove(); + return paths; } else { return this.replaceWithMultiple(nodes); } diff --git a/packages/babel-traverse/test/modification.js b/packages/babel-traverse/test/modification.js index e3fa8a9f9c..5542badb27 100644 --- a/packages/babel-traverse/test/modification.js +++ b/packages/babel-traverse/test/modification.js @@ -9,7 +9,7 @@ function getPath(code) { let path; traverse(ast, { Program: function(_path) { - path = _path; + path = _path.get("body.0"); _path.stop(); }, }); @@ -18,22 +18,21 @@ function getPath(code) { } function generateCode(path) { - return generate(path.node).code; + return generate(path.parentPath.node).code; } describe("modification", function() { describe("pushContainer", function() { it("pushes identifier into params", function() { const rootPath = getPath("function test(a) {}"); - const path = rootPath.get("body.0"); - path.pushContainer("params", t.identifier("b")); + rootPath.pushContainer("params", t.identifier("b")); assert.equal(generateCode(rootPath), "function test(a, b) {}"); }); it("pushes identifier into block", function() { const rootPath = getPath("function test(a) {}"); - const path = rootPath.get("body.0.body"); + const path = rootPath.get("body"); path.pushContainer("body", t.expressionStatement(t.identifier("b"))); assert.equal(generateCode(rootPath), "function test(a) {\n b;\n}"); @@ -42,18 +41,121 @@ describe("modification", function() { describe("unshiftContainer", function() { it("unshifts identifier into params", function() { const rootPath = getPath("function test(a) {}"); - const path = rootPath.get("body.0"); - path.unshiftContainer("params", t.identifier("b")); + rootPath.unshiftContainer("params", t.identifier("b")); assert.equal(generateCode(rootPath), "function test(b, a) {}"); }); it("unshifts identifier into block", function() { const rootPath = getPath("function test(a) {}"); - const path = rootPath.get("body.0.body"); + const path = rootPath.get("body"); path.unshiftContainer("body", t.expressionStatement(t.identifier("b"))); assert.equal(generateCode(rootPath), "function test(a) {\n b;\n}"); }); }); + + describe("insertBefore", function() { + it("returns inserted path with BlockStatement", function() { + const rootPath = getPath("if (x) { y; }"); + const path = rootPath.get("consequent.body.0"); + const result = path.insertBefore(t.identifier("b")); + + assert.equal(Array.isArray(result), true); + assert.equal(result.length, 1); + assert.deepEqual(result[0].node, t.identifier("b")); + assert.equal(generateCode(rootPath), "if (x) {\n b\n y;\n}"); + }); + + it("returns inserted path without BlockStatement", function() { + const rootPath = getPath("if (x) y;"); + const path = rootPath.get("consequent"); + const result = path.insertBefore(t.identifier("b")); + + assert.equal(Array.isArray(result), true); + assert.equal(result.length, 1); + assert.deepEqual(result[0].node, t.identifier("b")); + assert.equal(generateCode(rootPath), "if (x) {\n b\n y;\n}"); + }); + + it("returns inserted path without BlockStatement without ExpressionStatement", function() { + const rootPath = getPath("if (x) for (var i = 0; i < 0; i++) {}"); + const path = rootPath.get("consequent"); + const result = path.insertBefore(t.identifier("b")); + + assert.equal(Array.isArray(result), true); + assert.equal(result.length, 1); + assert.deepEqual(result[result.length - 1].node, t.identifier("b")); + assert.equal( + generateCode(rootPath), + "if (x) {\n b\n\n for (var i = 0; i < 0; i++) {}\n}", + ); + }); + + it("returns inserted path with BlockStatement without ExpressionStatement", function() { + const rootPath = getPath("if (x) { for (var i = 0; i < 0; i++) {} }"); + const path = rootPath.get("consequent.body.0"); + const result = path.insertBefore(t.identifier("b")); + + assert.equal(Array.isArray(result), true); + assert.equal(result.length, 1); + assert.deepEqual(result[result.length - 1].node, t.identifier("b")); + assert.equal( + generateCode(rootPath), + "if (x) {\n b\n\n for (var i = 0; i < 0; i++) {}\n}", + ); + }); + }); + + describe("insertAfter", function() { + it("returns inserted path with BlockStatement with ExpressionStatement", function() { + const rootPath = getPath("if (x) { y; }"); + const path = rootPath.get("consequent.body.0"); + const result = path.insertAfter(t.identifier("b")); + + assert.equal(Array.isArray(result), true); + assert.equal(result.length, 1); + assert.deepEqual(result[result.length - 1].node, t.identifier("b")); + assert.equal(generateCode(rootPath), "if (x) {\n y;\n b\n}"); + }); + + it("returns inserted path without BlockStatement with ExpressionStatement", function() { + const rootPath = getPath("if (x) y;"); + const path = rootPath.get("consequent"); + const result = path.insertAfter(t.identifier("b")); + + assert.equal(Array.isArray(result), true); + assert.equal(result.length, 1); + assert.deepEqual(result[result.length - 1].node, t.identifier("b")); + assert.equal(generateCode(rootPath), "if (x) {\n y;\n b\n}"); + }); + + it("returns inserted path without BlockStatement without ExpressionStatement", function() { + const rootPath = getPath("if (x) for (var i = 0; i < 0; i++) {}"); + const path = rootPath.get("consequent"); + const result = path.insertAfter(t.identifier("b")); + + assert.equal(Array.isArray(result), true); + assert.equal(result.length, 1); + assert.deepEqual(result[result.length - 1].node, t.identifier("b")); + assert.equal( + generateCode(rootPath), + "if (x) {\n for (var i = 0; i < 0; i++) {}\n\n b\n}", + ); + }); + + it("returns inserted path with BlockStatement without ExpressionStatement", function() { + const rootPath = getPath("if (x) { for (var i = 0; i < 0; i++) {} }"); + const path = rootPath.get("consequent.body.0"); + const result = path.insertAfter(t.identifier("b")); + + assert.equal(Array.isArray(result), true); + assert.equal(result.length, 1); + assert.deepEqual(result[result.length - 1].node, t.identifier("b")); + assert.equal( + generateCode(rootPath), + "if (x) {\n for (var i = 0; i < 0; i++) {}\n\n b\n}", + ); + }); + }); });