Use .call to avoid ASI issues when transforming imported names (#337)

Fixes #322
alangpierce · Nov 19, 2018 · 85f9c27 · 85f9c27
1 parent 57323b6
commit 85f9c27
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 16 deletions.
diff --git a/benchmark/sample/expression.ts b/benchmark/sample/expression.ts
@@ -633,7 +633,7 @@ function parseParenItem(): void {
 // argument to parseSubscripts to prevent it from consuming the
 // argument list.
 function parseNew(): void {
-  parseIdentifier();
+  expect(tt._new);
   if (eat(tt.dot)) {
     // new.target
     parseMetaProperty();

diff --git a/src/parser/traverser/expression.ts b/src/parser/traverser/expression.ts
@@ -652,7 +652,7 @@ function parseParenItem(): void {
 // argument to parseSubscripts to prevent it from consuming the
 // argument list.
 function parseNew(): void {
-  parseIdentifier();
+  expect(tt._new);
   if (eat(tt.dot)) {
     // new.target
     parseMetaProperty();

diff --git a/src/transformers/CJSImportTransformer.ts b/src/transformers/CJSImportTransformer.ts
@@ -177,20 +177,34 @@ export default class CJSImportTransformer extends Transformer {
     if (!replacement) {
       return false;
     }
-    // We need to change to (0, f) if this is a function call, so that it won't be interpreted as a
-    // method access. This can also happen in situations like (f)(), so a following close-paren
-    // should trigger this behavior as well if it eventually has an open-paren. In some cases, like
-    // export assignees, we must NOT turn the identifier into a normal expression, so we need to
-    // just to the regular replacement.
+    // Tolerate any number of closing parens while looking for an opening paren
+    // that indicates a function call.
     let possibleOpenParenIndex = this.tokens.currentIndex() + 1;
     while (
       possibleOpenParenIndex < this.tokens.tokens.length &&
       this.tokens.tokens[possibleOpenParenIndex].type === tt.parenR
     ) {
       possibleOpenParenIndex++;
     }
+    // Avoid treating imported functions as methods of their `exports` object
+    // by using `(0, f)` when the identifier is in a paren expression. Else
+    // use `Function.prototype.call` when the identifier is a guaranteed
+    // function call. When using `call`, pass undefined as the context.
     if (this.tokens.tokens[possibleOpenParenIndex].type === tt.parenL) {
-      this.tokens.replaceToken(`(0, ${replacement})`);
+      if (
+        this.tokens.tokenAtRelativeIndex(1).type === tt.parenL &&
+        this.tokens.tokenAtRelativeIndex(-1).type !== tt._new
+      ) {
+        this.tokens.replaceToken(`${replacement}.call(void 0, `);
+        // Remove the old paren.
+        this.tokens.removeToken();
+        // Balance out the new paren.
+        this.rootTransformer.processBalancedCode();
+        this.tokens.copyExpectedToken(tt.parenR);
+      } else {
+        // See here: http://2ality.com/2015/12/references.html
+        this.tokens.replaceToken(`(0, ${replacement})`);
+      }
     } else {
       this.tokens.replaceToken(replacement);
     }

diff --git a/test/imports-test.ts b/test/imports-test.ts
@@ -410,11 +410,55 @@ var _moduleName = require('moduleName');
       `"use strict";
       var _mymodule = require('my-module');
       
-      (0, _mymodule.bar)();
+      _mymodule.bar.call(void 0, );
     `,
     );
   });
 
+  it("never breaks weird call expressions", () => {
+    assertResult(
+      `
+      import a from 'a';
+      import b from 'b';
+      (a)();
+      ((a))();
+      a(b)();
+      (a + b)();
+      new (a)();
+      `,
+      `"use strict";${IMPORT_DEFAULT_PREFIX}
+      var _a = require('a'); var _a2 = _interopRequireDefault(_a);
+      var _b = require('b'); var _b2 = _interopRequireDefault(_b);
+      ((0, _a2.default))();
+      (((0, _a2.default)))();
+      _a2.default.call(void 0, (0, _b2.default))();
+      (_a2.default + (0, _b2.default))();
+      new ((0, _a2.default))();
+      `,
+    );
+  });
+
+  describe("ASI interop", () => {
+    it("prevents accidental invocation for imported functions", () => {
+      assertResult(
+        `
+        import {fun} from 'my-module'
+
+        let arr = []
+        fun()
+        fun(1, 2)
+        `,
+        `"use strict";
+        var _mymodule = require('my-module');
+
+        let arr = []
+        _mymodule.fun.call(void 0, )
+        _mymodule.fun.call(void 0, 1, 2)
+        `,
+      );
+    });
+  });
+
   it("uses wildcard name on default access when possible", () => {
     assertResult(
       `
@@ -475,7 +519,7 @@ var _moduleName = require('moduleName');
       
       class A {
         constructor() {
-          this.val = (0, _foo2.default)();
+          this.val = _foo2.default.call(void 0, );
         }
       }
     `,
@@ -729,13 +773,13 @@ module.exports = exports.default;
       
       const o = {
         f() {
-          (0, _foo.foo)(3);
+          _foo.foo.call(void 0, 3);
         }
       }
       
       class C {
         g() {
-          (0, _foo.foo)(4);
+          _foo.foo.call(void 0, 4);
         }
       }
     `,
@@ -788,7 +832,7 @@ module.exports = exports.default;
           return \`interpolated \${value}\`;
         }
         m2(id) {
-          (0, _things.foo)();
+          _things.foo.call(void 0, );
         }
       }
     `,
@@ -823,7 +867,7 @@ module.exports = exports.default;
       var _a = require('a');
       
       {
-        (0, _a.a)();
+        _a.a.call(void 0, );
       }
     `,
     );
@@ -845,7 +889,7 @@ module.exports = exports.default;
       
       switch (foo) {
         case 1: {
-          (0, _a.a)();
+          _a.a.call(void 0, );
         }
       }
     `,
@@ -868,7 +912,7 @@ module.exports = exports.default;
       
       switch (foo) {
         default: {
-          (0, _a.a)();
+          _a.a.call(void 0, );
         }
       }
     `,