fix: add implicit function call parens in the normalize step

src/patchers/NodePatcher.js

@@ -656,7 +656,7 @@ export default class NodePatcher {
    * Determines whether this patcher's node spanned multiple lines.
    */
   isMultiline(): boolean {
-    return /[\r\n]/.test(this.getOriginalSource());
+    return !this.node.virtual && /[\r\n]/.test(this.getOriginalSource());
   }
 
   /**

src/stages/main/patchers/FunctionApplicationPatcher.js

@@ -1,6 +1,6 @@
 import NodePatcher from './../../../patchers/NodePatcher.js';
 import type { Editor, Node, ParseContext } from './../../../patchers/types.js';
-import { CALL_START, COMMA } from 'coffee-lex';
+import { COMMA } from 'coffee-lex';
 import { isSemanticToken } from '../../../utils/types.js';
 
 export default class FunctionApplicationPatcher extends NodePatcher {
@@ -18,29 +18,15 @@ export default class FunctionApplicationPatcher extends NodePatcher {
     this.args.forEach(arg => arg.setRequiresExpression());
   }
 
+  /**
+   * Note that we don't need to worry about implicit function applications,
+   * since the normalize stage would have already added parens.
+   */
   patchAsExpression() {
-    let implicitCall = this.isImplicitCall();
     let { args, outerEndTokenIndex } = this;
 
     this.fn.patch();
 
-    if (implicitCall && args.length === 0) {
-      this.insert(this.fn.outerEnd, '()');
-      return;
-    }
-
-    if (implicitCall) {
-      let firstArg = args[0];
-      let hasOneArg = args.length === 1;
-      let firstArgIsOnNextLine = !firstArg ? false :
-        /[\r\n]/.test(this.context.source.slice(this.fn.outerEnd, firstArg.outerStart));
-      if ((hasOneArg && firstArg.node.virtual) || firstArgIsOnNextLine) {
-        this.insert(this.fn.outerEnd, '(');
-      } else {
-        this.overwrite(this.fn.outerEnd, firstArg.outerStart, '(');
-      }
-    }
-
     args.forEach((arg, i) => {
       arg.patch();
       let isLast = i === args.length - 1;
@@ -60,14 +46,6 @@ export default class FunctionApplicationPatcher extends NodePatcher {
         this.insert(arg.outerEnd, ',');
       }
     });
-
-    if (implicitCall) {
-      this.insert(this.innerEnd, ')');
-    }
-  }
-
-  isImplicitCall() {
-    return !this.fn.hasSourceTokenAfter(CALL_START);
   }
 
   /**

src/stages/normalize/index.js

@@ -1,6 +1,7 @@
 import ConditionalPatcher from './patchers/ConditionalPatcher.js';
 import ForInPatcher from './patchers/ForInPatcher.js';
 import ForOfPatcher from './patchers/ForOfPatcher.js';
+import FunctionApplicationPatcher from './patchers/FunctionApplicationPatcher.js';
 import NodePatcher from '../../patchers/NodePatcher.js';
 import PassthroughPatcher from '../../patchers/PassthroughPatcher.js';
 import ProgramPatcher from './patchers/ProgramPatcher.js';
@@ -34,6 +35,10 @@ export default class NormalizeStage extends TransformCoffeeScriptStage {
       case 'ForOf':
         return ForOfPatcher;
 
+      case 'FunctionApplication':
+      case 'NewOp':
+        return FunctionApplicationPatcher;
+
       case 'While':
         return WhilePatcher;
 

src/stages/normalize/patchers/FunctionApplicationPatcher.js

@@ -0,0 +1,53 @@
+import NodePatcher from './../../../patchers/NodePatcher.js';
+import type { Editor, Node, ParseContext } from './../../../patchers/types.js';
+import { CALL_START } from 'coffee-lex';
+
+export default class FunctionApplicationPatcher extends NodePatcher {
+  fn: NodePatcher;
+  args: Array<NodePatcher>;
+
+  constructor(node: Node, context: ParseContext, editor: Editor, fn: NodePatcher, args: Array<NodePatcher>) {
+    super(node, context, editor);
+    this.fn = fn;
+    this.args = args;
+  }
+
+  patchAsExpression() {
+    let implicitCall = this.isImplicitCall();
+    let { args } = this;
+
+    this.fn.patch();
+
+    if (implicitCall && args.length === 0) {
+      this.insert(this.fn.outerEnd, '()');
+      return;
+    }
+
+    if (implicitCall) {
+      let firstArg = args[0];
+      let hasOneArg = args.length === 1;
+      let firstArgIsOnNextLine = !firstArg ? false :
+        /[\r\n]/.test(this.context.source.slice(this.fn.outerEnd, firstArg.outerStart));
+      if ((hasOneArg && firstArg.node.virtual) || firstArgIsOnNextLine) {
+        this.insert(this.fn.outerEnd, '(');
+      } else {
+        this.overwrite(this.fn.outerEnd, firstArg.outerStart, '(');
+      }
+    }
+
+    args.forEach(arg => arg.patch());
+
+    if (implicitCall) {
+      let lastArg = args[args.length - 1];
+      if (lastArg.isMultiline()) {
+        this.appendLineAfter(')');
+      } else {
+        this.insert(this.innerEnd, ')');
+      }
+    }
+  }
+
+  isImplicitCall() {
+    return !this.fn.hasSourceTokenAfter(CALL_START);
+  }
+}

test/conditional_test.js

@@ -266,6 +266,15 @@ describe('conditionals', () => {
   });
 
   it('works with nested POST-`if`', () => {
+    check(`
+      a(b) if c(d) unless e(f)
+    `, `
+      if (!e(f)) { if (c(d)) { a(b); } }
+    `);
+  });
+
+  // TODO: Enable once https://github.com/Rich-Harris/magic-string/pull/89 lands.
+  it.skip('works with nested POST-`if` with implicit calls', () => {
     check(`
       a b if c d unless e f
     `, `

test/for_test.js

@@ -547,4 +547,16 @@ describe('for loops', () => {
         }
       }
     `));
+
+  it('handles for loops over implicit function calls', () =>
+    check(`
+      for a in b c
+        d()
+    `, `
+      let iterable = b(c);
+      for (let i = 0; i < iterable.length; i++) {
+        let a = iterable[i];
+        d();
+      }
+    `));
 });

test/function_test.js

@@ -156,4 +156,24 @@ describe('functions', () => {
   it('keeps function with a single assignment as a parameter in braces', () => {
     check(`(args=false) =>`, `(args=false) => {};`);
   });
+
+  it('places the function end in the right place when ending in an implicit function call', () =>
+    check(`
+      A = {
+        b: ->
+          return c d,
+            e,
+              f
+      }
+      G
+    `, `
+      const A = {
+        b() {
+          return c(d,
+            e,
+              f);
+        }
+      };
+      G;
+    `));
 });

test/object_test.js

@@ -81,7 +81,8 @@ describe('objects', () => {
       a(b, {
         c: d,
         e: f
-      });
+      }
+      );
     `);
   });