Skip to content

Commit

Permalink
fix: account for trailing commas in nested calls
Browse files Browse the repository at this point in the history
Fixes #2413

When there was an implicit object as the sole argument to a nested function application that ended with a trailing comma, the normalize stage would fail to generate corect CoffeeScript. The normalize stage would insert the opening and closing parentheses, but also attempt to remove the trailing comma as it was "unnecessary". Normally this doesn't cause a problem because the comma is removed and then the closing parenthesis is added after it. However, because there are two nested function applications, we end up with these operations:

1. insert opening parenthesis for outer function application
2. remove comma after last argument of inner function appliication
3. insert opening parenthesis for inner function application
4. remove same comma again for outer function application, removing the newly-inserted parenthesis from step 3
5. insert closing parenthesis for outer function application

Thus, only 1 closing parenthesis is added and there is a mismatch causing a syntax error. The simplest fix seems to be to not remove the comma if it attached to the last node in the list (i.e. of arguments, in this case). This does change the behavior to preserve trailing commas in some cases where they were previously removed, but that's probably a good thing.
  • Loading branch information
eventualbuddha committed Mar 22, 2022
1 parent 51f9564 commit b3d9600
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/utils/normalizeListItem.ts
Expand Up @@ -19,7 +19,7 @@ export default function normalizeListItem(
// when combined with other normalize stage transformations. So just
// remove the redundant comma.
const lastToken = listItemPatcher.lastToken();
if (lastToken.type === SourceType.COMMA && !(listItemPatcher.node instanceof Elision)) {
if (lastToken.type === SourceType.COMMA && !(listItemPatcher.node instanceof Elision) && nextListItemPatcher) {
patcher.remove(lastToken.start, lastToken.end);
}
// CoffeeScript allows semicolon-separated lists, so just change them to
Expand Down
16 changes: 15 additions & 1 deletion test/object_test.ts
Expand Up @@ -663,7 +663,7 @@ describe('objects', () => {
`,
`
a({
b: c});
b: c,});
`
);
});
Expand Down Expand Up @@ -737,4 +737,18 @@ describe('objects', () => {
`
);
});

it('regression test #2413', () => {
check(
`
a ->
b
c: 1,
`,
`
a(() => b({
c: 1,}));
`
);
});
});

0 comments on commit b3d9600

Please sign in to comment.