feat(no-redundant-notify): also catch unsubscribe (#113)

Adds `unsubscribe` to the logic that guards against sending redundant
notifications.

`unsubscribe` is already discouraged by `no-subject-unsubscribe`, but
the subscriber argument of `new Observable` could also be affected.
Also, `unsubscribe` technically isn't a "notification", but it's a rare
enough use case that it's probably not worth splitting into a separate
rule.

Resolves #107

Author: JasonWeinzierl

docs/rules/no-redundant-notify.md

@@ -6,7 +6,8 @@
 
 <!-- end auto-generated rule header -->
 
-This rule effects failures if an attempt is made to send a notification to an observer after a `complete` or `error` notification has already been sent.
+This rule effects failures if an attempt is made to send a notification to an observer after a `complete` or `error` notification has already been sent,
+or if `unsubscribe` has been called.
 
 Note that the rule _does not perform extensive analysis_. It uses a straightforward and limited approach to catch obviously redundant notifications.
 

src/rules/no-redundant-notify.ts

@@ -32,7 +32,7 @@ export const noRedundantNotifyRule = ruleCreator({
     const sourceCode = context.sourceCode;
     const { couldBeType } = getTypeServices(context);
     return {
-      'ExpressionStatement[expression.callee.property.name=/^(complete|error)$/] + ExpressionStatement[expression.callee.property.name=/^(next|complete|error)$/]':
+      'ExpressionStatement[expression.callee.property.name=/^(complete|error|unsubscribe)$/] + ExpressionStatement[expression.callee.property.name=/^(next|complete|error|unsubscribe)$/]':
         (node: es.ExpressionStatement) => {
           const parent = node.parent;
           if (!parent) {

tests/rules/no-redundant-notify.test.ts

@@ -21,6 +21,14 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, {
         observer.error(new Error("Kaboom!"));
       })
     `,
+    stripIndent`
+      // observable next + unsubscribe
+      import { Observable } from "rxjs";
+      const observable = new Observable<number>(observer => {
+        observer.next(42);
+        observer.unsubscribe();
+      })
+    `,
     stripIndent`
       // subject next + complete
       import { Subject } from "rxjs";
@@ -35,6 +43,13 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, {
       subject.next(42);
       subject.error(new Error("Kaboom!"));
     `,
+    stripIndent`
+      // subject next + unsubscribe
+      import { Subject } from "rxjs";
+      const subject = new Subject<number>();
+      subject.next(42);
+      subject.unsubscribe();
+    `,
     stripIndent`
       // different names with error
       import { Subject } from "rxjs";
@@ -51,6 +66,14 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, {
       a.complete();
       b.complete();
     `,
+    stripIndent`
+      // different names with unsubscribe
+      import { Subject } from "rxjs";
+      const a = new Subject<number>();
+      const b = new Subject<number>();
+      a.unsubscribe();
+      b.unsubscribe();
+    `,
     stripIndent`
       // non-observer
       import { Subject } from "rxjs";
@@ -139,6 +162,51 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, {
         });
       `,
     ),
+    fromFixture(
+      stripIndent`
+        // observable unsubscribe + next
+        import { Observable } from "rxjs";
+        const observable = new Observable<number>(observer => {
+          observer.unsubscribe();
+          observer.next(42);
+                   ~~~~ [forbidden]
+        })
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // observable unsubscribe + complete
+        import { Observable } from "rxjs";
+        const observable = new Observable<number>(observer => {
+          observer.unsubscribe();
+          observer.complete();
+                   ~~~~~~~~ [forbidden]
+        })
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // observable unsubscribe + error
+        import { Observable } from "rxjs";
+        const observable = new Observable<number>(observer => {
+          observer.unsubscribe();
+          observer.error(new Error("Kaboom!"));
+                   ~~~~~ [forbidden]
+        })
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // observable unsubscribe + unsubscribe
+        import { Observable } from "rxjs";
+        const observable = new Observable<number>(observer => {
+          observer.unsubscribe();
+          observer.unsubscribe();
+                   ~~~~~~~~~~~ [forbidden]
+        })
+      `,
+    ),
+
     fromFixture(
       stripIndent`
         // subject complete + next
@@ -199,5 +267,45 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, {
                 ~~~~~ [forbidden]
       `,
     ),
+    fromFixture(
+      stripIndent`
+        // subject unsubscribe + next
+        import { Subject } from "rxjs";
+        const subject = new Subject<number>();
+        subject.unsubscribe();
+        subject.next(42);
+                ~~~~ [forbidden]
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subject unsubscribe + complete
+        import { Subject } from "rxjs";
+        const subject = new Subject<number>();
+        subject.unsubscribe();
+        subject.complete();
+                ~~~~~~~~ [forbidden]
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subject unsubscribe + error
+        import { Subject } from "rxjs";
+        const subject = new Subject<number>();
+        subject.unsubscribe();
+        subject.error(new Error("Kaboom!"));
+                ~~~~~ [forbidden]
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subject unsubscribe + unsubscribe
+        import { Subject } from "rxjs";
+        const subject = new Subject<number>();
+        subject.unsubscribe();
+        subject.unsubscribe();
+                ~~~~~~~~~~~ [forbidden]
+      `,
+    ),
   ],
 });