feat(no-subscribe-in-pipe)!: new rule to forbid calling subscribe within pipe operators (#59)

BREAKING CHANGE: new rule added to `recommended` config.

New rule to forbid calling `subscribe` within `pipe`.  This was previously caught by `no-nested-subscribe` until it was changed due to cartant#67.

This rule was proposed to the original repo and was re-proposed here by request: cartant#131 (comment)

Signed-off-by: Dane Vanderbilt <Danevandy99@users.noreply.github.com>
Co-authored-by: Jason Weinzierl <weinzierljason@gmail.com>

Author: Danevandy99

README.md

@@ -108,6 +108,7 @@ The package includes the following rules.
 | [no-subject-unsubscribe](docs/rules/no-subject-unsubscribe.md)         | Disallow calling the `unsubscribe` method of subjects.                                                    | ✅ 🔒 |    |    | 💭 |    |
 | [no-subject-value](docs/rules/no-subject-value.md)                     | Disallow accessing the `value` property of a `BehaviorSubject` instance.                                  |      |    |    | 💭 |    |
 | [no-subscribe-handlers](docs/rules/no-subscribe-handlers.md)           | Disallow passing handlers to `subscribe`.                                                                 |      |    |    | 💭 |    |
+| [no-subscribe-in-pipe](docs/rules/no-subscribe-in-pipe.md)             | Disallow calling of `subscribe` within any RxJS operator inside a `pipe`.                                 | ✅ 🔒 |    |    | 💭 |    |
 | [no-tap](docs/rules/no-tap.md)                                         | Disallow the `tap` operator.                                                                              |      |    |    |    | ❌  |
 | [no-topromise](docs/rules/no-topromise.md)                             | Disallow use of the `toPromise` method.                                                                   | ✅ 🔒 |    | 💡 | 💭 |    |
 | [no-unbound-methods](docs/rules/no-unbound-methods.md)                 | Disallow passing unbound methods.                                                                         | ✅ 🔒 |    |    | 💭 |    |

docs/rules/no-subscribe-in-pipe.md

@@ -0,0 +1,37 @@
+# Disallow calling of `subscribe` within any RxJS operator inside a `pipe` (`rxjs-x/no-subscribe-in-pipe`)
+
+💼 This rule is enabled in the following configs: ✅ `recommended`, 🔒 `strict`.
+
+💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting).
+
+<!-- end auto-generated rule header -->
+
+This rule effects failures if `subscribe` is called within any operator inside a `pipe` operation.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```ts
+import { of } from "rxjs";
+import { map } from "rxjs/operators";
+
+of(42, 54).pipe(
+  map(value => {
+    of(value).subscribe(console.log); // This will trigger the rule
+    return value * 2;
+  })
+).subscribe(result => console.log(result));
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+import { of } from "rxjs";
+import { map, tap } from "rxjs/operators";
+
+of(42, 54).pipe(
+  tap(value => console.log(value)),
+  map(value => value * 2)
+).subscribe(result => console.log(result));
+```

src/configs/recommended.ts

