From 3746273eda10af0deeee5c423d75f252f8963084 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Fri, 29 Sep 2017 19:00:10 -0400 Subject: [PATCH] Path#ensureBlock keeps path context (#6337) * Path#ensureBlock keeps path context This ensures that if you're inside an ArrowFunction with an expression body (say, you're on the BooleanLiteral in `() => true`), you don't suddenly lose your path context after inserting a variable. This is because of https://github.com/babel/babel/pull/6335/commits/82d8aded8ed452e2893e72175319ae78fb324349#diff-9e0668ad44535be897b934e7077ecea5R14. Basically, an innocent `Scope#push` caused my visitor to suddenly stop working. Now, we mutate the Path so it's still in the tree. * Tests --- .../fixtures/general/no-for-in/expected.js | 1 + .../src/index.js | 10 --- .../src/index.js | 4 +- .../expected.js | 1 + .../babel-traverse/src/path/conversion.js | 45 +++++++++++- packages/babel-traverse/src/scope/index.js | 2 +- packages/babel-traverse/test/conversion.js | 69 +++++++++++++++++++ 7 files changed, 118 insertions(+), 14 deletions(-) create mode 100644 packages/babel-traverse/test/conversion.js diff --git a/packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-for-in/expected.js b/packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-for-in/expected.js index c5f8712367..eb57557844 100644 --- a/packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-for-in/expected.js +++ b/packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-for-in/expected.js @@ -2,4 +2,5 @@ var MULTIPLIER = 5; for (MULTIPLIER in arr) { throw new Error("\"MULTIPLIER\" is read-only"); + ; } diff --git a/packages/babel-plugin-transform-class-properties/src/index.js b/packages/babel-plugin-transform-class-properties/src/index.js index 9bc21d287e..ff16a3c73b 100644 --- a/packages/babel-plugin-transform-class-properties/src/index.js +++ b/packages/babel-plugin-transform-class-properties/src/index.js @@ -193,16 +193,6 @@ export default function({ types: t }) { path.insertAfter(nodes); }, - ArrowFunctionExpression(path) { - const classExp = path.get("body"); - if (!classExp.isClassExpression()) return; - - const body = classExp.get("body"); - const members = body.get("body"); - if (members.some(member => member.isClassProperty())) { - path.ensureBlock(); - } - }, }, }; } 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 e0a6652749..d9465c403c 100644 --- a/packages/babel-plugin-transform-es2015-block-scoping/src/index.js +++ b/packages/babel-plugin-transform-es2015-block-scoping/src/index.js @@ -44,8 +44,8 @@ export default function() { }, Loop(path, file) { - const { node, parent, scope } = path; - t.ensureBlock(node); + const { parent, scope } = path; + path.ensureBlock(); const blockScoping = new BlockScoping( path, path.get("body"), diff --git a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js index 6da45a7523..29238be5a1 100644 --- a/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js +++ b/packages/babel-plugin-transform-es2015-parameters/test/fixtures/parameters/rest-member-expression-deoptimisation/expected.js @@ -160,6 +160,7 @@ function forOf() { try { for (var _iterator = this[Symbol.iterator](), _step; !(_iteratorNormalCompletion = (_step = _iterator.next()).done); _iteratorNormalCompletion = true) { rest[0] = _step.value; + ; } } catch (err) { _didIteratorError = true; diff --git a/packages/babel-traverse/src/path/conversion.js b/packages/babel-traverse/src/path/conversion.js index d145e3e840..76dd012b11 100644 --- a/packages/babel-traverse/src/path/conversion.js +++ b/packages/babel-traverse/src/path/conversion.js @@ -23,7 +23,50 @@ export function toComputedKey(): Object { } export function ensureBlock() { - return t.ensureBlock(this.node); + const body = this.get("body"); + const bodyNode = body.node; + + if (Array.isArray(body)) { + throw new Error("Can't convert array path to a block statement"); + } + if (!bodyNode) { + throw new Error("Can't convert node without a body"); + } + + if (body.isBlockStatement()) { + return bodyNode; + } + + const statements = []; + + let stringPath = "body"; + let key; + let listKey; + if (body.isStatement()) { + listKey = "body"; + key = 0; + statements.push(body.node); + } else { + stringPath += ".body.0"; + if (this.isFunction()) { + key = "argument"; + statements.push(t.returnStatement(body.node)); + } else { + key = "expression"; + statements.push(t.expressionStatement(body.node)); + } + } + + this.node.body = t.blockStatement(statements); + const parentPath = this.get(stringPath); + body.setup( + parentPath, + listKey ? parentPath.node[listKey] : parentPath.node, + listKey, + key, + ); + + return this.node; } /** diff --git a/packages/babel-traverse/src/scope/index.js b/packages/babel-traverse/src/scope/index.js index 113191d95c..43dbc11b84 100644 --- a/packages/babel-traverse/src/scope/index.js +++ b/packages/babel-traverse/src/scope/index.js @@ -799,7 +799,7 @@ export default class Scope { } if (path.isLoop() || path.isCatchClause() || path.isFunction()) { - t.ensureBlock(path.node); + path.ensureBlock(); path = path.get("body"); } diff --git a/packages/babel-traverse/test/conversion.js b/packages/babel-traverse/test/conversion.js new file mode 100644 index 0000000000..f5f383269a --- /dev/null +++ b/packages/babel-traverse/test/conversion.js @@ -0,0 +1,69 @@ +import traverse from "../lib"; +import assert from "assert"; +import { parse } from "babylon"; +import generate from "babel-generator"; +import * as t from "babel-types"; + +function getPath(code) { + const ast = parse(code); + let path; + traverse(ast, { + Program: function(_path) { + path = _path.get("body.0"); + _path.stop(); + }, + }); + + return path; +} + +function generateCode(path) { + return generate(path.parentPath.node).code; +} + +describe("conversion", function() { + describe("ensureBlock", function() { + it("throws converting node without body to block", function() { + const rootPath = getPath("true;"); + + assert.throws(() => { + rootPath.ensureBlock(); + }, /Can't convert node without a body/); + }); + + it("throws converting already block array", function() { + const rootPath = getPath("function test() { true; }").get("body"); + assert.throws(() => { + rootPath.ensureBlock(); + }, /Can't convert array path to a block statement/); + }); + + it("converts arrow function with expression body to block", function() { + const rootPath = getPath("() => true").get("expression"); + rootPath.ensureBlock(); + assert.equal(generateCode(rootPath), "() => {\n return true;\n};"); + }); + + it("preserves arrow function body's context", function() { + const rootPath = getPath("() => true").get("expression"); + const body = rootPath.get("body"); + rootPath.ensureBlock(); + body.replaceWith(t.booleanLiteral(false)); + assert.equal(generateCode(rootPath), "() => {\n return false;\n};"); + }); + + it("converts for loop with statement body to block", function() { + const rootPath = getPath("for (;;) true;"); + rootPath.ensureBlock(); + assert.equal(generateCode(rootPath), "for (;;) {\n true;\n}"); + }); + + it("preserves for loop body's context", function() { + const rootPath = getPath("for (;;) true;"); + const body = rootPath.get("body"); + rootPath.ensureBlock(); + body.replaceWith(t.booleanLiteral(false)); + assert.equal(generateCode(rootPath), "for (;;) {\n false;\n}"); + }); + }); +});