From 6cf041e7a40dc9238f8af95e400514a0d955cb14 Mon Sep 17 00:00:00 2001 From: Alan Pierce Date: Sat, 9 Jul 2016 17:29:25 -0700 Subject: [PATCH] fix: don't remove comments when removing trailing empty returns Before this commit, the comment after a removed return statement would be destroyed: http://decaffeinate-project.org/repl/#?evaluate=true&code=-%3E%0A%20%20a%20%20%23%20foo%0A%20%20return Now, the code is a little more careful by deleting up to the start of the line or the previous semicolon. Also, the code now exits early and doesn't patch the `return` node or insert a semicolon at the end, since that was causing a semicolon at the end of the comment. This also fixes a crash on this type of input that I ran into when using magic-string with https://github.com/Rich-Harris/magic-string/pull/89 applied. --- src/stages/main/patchers/BlockPatcher.js | 16 ++++++++++---- test/return_test.js | 28 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/stages/main/patchers/BlockPatcher.js b/src/stages/main/patchers/BlockPatcher.js index cf34b76fa..af577a930 100644 --- a/src/stages/main/patchers/BlockPatcher.js +++ b/src/stages/main/patchers/BlockPatcher.js @@ -37,14 +37,22 @@ export default class BlockPatcher extends NodePatcher { this.statements.forEach( (statement, i, statements) => { if (i === statements.length - 1 && this.parent instanceof FunctionPatcher) { - let previousStatement = statements[i - 1]; if (statement instanceof ReturnPatcher && !statement.expression) { + let removeStart; + if (statements.length > 1) { + let startOfLineIndex = this.context.sourceTokens.lastIndexOfTokenMatchingPredicate( + token => token.type === NEWLINE || token.type === SEMICOLON, + statement.outerStartTokenIndex + ); + removeStart = this.sourceTokenAtIndex(startOfLineIndex).start; + } else { + removeStart = statement.outerStart; + } this.remove( - previousStatement ? - previousStatement.outerEnd : - statement.outerStart, + removeStart, statement.outerEnd ); + return; } } if (statement.isSurroundedByParentheses()) { diff --git a/test/return_test.js b/test/return_test.js index e3b46bda4..d09fdbffc 100644 --- a/test/return_test.js +++ b/test/return_test.js @@ -27,4 +27,32 @@ describe('return', () => { () => true ? null : undefined; `) ); + + it('preserves comments when removing trailing empty returns', () => + check(` + -> + a # b + return + `, ` + (function() { + a; // b + }); + `) + ); + + it('correctly removes trailing empty returns on the same line as another statement', () => + check(` + -> a; return + `, ` + (function() { a; }); + `) + ); + + it('correctly removes trailing empty returns as the only function statement', () => + check(` + -> return + `, ` + (function() { }); + `) + ); });