@@ -20,6 +20,7 @@ export const createRecommendedConfig = (
     'rxjs-x/no-redundant-notify': 'error',
     'rxjs-x/no-sharereplay': 'error',
     'rxjs-x/no-subject-unsubscribe': 'error',
+    'rxjs-x/no-subscribe-in-pipe': 'error',
     'rxjs-x/no-topromise': 'error',
     'rxjs-x/no-unbound-methods': 'error',
     'rxjs-x/no-unsafe-subject-next': 'error',

src/configs/strict.ts

@@ -29,6 +29,7 @@ export const createStrictConfig = (
     'rxjs-x/no-sharereplay': 'error',
     'rxjs-x/no-subclass': 'error',
     'rxjs-x/no-subject-unsubscribe': 'error',
+    'rxjs-x/no-subscribe-in-pipe': 'error',
     'rxjs-x/no-topromise': 'error',
     'rxjs-x/no-unbound-methods': 'error',
     'rxjs-x/no-unsafe-subject-next': 'error',

src/index.ts

@@ -35,6 +35,7 @@ import { noSubclassRule } from './rules/no-subclass';
 import { noSubjectUnsubscribeRule } from './rules/no-subject-unsubscribe';
 import { noSubjectValueRule } from './rules/no-subject-value';
 import { noSubscribeHandlersRule } from './rules/no-subscribe-handlers';
+import { noSubscribeInPipeRule } from './rules/no-subscribe-in-pipe';
 import { noTapRule } from './rules/no-tap';
 import { noTopromiseRule } from './rules/no-topromise';
 import { noUnboundMethodsRule } from './rules/no-unbound-methods';
@@ -83,6 +84,7 @@ const plugin = {
     'no-subject-unsubscribe': noSubjectUnsubscribeRule,
     'no-subject-value': noSubjectValueRule,
     'no-subscribe-handlers': noSubscribeHandlersRule,
+    'no-subscribe-in-pipe': noSubscribeInPipeRule,
     'no-tap': noTapRule,
     'no-topromise': noTopromiseRule,
     'no-unbound-methods': noUnboundMethodsRule,

src/rules/no-subscribe-in-pipe.ts

@@ -0,0 +1,63 @@
+import { AST_NODE_TYPES, TSESTree as es } from '@typescript-eslint/utils';
+import { getTypeServices } from '../etc';
+import { ruleCreator } from '../utils';
+
+export const noSubscribeInPipeRule = ruleCreator({
+  defaultOptions: [],
+  meta: {
+    docs: {
+      description:
+        'Disallow calling of `subscribe` within any RxJS operator inside a `pipe`.',
+      recommended: 'recommended',
+      requiresTypeChecking: true,
+    },
+    fixable: undefined,
+    hasSuggestions: false,
+    messages: {
+      forbidden: 'Subscribe calls within pipe operators are forbidden.',
+    },
+    schema: [],
+    type: 'problem',
+  },
+  name: 'no-subscribe-in-pipe',
+  create: (context) => {
+    const { couldBeObservable, couldBeType } = getTypeServices(context);
+
+    function isWithinPipe(node: es.Node): boolean {
+      let parent = node.parent;
+
+      while (parent) {
+        if (
+          parent.type === AST_NODE_TYPES.CallExpression
+          && parent.callee.type === AST_NODE_TYPES.MemberExpression
+          && parent.callee.property.type === AST_NODE_TYPES.Identifier
+          && parent.callee.property.name === 'pipe'
+        ) {
+          return true;
+        }
+        parent = parent.parent;
+      }
+      return false;
+    }
+
+    return {
+      'CallExpression > MemberExpression[property.name=\'subscribe\']': (
+        node: es.MemberExpression,
+      ) => {
+        if (
+          !couldBeObservable(node.object)
+          && !couldBeType(node.object, 'Subscribable')
+        ) {
+          return;
+        }
+
+        if (isWithinPipe(node)) {
+          context.report({
+            messageId: 'forbidden',
+            node: node.property,
+          });
+        }
+      },
+    };
+  },
+});

tests/rules/no-subscribe-in-pipe.test.ts

@@ -0,0 +1,206 @@
+import { stripIndent } from 'common-tags';
+import { noSubscribeInPipeRule } from '../../src/rules/no-subscribe-in-pipe';
+import { fromFixture } from '../etc';
+import { ruleTester } from '../rule-tester';
+
+ruleTester({ types: true }).run('no-subscribe-in-pipe', noSubscribeInPipeRule, {
+  valid: [
+    stripIndent`
+      // subscribe outside of pipe
+      import { of } from "rxjs";
+      of(47).subscribe(value => {
+        console.log(value);
+      });
+    `,
+    stripIndent`
+      // pipe without subscribe
+      import { of } from "rxjs";
+      import { map } from "rxjs/operators";
+      of(47).pipe(
+        map(x => x * 2)
+      ).subscribe(value => {
+        console.log(value);
+      });
+    `,
+    stripIndent`
+      // nested pipes without subscribe
+      import { of } from "rxjs";
+      import { map, mergeMap } from "rxjs/operators";
+      of(47).pipe(
+        mergeMap(x => of(x).pipe(
+          map(y => y * 2)
+        ))
+      ).subscribe(value => {
+        console.log(value);
+      });
+    `,
+    stripIndent`
+      // subscribe in a function outside pipe
+      import { of } from "rxjs";
+      import { map } from "rxjs/operators";
+      const logValue = (value) => of(value).subscribe(console.log);
+      of(47).pipe(
+        map(x => x * 2)
+      ).subscribe(logValue);
+    `,
+    stripIndent`
+      // subscribe method on a non-Observable object inside pipe
+      import { of } from "rxjs";
+      import { map } from "rxjs/operators";
+      const customObject = { subscribe: () => {} };
+      of(47).pipe(
+        map(x => {
+          customObject.subscribe();
+          return x * 2;
+        })
+      ).subscribe();
+    `,
+    stripIndent`
+      // subscribe as a variable name inside pipe
+      import { of } from "rxjs";
+      import { map } from "rxjs/operators";
+      of(47).pipe(
+        map(x => {
+          const subscribe = 5;
+          return x * subscribe;
+        })
+      ).subscribe();
+    `,
+    stripIndent`
+      // subscribe in a comment inside pipe
+      import { of } from "rxjs";
+      import { map } from "rxjs/operators";
+      of(47).pipe(
+        map(x => {
+          // of(x).subscribe(console.log);
+          return x * 2;
+        })
+      ).subscribe();
+    `,
+    stripIndent`
+      // subscribe as a string inside pipe
+      import { of } from "rxjs";
+      import { map } from "rxjs/operators";
+      of(47).pipe(
+        map(x => {
+          console.log("subscribe");
+          return x * 2;
+        })
+      ).subscribe();
+    `,
+    stripIndent`
+      // subscribe inside of an Observable constructor
+      import { Observable, of } from "rxjs";
+
+      new Observable<number>(subscriber => {
+        of(42).subscribe(subscriber);
+      }).subscribe();
+      `,
+  ],
+  invalid: [
+    fromFixture(
+      stripIndent`
+        // subscribe inside map operator
+        import { of } from "rxjs";
+        import { map } from "rxjs/operators";
+        of(47).pipe(
+          map(x => {
+            of(x).subscribe(console.log);
+                  ~~~~~~~~~ [forbidden]
+            return x * 2;
+          })
+        ).subscribe();
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subscribe inside mergeMap operator
+        import { of } from "rxjs";
+        import { mergeMap } from "rxjs/operators";
+        of(47).pipe(
+          mergeMap(x => of(x).pipe(
+            map(y => {
+              of(y).subscribe(console.log);
+                    ~~~~~~~~~ [forbidden]
+              return y * 2;
+            })
+          ))
+        ).subscribe();
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subscribe inside tap operator
+        import { of } from "rxjs";
+        import { tap } from "rxjs/operators";
+        of(47).pipe(
+          tap(x => {
+            of(x).subscribe(console.log);
+                  ~~~~~~~~~ [forbidden]
+          })
+        ).subscribe();
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subscribe inside switchMap operator
+        import { of } from "rxjs";
+        import { switchMap } from "rxjs/operators";
+        of(47).pipe(
+          switchMap(x => {
+            of(x).subscribe(console.log);
+                  ~~~~~~~~~ [forbidden]
+            return of(x * 2);
+          })
+        ).subscribe();
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subscribe inside nested pipes
+        import { of } from "rxjs";
+        import { map, mergeMap } from "rxjs/operators";
+        of(47).pipe(
+          mergeMap(x => of(x).pipe(
+            map(y => {
+              of(y).subscribe(console.log);
+                    ~~~~~~~~~ [forbidden]
+              return y * 2;
+            })
+          ))
+        ).subscribe();
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subscribe inside a deeply nested function in pipe
+        import { of } from "rxjs";
+        import { map } from "rxjs/operators";
+        of(47).pipe(
+          map(x => {
+            const nestedFunc = () => {
+              const deeplyNested = () => {
+                of(x).subscribe(console.log);
+                      ~~~~~~~~~ [forbidden]
+              };
+              deeplyNested();
+            };
+            nestedFunc();
+            return x * 2;
+          })
+        ).subscribe();
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // subscribe in a ternary operator in pipe
+        import { of } from "rxjs";
+        import { map } from "rxjs/operators";
+        of(47).pipe(
+          map(x => x > 50 ? x : of(x).subscribe(console.log))
+                                      ~~~~~~~~~ [forbidden]
+        ).subscribe();
+      `,
+    ),
+  ],
+});