Skip to content

Commit

Permalink
fix: don't remove comments when removing trailing empty returns (#295)
Browse files Browse the repository at this point in the history
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  Rich-Harris/magic-string#89 applied.
  • Loading branch information
alangpierce authored and eventualbuddha committed Jul 10, 2016
1 parent 6cfbff4 commit a0cf8ba
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/stages/main/patchers/BlockPatcher.js
Expand Up @@ -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()) {
Expand Down
28 changes: 28 additions & 0 deletions test/return_test.js
Expand Up @@ -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() { });
`)
);
});

0 comments on commit a0cf8ba

Please sign in to comment.