Repository: decaffeinate/decaffeinate
Pull request #2420 (2022-04-20 02:31:57)

fix: place this assignments after super under CS2
CoffeeScript 2 puts autogenerated `this` assignments after the call to the `super`-constructor.

For example, in this code:

```coffee
class A extends B
  constructor: (@a) ->
    super()
  f: =>
```

The assignments to `this.a` and `this.f` are generated just after `super()`.

This commit ensures that decaffeinate follows the same rule.

eventualbuddha: Sorry, @qnighy, I missed this when I was looking at #2141. Is there anything here that didn't get fixed by #2421?
qnighy: In the example above, `this.f` was fixed by #2141 and `this.a` not.

Patch:
diff --git a/src/stages/main/patchers/ConstructorPatcher.ts b/src/stages/main/patchers/ConstructorPatcher.ts
index efe4e0f0..aa474398 100644
--- a/src/stages/main/patchers/ConstructorPatcher.ts
+++ b/src/stages/main/patchers/ConstructorPatcher.ts
@@ -84,7 +84,7 @@ export default class ConstructorPatcher extends ObjectBodyMemberPatcher {
 
     // Any bindings would ideally go before the super call, so if there are any,
     // we'll need this before super.
-    if (this.getBindings().length > 0) {
+    if (this.getBindings().length > 0 && !this.options.useCS2) {
       return 'Cannot automatically convert a subclass that uses bound methods.';
     }
 
diff --git a/src/stages/normalize/patchers/ConstructorPatcher.ts b/src/stages/normalize/patchers/ConstructorPatcher.ts
index 0dceebcb..3efe357c 100644
--- a/src/stages/normalize/patchers/ConstructorPatcher.ts
+++ b/src/stages/normalize/patchers/ConstructorPatcher.ts
@@ -1,6 +1,7 @@
 import NodePatcher from '../../../patchers/NodePatcher';
 import PassthroughPatcher from '../../../patchers/PassthroughPatcher';
 import { PatcherContext } from '../../../patchers/types';
+import FunctionPatcher from './FunctionPatcher';
 
 export default class ConstructorPatcher extends PassthroughPatcher {
   assignee: NodePatcher;
@@ -10,5 +11,9 @@ export default class ConstructorPatcher extends PassthroughPatcher {
     super(patcherContext, assignee, expression);
     this.assignee = assignee;
     this.expression = expression;
+
+    if (expression instanceof FunctionPatcher) {
+      expression.enableThisAfterSuper();
+    }
   }
 }
diff --git a/src/stages/normalize/patchers/FunctionPatcher.ts b/src/stages/normalize/patchers/FunctionPatcher.ts
index 9f91cb3a..c458525f 100644
--- a/src/stages/normalize/patchers/FunctionPatcher.ts
+++ b/src/stages/normalize/patchers/FunctionPatcher.ts
@@ -2,6 +2,7 @@ import { SourceType } from 'coffee-lex';
 import { Constructor, Node } from 'decaffeinate-parser/dist/nodes';
 import { PatcherContext } from '../../../patchers/types';
 import canPatchAssigneeToJavaScript from '../../../utils/canPatchAssigneeToJavaScript';
+import containsSuperCall from '../../../utils/containsSuperCall';
 import getAssigneeBindings from '../../../utils/getAssigneeBindings';
 import normalizeListItem from '../../../utils/normalizeListItem';
 import notNull from '../../../utils/notNull';
@@ -17,6 +18,7 @@ import RestPatcher from './SpreadPatcher';
 export default class FunctionPatcher extends NodePatcher {
   parameters: Array<NodePatcher>;
   body: BlockPatcher | null;
+  _thisAfterSuper = false;
 
   constructor(patcherContext: PatcherContext, parameters: Array<NodePatcher>, body: BlockPatcher | null) {
     super(patcherContext);
@@ -36,10 +38,12 @@ export default class FunctionPatcher extends NodePatcher {
 
     const firstRestParamIndex = this.getFirstRestParamIndex();
     const assignments = [];
+    const thisAssignmentsAfterSuper = [];
     for (const [i, parameter] of this.parameters.entries()) {
       if (firstRestParamIndex === -1 || i < firstRestParamIndex) {
-        const { newAssignments, newBindings } = this.patchParameterAndGetAssignments(parameter);
+        const { newAssignments, newThisAssignments, newBindings } = this.patchParameterAndGetAssignments(parameter);
         assignments.push(...newAssignments);
+        thisAssignmentsAfterSuper.push(...newThisAssignments);
         neededExplicitBindings.push(...newBindings);
       } else {
         parameter.patch();
@@ -97,6 +101,9 @@ export default class FunctionPatcher extends NodePatcher {
 
     // If there were assignments from parameters insert them
     if (this.body) {
+      if (thisAssignmentsAfterSuper.length > 0) {
+        this.body.insertStatementsAtIndex(thisAssignmentsAfterSuper, this.getIndexOfSuperStatement() + 1);
+      }
       // before the actual body
       for (const assignment of assignments) {
         this.body.insertLineBefore(assignment);
@@ -121,6 +128,7 @@ export default class FunctionPatcher extends NodePatcher {
    */
   patchParameterAndGetAssignments(parameter: NodePatcher): {
     newAssignments: Array<string>;
+    newThisAssignments: Array<string>;
     newBindings: Array<string>;
   } {
     const thisAssignments: Array<string> = [];
@@ -165,10 +173,19 @@ export default class FunctionPatcher extends NodePatcher {
     delete this.addDefaultParamAssignmentAtScopeHeader;
     delete this.addThisAssignmentAtScopeHeader;
 
-    return {
-      newAssignments: [...defaultParamAssignments, ...thisAssignments],
-      newBindings,
-    };
+    if (this.options.useCS2 && this.thisAfterSuperEnabled() && this.getIndexOfSuperStatement() >= 0) {
+      return {
+        newAssignments: defaultParamAssignments,
+        newThisAssignments: thisAssignments,
+        newBindings,
+      };
+    } else {
+      return {
+        newAssignments: [...defaultParamAssignments, ...thisAssignments],
+        newThisAssignments: [],
+        newBindings,
+      };
+    }
   }
 
   /**
@@ -246,4 +263,33 @@ export default class FunctionPatcher extends NodePatcher {
     }
     return -1;
   }
+
+  getIndexOfSuperStatement(): number {
+    if (!this.body) {
+      return -1;
+    }
+    const statements = this.body.statements;
+    for (let i = 0; i < statements.length; i++) {
+      if (containsSuperCall(statements[i].node)) {
+        return i;
+      }
+    }
+    return -1;
+  }
+
+  /**
+   * Call before initialization to tell the patcher to place
+   * assignments to `this` after `super()`.
+   */
+  enableThisAfterSuper(): void {
+    this._thisAfterSuper = true;
+  }
+
+  /**
+   * Determines whether the patcher needs to place
+   * assignments to `this` after `super()`.
+   */
+  thisAfterSuperEnabled(): boolean {
+    return this._thisAfterSuper;
+  }
 }


Test patch:
diff --git a/test/class_test.ts b/test/class_test.ts
index a0729151..43282bd0 100644
--- a/test/class_test.ts
+++ b/test/class_test.ts
@@ -180,6 +180,45 @@ describe('classes', () => {
       );
     });
 
+    it('subclass constructor with function body', () => {
+      checkCS1(
+        `
+        class A extends B
+          constructor: (@a) ->
+            super()
+      `,
+        `
+        class A extends B {
+          constructor(a) {
+            this.a = a;
+            super();
+          }
+        }
+      `
+      );
+
+      checkCS2(
+        `
+        class A extends B
+          constructor: (@a) ->
+            super()
+      `,
+        `
+        class A extends B {
+          constructor(a) {
+            super();
+            this.a = a;
+          }
+        }
+      `,
+        {
+          options: {
+            disallowInvalidConstructors: true,
+          },
+        }
+      );
+    });
+
     it('method', () => {
       check(
         `
@@ -294,6 +333,39 @@ describe('classes', () => {
         'Cannot automatically convert a subclass that uses bound methods.',
         {
           disallowInvalidConstructors: true,
+          useCS2: false,
+        }
+      );
+    });
+
+    it('passes existing constructor with bound methods in a subclass in CS2', () => {
+      checkCS2(
+        `
+      class A extends B
+        a: =>
+          1
+
+        constructor: ->
+          super()
+          this.b = 2;
+    `,
+        `
+      class A extends B {
+        a() {
+          return 1;
+        }
+
+        constructor() {
+          super();
+          this.a = this.a.bind(this);
+          this.b = 2;
+        }
+      }
+    `,
+        {
+          options: {
+            disallowInvalidConstructors: true,
+          },
         }
       );
     });