Allow super.ngOnDestroy() calls to base class

Author: Samuel-Therrien-Beslogic

source/rules/prefer-takeuntil.ts

@@ -26,6 +26,9 @@ const messages = {
 } as const;
 type MessageIds = keyof typeof messages;
 
+const ngOnDestroyMethodSelector =
+  "MethodDefinition[key.name='ngOnDestroy'][kind='method']";
+
 const defaultOptions: readonly {
   alias?: string[];
   checkComplete?: boolean;
@@ -92,6 +95,7 @@ const rule = ruleCreator({
       nextCallExpressions: es.CallExpression[];
       ngOnDestroyDefinition?: es.MethodDefinition;
       extendsSuperClassDeclaration?: es.ClassDeclaration;
+      superNgOnDestroyCallExpression?: es.CallExpression;
       subscribeCallExpressions: es.CallExpression[];
       subscribeCallExpressionsToNames: Map<es.CallExpression, Set<string>>;
     };
@@ -123,6 +127,7 @@ const rule = ruleCreator({
         nextCallExpressions,
         ngOnDestroyDefinition,
         extendsSuperClassDeclaration,
+        superNgOnDestroyCallExpression,
         subscribeCallExpressionsToNames,
       } = entry;
       if (subscribeCallExpressionsToNames.size === 0) {
@@ -163,26 +168,39 @@ const rule = ruleCreator({
         };
         namesToChecks.set(name, check);
 
-        if (!checkSubjectProperty(name, entry)) {
-          check.descriptors.push({
-            data: { name },
-            messageId: "notDeclared",
-            node: classDeclaration.id ?? classDeclaration,
-          });
-        }
-        if (!checkSubjectCall(name, nextCallExpressions)) {
-          check.descriptors.push({
-            data: { method: "next", name },
-            messageId: "notCalled",
-            node: ngOnDestroyDefinition.key,
-          });
-        }
-        if (checkComplete && !checkSubjectCall(name, completeCallExpressions)) {
-          check.descriptors.push({
-            data: { method: "complete", name },
-            messageId: "notCalled",
-            node: ngOnDestroyDefinition.key,
-          });
+        if (extendsSuperClassDeclaration) {
+          if (!superNgOnDestroyCallExpression) {
+            check.descriptors.push({
+              data: { method: "ngOnDestroy", name: "super" },
+              messageId: "notCalled",
+              node: ngOnDestroyDefinition.key,
+            });
+          }
+        } else {
+          if (!checkSubjectProperty(name, entry)) {
+            check.descriptors.push({
+              data: { name },
+              messageId: "notDeclared",
+              node: classDeclaration.id ?? classDeclaration,
+            });
+          }
+          if (!checkSubjectCall(name, nextCallExpressions)) {
+            check.descriptors.push({
+              data: { method: "next", name },
+              messageId: "notCalled",
+              node: ngOnDestroyDefinition.key,
+            });
+          }
+          if (
+            checkComplete &&
+            !checkSubjectCall(name, completeCallExpressions)
+          ) {
+            check.descriptors.push({
+              data: { method: "complete", name },
+              messageId: "notCalled",
+              node: ngOnDestroyDefinition.key,
+            });
+          }
         }
       });
 
@@ -367,23 +385,28 @@ const rule = ruleCreator({
           entry.propertyDefinitions.push(node);
         }
       },
-      "MethodDefinition[key.name='ngOnDestroy'][kind='method']": (
-        node: es.MethodDefinition
-      ) => {
+      [ngOnDestroyMethodSelector]: (node: es.MethodDefinition) => {
         const entry = getEntry();
         if (entry && entry.hasDecorator) {
           entry.ngOnDestroyDefinition = node;
         }
       },
       ...extendsSuperClassDeclaration,
-      "MethodDefinition[key.name='ngOnDestroy'][kind='method'] CallExpression[callee.property.name='next']":
+      [`${ngOnDestroyMethodSelector} CallExpression[callee.object.type='Super'][callee.property.name='ngOnDestroy']`]:
+        (node: es.CallExpression) => {
+          const entry = getEntry();
+          if (entry && entry.hasDecorator) {
+            entry.superNgOnDestroyCallExpression = node;
+          }
+        },
+      [`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='next']`]:
         (node: es.CallExpression) => {
           const entry = getEntry();
           if (entry && entry.hasDecorator) {
             entry.nextCallExpressions.push(node);
           }
         },
-      "MethodDefinition[key.name='ngOnDestroy'][kind='method'] CallExpression[callee.property.name='complete']":
+      [`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='complete']`]:
         (node: es.CallExpression) => {
           const entry = getEntry();
           if (entry && entry.hasDecorator) {

tests/rules/prefer-composition.ts

@@ -100,20 +100,21 @@ ruleTester({ types: true }).run("prefer-composition", rule, {
     },
     {
       code: stripIndent`
+        // extends superClass
         // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1
-        import { Component } from "@angular/core";
-        import { of } from "rxjs";
-        import { switchMap, take } from "rxjs/operators";
+        import { Component, Directive, OnDestroy } from "@angular/core";
+        import { of, Subject } from "rxjs";
+        import { switchMap, takeUntil } from "rxjs/operators";
 
         const o = of("o");
 
         @Directive()
         abstract class BaseComponent implements OnDestroy {
           private readonly destroySubject = new Subject<void>();
-          protected readonly destroy = this.destroy.asObservable();
+          protected readonly destroy = this.destroySubject.asObservable();
           ngOnDestroy() {
-            this.destroy.next();
-            this.destroy.complete();
+            this.destroySubject.next();
+            this.destroySubject.complete();
           }
         }
 

tests/rules/prefer-takeuntil.ts

@@ -371,21 +371,22 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, {
     },
     {
       code: stripIndent`
+        // extends superClass
         // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1
-        import { Component } from "@angular/core";
-        import { of } from "rxjs";
-        import { switchMap, take } from "rxjs/operators";
+        import { Component, Directive, OnDestroy } from "@angular/core";
+        import { of, Subject } from "rxjs";
+        import { switchMap, takeUntil } from "rxjs/operators";
 
         const o = of("o");
 
         @Directive()
         abstract class BaseComponent implements OnDestroy {
           private readonly destroySubject = new Subject<void>();
-          protected readonly destroy = this.destroy.asObservable();
+          protected readonly destroy = this.destroySubject.asObservable();
 
           ngOnDestroy() {
-            this.destroy.next();
-            this.destroy.complete();
+            this.destroySubject.next();
+            this.destroySubject.complete();
           }
         }
 
@@ -404,6 +405,51 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, {
       options: [
         {
           superClass: ["BaseComponent"],
+          checkComplete: true,
+        },
+      ],
+    },
+    {
+      code: stripIndent`
+        // extends superClass and implements OnDestroy
+        // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1
+        import { Component, Directive, OnDestroy } from "@angular/core";
+        import { of, Subject } from "rxjs";
+        import { switchMap, takeUntil } from "rxjs/operators";
+
+        const o = of("o");
+
+        @Directive()
+        abstract class BaseComponent implements OnDestroy {
+          private readonly destroySubject = new Subject<void>();
+          protected readonly destroy = this.destroySubject.asObservable();
+
+          ngOnDestroy() {
+            this.destroySubject.next();
+            this.destroySubject.complete();
+          }
+        }
+
+        @Component({
+          selector: "component-with-super-class-and-destroy"
+        })
+        class CorrectDestroyComponent extends BaseComponent implements OnDestroy {
+          someMethod() {
+            o.pipe(
+              switchMap(_ => o),
+              takeUntil(this.destroy)
+            ).subscribe();
+          }
+
+          override ngOnDestroy(): void {
+            super.ngOnDestroy()
+          }
+        }
+      `,
+      options: [
+        {
+          superClass: ["BaseComponent"],
+          checkComplete: true,
         },
       ],
     },
@@ -695,5 +741,89 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, {
         ],
       }
     ),
+    fromFixture(
+      stripIndent`
+        // extends superClass and implements OnDestroy, missing super.ngOnDestroy()
+        // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1
+        import { Component, Directive, OnDestroy } from "@angular/core";
+        import { of, Subject } from "rxjs";
+        import { switchMap, takeUntil } from "rxjs/operators";
+
+        const o = of("o");
+
+        @Directive()
+        abstract class BaseComponent implements OnDestroy {
+          private readonly destroySubject = new Subject<void>();
+          protected readonly destroy = this.destroySubject.asObservable();
+
+          ngOnDestroy() {
+            this.destroySubject.next();
+            this.destroySubject.complete();
+          }
+        }
+
+        @Component({
+          selector: "missing-super-call"
+        })
+        class MissingSuperCallComponent extends BaseComponent implements OnDestroy {
+          someMethod() {
+            o.pipe(
+              switchMap(_ => o),
+              takeUntil(this.destroy)
+            ).subscribe();
+          }
+
+          override ngOnDestroy(): void {
+                   ~~~~~~~~~~~ [notCalled { "method": "ngOnDestroy", "name": "super" }]
+          }
+        }
+      `,
+      {
+        options: [
+          {
+            superClass: ["BaseComponent"],
+            checkComplete: true,
+          },
+        ],
+      }
+    ),
+    fromFixture(
+      stripIndent`
+        // Calls super.ngOnDestroy() w/o extending base class
+        // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1
+        import { Component, OnDestroy } from "@angular/core";
+        import { of } from "rxjs";
+        import { switchMap, takeUntil } from "rxjs/operators";
+
+        const o = of("o");
+
+        @Component({
+          selector: "missing-base"
+        })
+        class MissingBaseComponent extends SomeClass implements OnDestroy {
+              ~~~~~~~~~~~~~~~~~~~~ [notDeclared { "name": "destroy" }]
+          someMethod() {
+            o.pipe(
+              switchMap(_ => o),
+              takeUntil(this.destroy)
+            ).subscribe();
+          }
+
+          override ngOnDestroy(): void {
+                   ~~~~~~~~~~~ [notCalled { "method": "next", "name": "destroy" }]
+                   ~~~~~~~~~~~ [notCalled { "method": "complete", "name": "destroy" }]
+            super.ngOnDestroy()
+          }
+        }
+      `,
+      {
+        options: [
+          {
+            superClass: ["BaseComponent"],
+            checkComplete: true,
+          },
+        ],
+      }
+    ),
   ],
 });