From a540cbe8016b1f86d8b9106a40e892893e5dde1c Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Thu, 30 Apr 2015 05:06:56 -0700 Subject: [PATCH 1/2] Failing test with return not on the same line as it's expression Note that this is not a parenthesis issue but this was the easiest way to reproduce it. I ran into it when testing generators with `retainLines` and the generated `return` statement (replacing yeild) was printed on the line preceding the expression being yielded. --- .../edgecase/return-with-retainlines-option/actual.js | 5 +++++ .../edgecase/return-with-retainlines-option/expected.js | 2 ++ .../edgecase/return-with-retainlines-option/options.json | 3 +++ 3 files changed, 10 insertions(+) create mode 100644 test/core/fixtures/generation/edgecase/return-with-retainlines-option/actual.js create mode 100644 test/core/fixtures/generation/edgecase/return-with-retainlines-option/expected.js create mode 100644 test/core/fixtures/generation/edgecase/return-with-retainlines-option/options.json diff --git a/test/core/fixtures/generation/edgecase/return-with-retainlines-option/actual.js b/test/core/fixtures/generation/edgecase/return-with-retainlines-option/actual.js new file mode 100644 index 0000000000..66e5d50920 --- /dev/null +++ b/test/core/fixtures/generation/edgecase/return-with-retainlines-option/actual.js @@ -0,0 +1,5 @@ +function foo(l) { + return ( + l + ); +} diff --git a/test/core/fixtures/generation/edgecase/return-with-retainlines-option/expected.js b/test/core/fixtures/generation/edgecase/return-with-retainlines-option/expected.js new file mode 100644 index 0000000000..ccb44de809 --- /dev/null +++ b/test/core/fixtures/generation/edgecase/return-with-retainlines-option/expected.js @@ -0,0 +1,2 @@ +function foo(l) { + return l;} diff --git a/test/core/fixtures/generation/edgecase/return-with-retainlines-option/options.json b/test/core/fixtures/generation/edgecase/return-with-retainlines-option/options.json new file mode 100644 index 0000000000..97925bbcb6 --- /dev/null +++ b/test/core/fixtures/generation/edgecase/return-with-retainlines-option/options.json @@ -0,0 +1,3 @@ +{ + "retainLines": true +} From 4c2ae5dd1962d62d9e871934f9ed861f78da3d43 Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Thu, 30 Apr 2015 05:43:08 -0700 Subject: [PATCH 2/2] Parenthesize statement argument when on a different line --- src/babel/generation/index.js | 15 ++++++++++++--- .../return-with-retainlines-option/expected.js | 3 ++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/babel/generation/index.js b/src/babel/generation/index.js index ed784c4019..09d74c4fe0 100644 --- a/src/babel/generation/index.js +++ b/src/babel/generation/index.js @@ -147,13 +147,22 @@ class CodeGenerator { return print; } - catchUp(node) { + catchUp(node, parent) { // catch up to this nodes newline if we're behind if (node.loc && this.format.retainLines && this.buffer.buf) { + var needsParens = false; + if (parent && (this.position.line < node.loc.start.line) && + (t.isContinueStatement(parent) || t.isBreakStatement(parent) || + t.isReturnStatement(parent) || t.isThrowStatement(parent))) { + needsParens = true; + this._push("("); + } while (this.position.line < node.loc.start.line) { this._push("\n"); } + return needsParens; } + return false; } print(node, parent, opts = {}) { @@ -207,7 +216,7 @@ class CodeGenerator { this.printLeadingComments(node, parent); - this.catchUp(node); + var needsParensFromCatchup = this.catchUp(node, parent); newline(true); @@ -220,7 +229,7 @@ class CodeGenerator { this.newline(); this.dedent(); } - if (needsParens) this.push(")"); + if (needsParens || needsParensFromCatchup) this.push(")"); this.map.mark(node, "end"); if (opts.after) opts.after(); diff --git a/test/core/fixtures/generation/edgecase/return-with-retainlines-option/expected.js b/test/core/fixtures/generation/edgecase/return-with-retainlines-option/expected.js index ccb44de809..7b9861c5aa 100644 --- a/test/core/fixtures/generation/edgecase/return-with-retainlines-option/expected.js +++ b/test/core/fixtures/generation/edgecase/return-with-retainlines-option/expected.js @@ -1,2 +1,3 @@ function foo(l) { - return l;} + return ( + l);}