Repository: decaffeinate/decaffeinate
Pull request #2143 (2020-11-09 17:34:43)

fix: bind after super call in CS2
CS2 changed the behavior of when classes would bind methods to be compatible with ES6 classes. They bind after the call to super, rather than before as in CS1. When targeting CS2, follow that same behavior.

Refs https://github.com/decaffeinate/decaffeinate/issues/2141.


Patch:
diff --git a/src/stages/main/patchers/ClassBlockPatcher.ts b/src/stages/main/patchers/ClassBlockPatcher.ts
index 4e0951fa..26c91ffd 100644
--- a/src/stages/main/patchers/ClassBlockPatcher.ts
+++ b/src/stages/main/patchers/ClassBlockPatcher.ts
@@ -29,12 +29,14 @@ export default class ClassBlockPatcher extends BlockPatcher {
       const boundMethods = this.boundInstanceMethods();
       if (boundMethods.length > 0) {
         const isSubclass = this.getClassPatcher().isSubclass();
-        if (isSubclass && !this.shouldAllowInvalidConstructors()) {
-          throw this.error(
-            getInvalidConstructorErrorMessage('Cannot automatically convert a subclass that uses bound methods.')
-          );
-        } else if (isSubclass) {
-          this.addSuggestion(FIX_INVALID_CONSTRUCTOR);
+        if (!this.options.useCS2) {
+          if (isSubclass && !this.shouldAllowInvalidConstructors()) {
+            throw this.error(
+              getInvalidConstructorErrorMessage('Cannot automatically convert a subclass that uses bound methods.')
+            );
+          } else if (isSubclass) {
+            this.addSuggestion(FIX_INVALID_CONSTRUCTOR);
+          }
         }
 
         const { source } = this.context;
@@ -47,11 +49,20 @@ export default class ClassBlockPatcher extends BlockPatcher {
         } else {
           constructor += `constructor() {\n`;
         }
-        boundMethods.forEach((method) => {
-          constructor += `${methodBodyIndent}${getBindingCodeForMethod(method)};\n`;
-        });
+        const bindingStatements = boundMethods.map(
+          (method) => `${methodBodyIndent}${getBindingCodeForMethod(method)};\n`
+        );
         if (isSubclass) {
-          constructor += `${methodBodyIndent}super(...args)\n`;
+          const superStatement = `${methodBodyIndent}super(...args)\n`;
+
+          if (this.options.useCS2) {
+            // The behavior is different in CS2. Binding happens after `super`.
+            constructor += superStatement + bindingStatements;
+          } else {
+            constructor += bindingStatements + superStatement;
+          }
+        } else {
+          constructor += bindingStatements;
         }
         constructor += `${methodIndent}}\n\n${methodIndent}`;
         this.prependLeft(insertionPoint, constructor);


Test patch:
diff --git a/test/class_test.ts b/test/class_test.ts
index dc878f36..cf669616 100644
--- a/test/class_test.ts
+++ b/test/class_test.ts
@@ -335,7 +335,7 @@ describe('classes', () => {
     });
 
     it('creates a constructor for bound methods with a `super` call in extended classes when requested', () => {
-      check(
+      checkCS1(
         `
       class A extends B
         a: =>
@@ -354,6 +354,31 @@ describe('classes', () => {
       }
     `
       );
+
+      // The behavior is different in CS2. Binding happens after `super`.
+      // CS2 also has a runtime check to verify inside bound methods, but
+      // we don't bother with that:
+      //
+      // https://coffeescript.org/#try:class%20A%20extends%20B%0A%20%20a%3A%20%3D%3E%0A%20%20%20%201
+      checkCS2(
+        `
+      class A extends B
+        a: =>
+          1
+    `,
+        `
+      class A extends B {
+        constructor(...args) {
+          super(...args);
+          this.a = this.a.bind(this);
+        }
+
+        a() {
+          return 1;
+        }
+      }
+    `
+      );
     });
 
     it('adds to an existing constructor for bound methods before a `super` call when requested', () => {
diff --git a/test/suggestions_test.ts b/test/suggestions_test.ts
index 0c8020ad..b127efaf 100644
--- a/test/suggestions_test.ts
+++ b/test/suggestions_test.ts
@@ -1,8 +1,8 @@
-import check, { checkCS1 } from './support/check';
+import check, { checkCS1, checkCS2 } from './support/check';
 
 describe('suggestions', () => {
   it('provides a suggestion for invalid constructors', () => {
-    check(
+    checkCS1(
       `
       class A extends B
         c: =>
@@ -32,6 +32,35 @@ describe('suggestions', () => {
         },
       }
     );
+    checkCS2(
+      `
+      class A extends B
+        c: =>
+          d
+    `,
+      `
+      /*
+       * decaffeinate suggestions:
+       * DS102: Remove unnecessary code created because of implicit returns
+       * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
+       */
+      class A extends B {
+        constructor(...args) {
+          super(...args);
+          this.c = this.c.bind(this);
+        }
+      
+        c() {
+          return d;
+        }
+      }
+    `,
+      {
+        options: {
+          disableSuggestionComment: false,
+        },
+      }
+    );
   });
 
   it('provides no suggestions for an ordinary file', () => {