fix: handle for...when loops ending in implicit function calls (#297)

The problem was that the code was inserting tokens out of order, which was
confusing magic-string. Here's what the old code did:
* Wrap the loop body in an `if` statement for the filter:
  * Prepend `if (c) {` to the start of the loop body.
  * Append `}` to the end of the loop body.
* Patch the loop body. Among other things, this appended `)` to the end because
  the loop body ended in an implicit function call.

So, from magic-string's perspective, the index at the end of the loop body had a
`}` appended, then a `)` appended, which is wrong. To fix, I reworked the
function to prepend the text, then patch, then append the curly-brace. I also had
to manually create the closing curly brace for the loop since I was appending
code after `patch` finished. All of this complexity only happens when there's a
filter, so I split it into its own case.

Note that another approach that might alleviate this type of problem is to
preprocess the coffeescript code (maybe in the normalize stage) to add parens
for implicit function calls. It seems like implicit function calls are a common
source of these types of problems, although I imagine there are others that
would still require the code to be careful about patching order.

Closes #296.

src/stages/main/patchers/ForInPatcher.js

@@ -58,27 +58,22 @@ export default class ForInPatcher extends ForPatcher {
   patchForLoopBody() {
     this.removeThenToken();
 
-    let intro = [];
-    let outro = [];
-
     let valueAssignment = `${this.getValueBinding()} = ${this.getTargetReference()}[${this.getIndexBinding()}]`;
     if (this.valAssignee.statementNeedsParens()) {
       valueAssignment = `(${valueAssignment})`;
     }
-    intro.push(valueAssignment);
 
-    let filter = this.filter;
-    let body = this.body;
-    if (filter) {
-      intro.push(`if (${this.getFilterCode()}) {`);
-      outro.push(`}`);
-    }
+    let { filter, body } = this;
 
-    body.insertStatementsAtIndex(intro, 0);
-    body.insertStatementsAtIndex(outro, body.statements.length);
-    body.patch({ leftBrace: false });
+    body.insertStatementsAtIndex([valueAssignment], 0);
     if (filter) {
+      body.insertStatementsAtIndex([`if (${this.getFilterCode()}) {`], 0);
+      body.patch({ leftBrace: false, rightBrace: false });
       body.indent();
+      body.appendLineAfter('}', -1);
+      body.appendLineAfter('}', -2);
+    } else {
+      body.patch({ leftBrace: false });
     }
   }
 

test/for_test.js

@@ -534,4 +534,17 @@ describe('for loops', () => {
       }
     `);
   });
+
+  it('handles for...when loops ending in implicit function calls', () =>
+    check(`
+      for a in b when c
+        d e
+    `, `
+      for (let i = 0; i < b.length; i++) {
+        let a = b[i];
+        if (c) {
+          d(e);
+        }
+      }
+    `));
 });