fix: allow prefer-takeuntil and prefer-composition rules to support private JS properties

NachoVazquez · NachoVazquez · commit ffcdbf784245 · 2022-06-26T16:54:37.000-03:00
diff --git a/.husky/pre-commit b/.husky/pre-commit
diff --git a/source/rules/prefer-composition.ts b/source/rules/prefer-composition.ts
@@ -14,7 +14,7 @@ import {
   isMemberExpression,
   isVariableDeclarator,
 } from "eslint-etc";
-import { ruleCreator } from "../utils";
+import { ruleCreator, isPrivateIdentifier } from "../utils";
 
 const defaultOptions: readonly {
   checkDecorators?: string[];
@@ -145,7 +145,11 @@ const rule = ruleCreator({
       const { callee } = callExpression;
       if (isMemberExpression(callee)) {
         const { object } = callee;
-        if (isMemberExpression(object) && isIdentifier(object.property)) {
+        if (
+          isMemberExpression(object) &&
+          (isIdentifier(object.property) ||
+            isPrivateIdentifier(object.property))
+        ) {
           return object.property.name;
         }
         if (isIdentifier(object)) {
@@ -186,6 +190,7 @@ const rule = ruleCreator({
       // subscription or if it's assigned to a variable that is added to a
       // subscription.
       const { addCallExpressions, subscriptions } = entry;
+
       const parent = getParent(callExpression);
       if (!parent) {
         return false;
@@ -208,6 +213,7 @@ const rule = ruleCreator({
         subscriptions.add(name);
         return true;
       }
+
       if (isVariableDeclarator(parent) && isIdentifier(parent.id)) {
         return isVariableComposed(parent.id, entry);
       }
diff --git a/source/rules/prefer-takeuntil.ts b/source/rules/prefer-takeuntil.ts
@@ -15,7 +15,7 @@ import {
   isMemberExpression,
   isThisExpression,
 } from "eslint-etc";
-import { ruleCreator } from "../utils";
+import { ruleCreator, isPrivateIdentifier } from "../utils";
 
 const messages = {
   noDestroy: "`ngOnDestroy` is not implemented.",
@@ -207,7 +207,7 @@ const rule = ruleCreator({
           if (
             isMemberExpression(arg) &&
             isThisExpression(arg.object) &&
-            isIdentifier(arg.property)
+            (isIdentifier(arg.property) || isPrivateIdentifier(arg.property))
           ) {
             return { found: true, name: arg.property.name };
           } else if (arg && isIdentifier(arg)) {
@@ -233,7 +233,8 @@ const rule = ruleCreator({
           (isMemberExpression(callee) &&
             isMemberExpression(callee.object) &&
             isThisExpression(callee.object.object) &&
-            isIdentifier(callee.object.property) &&
+            (isIdentifier(callee.object.property) ||
+              isPrivateIdentifier(callee.object.property)) &&
             callee.object.property.name === name)
       );
       return Boolean(callExpression);
diff --git a/source/utils.ts b/source/utils.ts
@@ -4,8 +4,16 @@
  */
 
 import { ESLintUtils } from "@typescript-eslint/experimental-utils";
+import { TSESTree as es } from "@typescript-eslint/experimental-utils";
 
 export const ruleCreator = ESLintUtils.RuleCreator(
   (name) =>
     `https://github.com/cartant/eslint-plugin-rxjs-angular/tree/main/docs/rules/${name}.md`
 );
+
+// TODO: Remove when eslint-etc supports isPrivateIdentifier
+export function isPrivateIdentifier(
+  node: es.Node
+): node is es.PrivateIdentifier {
+  return node.type === "PrivateIdentifier";
+}
diff --git a/tests/rules/prefer-composition.ts b/tests/rules/prefer-composition.ts
@@ -32,6 +32,28 @@ ruleTester({ types: true }).run("prefer-composition", rule, {
         }
       `,
     },
+    {
+      code: stripIndent`
+        // composed component with private JavaScript property
+        import { Component, OnDestroy, OnInit } from "@angular/core";
+        import { of, Subscription } from "rxjs";
+
+        @Component({
+          selector: "composed-component",
+          template: "<span>{{ value }}</span>"
+        })
+        export class ComposedComponent implements OnInit, OnDestroy {
+          value: string;
+          #subscription = new Subscription();
+          ngOnInit() {
+            this.#subscription.add(of("foo").subscribe(value => this.value = value));
+          }
+          ngOnDestroy() {
+            this.#subscription.unsubscribe();
+          }
+        }
+      `,
+    },
     {
       code: stripIndent`
         // variable composed component
@@ -47,7 +69,7 @@ ruleTester({ types: true }).run("prefer-composition", rule, {
           private subscription = new Subscription();
           ngOnInit() {
             let subscription = of("foo").subscribe(value => this.value = value);
-            this.subscription.add(subscription);1
+            this.subscription.add(subscription);
             subscription = of("bar").subscribe(value => this.value = value);
             this.subscription.add(subscription);
           }
@@ -57,6 +79,31 @@ ruleTester({ types: true }).run("prefer-composition", rule, {
         }
       `,
     },
+    {
+      code: stripIndent`
+        // variable composed component with private JavaScript property
+        import { Component, OnDestroy, OnInit } from "@angular/core";
+        import { of, Subscription } from "rxjs";
+
+        @Component({
+          selector: "variable-composed-component",
+          template: "<span>{{ value }}</span>"
+        })
+        export class VariableComposedComponent implements OnInit, OnDestroy {
+          value: string;
+          #subscription = new Subscription();
+          ngOnInit() {
+            let subscription = of("foo").subscribe(value => this.value = value);
+            this.#subscription.add(subscription);
+            subscription = of("bar").subscribe(value => this.value = value);
+            this.#subscription.add(subscription);
+          }
+          ngOnDestroy() {
+            this.#subscription.unsubscribe();
+          }
+        }
+      `,
+    },
     {
       code: stripIndent`
         // destructured composed component
@@ -145,6 +192,28 @@ ruleTester({ types: true }).run("prefer-composition", rule, {
         }
       `
     ),
+    fromFixture(
+      stripIndent`
+        // not unsubscribed component with private JavaScript property
+        import { Component, OnDestroy, OnInit } from "@angular/core";
+        import { of, Subscription } from "rxjs";
+
+        @Component({
+          selector: "not-unsubscribed-component",
+          template: "<span>{{ value }}</span>"
+        })
+        export class NotUnsubscribedComponent implements OnInit, OnDestroy {
+          value: string;
+          #subscription = new Subscription();
+          ~~~~~~~~~~~~~ [notUnsubscribed]
+          ngOnInit() {
+            this.#subscription.add(of("foo").subscribe(value => this.value = value));
+          }
+          ngOnDestroy() {
+          }
+        }
+      `
+    ),
     fromFixture(
       stripIndent`
         // not destroyed component
@@ -165,6 +234,26 @@ ruleTester({ types: true }).run("prefer-composition", rule, {
         }
       `
     ),
+    fromFixture(
+      stripIndent`
+        // not destroyed component with Private JavaScript property
+        import { Component, OnDestroy, OnInit } from "@angular/core";
+        import { of, Subscription } from "rxjs";
+
+        @Component({
+          selector: "not-destroyed-component",
+          template: "<span>{{ value }}</span>"
+        })
+        export class NotDestroyedComponent implements OnInit {
+                     ~~~~~~~~~~~~~~~~~~~~~ [notImplemented]
+          value: string;
+          #subscription = new Subscription();
+          ngOnInit() {
+            this.#subscription.add(of("foo").subscribe(value => this.value = value));
+          }
+        }
+      `
+    ),
     fromFixture(
       stripIndent`
         // not declared
diff --git a/tests/rules/prefer-takeuntil.ts b/tests/rules/prefer-takeuntil.ts
