feat(no-sharereplay-before-takeuntil): new rule

docs/rules/no-sharereplay-before-takeuntil.md

@@ -0,0 +1,42 @@
+# Disallow using `shareReplay({ refCount: false })` before `takeUntil` (`rxjs-x/no-sharereplay-before-takeuntil`)
+
+💼 This rule is enabled in the 🔒 `strict` config.
+
+<!-- end auto-generated rule header -->
+
+This rule effects failures if the `shareReplay` operator is used without its reference counting behavior
+and placed in an observable composition before `takeUntil`.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```ts
+source.pipe(
+    shareReplay(),
+    takeUntil(notifier),
+);
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+source.pipe(
+    shareReplay({ refCount: true }),
+    takeUntil(notifier),
+);
+```
+
+## When Not To Use It
+
+If you are confident your project uses `shareReplay` and `takeUntil` correctly,
+then you may not need this rule.
+
+## Further reading
+
+- [Avoiding takeUntil leaks#An update](https://ncjamieson.com/avoiding-takeuntil-leaks/#an-update)
+
+## Resources
+
+- [Rule source](/src/rules/no-sharereplay-before-takeuntil.ts)
+- [Test source](/tests/rules/no-sharereplay-before-takeuntil.test.ts)

src/configs/strict.ts

@@ -26,6 +26,7 @@ export const createStrictConfig = (
     'rxjs-x/no-misused-observables': 'error',
     'rxjs-x/no-nested-subscribe': 'error',
     'rxjs-x/no-redundant-notify': 'error',
+    'rxjs-x/no-sharereplay-before-takeuntil': 'error',
     'rxjs-x/no-sharereplay': 'error',
     'rxjs-x/no-subclass': 'error',
     'rxjs-x/no-subject-unsubscribe': 'error',

src/index.ts

@@ -32,6 +32,7 @@ import { noMisusedObservablesRule } from './rules/no-misused-observables';
 import { noNestedSubscribeRule } from './rules/no-nested-subscribe';
 import { noRedundantNotifyRule } from './rules/no-redundant-notify';
 import { noSharereplayRule } from './rules/no-sharereplay';
+import { noSharereplayBeforeTakeuntilRule } from './rules/no-sharereplay-before-takeuntil';
 import { noSubclassRule } from './rules/no-subclass';
 import { noSubjectUnsubscribeRule } from './rules/no-subject-unsubscribe';
 import { noSubjectValueRule } from './rules/no-subject-value';
@@ -79,6 +80,7 @@ const allRules = {
   'no-nested-subscribe': noNestedSubscribeRule,
   'no-redundant-notify': noRedundantNotifyRule,
   'no-sharereplay': noSharereplayRule,
+  'no-sharereplay-before-takeuntil': noSharereplayBeforeTakeuntilRule,
   'no-subclass': noSubclassRule,
   'no-subject-unsubscribe': noSubjectUnsubscribeRule,
   'no-subject-value': noSubjectValueRule,

src/rules/no-sharereplay-before-takeuntil.ts

@@ -0,0 +1,86 @@
+import { TSESTree as es } from '@typescript-eslint/utils';
+import { DEFAULT_VALID_POST_COMPLETION_OPERATORS } from '../constants';
+import { isIdentifier, isLiteral, isMemberExpression, isObjectExpression, isProperty } from '../etc';
+import { findIsLastOperatorOrderValid, ruleCreator } from '../utils';
+
+export const noSharereplayBeforeTakeuntilRule = ruleCreator({
+  defaultOptions: [],
+  meta: {
+    docs: {
+      description: 'Disallow using `shareReplay({ refCount: false })` before `takeUntil`.',
+      recommended: 'strict',
+    },
+    messages: {
+      forbidden: 'shareReplay before takeUntil is forbidden unless \'refCount: true\' is specified.',
+    },
+    schema: [],
+    type: 'problem',
+  },
+  name: 'no-sharereplay-before-takeuntil',
+  create: (context) => {
+    function checkCallExpression(node: es.CallExpression) {
+      const pipeCallExpression = node.parent as es.CallExpression;
+      if (
+        !pipeCallExpression.arguments
+        || !isMemberExpression(pipeCallExpression.callee)
+        || !isIdentifier(pipeCallExpression.callee.property)
+        || pipeCallExpression.callee.property.name !== 'pipe'
+      ) {
+        return;
+      }
+
+      const { isOrderValid, operatorNode: takeUntilNode } = findIsLastOperatorOrderValid(
+        pipeCallExpression,
+        /^takeUntil$/,
+        DEFAULT_VALID_POST_COMPLETION_OPERATORS,
+      );
+      if (!isOrderValid || !takeUntilNode) {
+        // takeUntil is not present or in an unsafe position itself.
+        return;
+      }
+
+      if (takeUntilNode.range[0] < node.range[0]) {
+        // takeUntil is before shareReplay.
+        return;
+      }
+
+      const shareReplayConfig = node.arguments[0];
+      if (
+        !shareReplayConfig
+        || !isObjectExpression(shareReplayConfig)
+      ) {
+        // refCount defaults to false if no config is provided.
+        context.report({
+          messageId: 'forbidden',
+          node: node.callee,
+        });
+        return;
+      }
+
+      const refCountElement = shareReplayConfig.properties
+        .filter(isProperty)
+        .find(prop =>
+          isIdentifier(prop.key)
+          && prop.key.name === 'refCount');
+      if (
+        !refCountElement
+        || (isLiteral(refCountElement.value)
+          && refCountElement.value.value === false)
+      ) {
+        context.report({
+          messageId: 'forbidden',
+          node: node.callee,
+        });
+      }
+    }
+
+    return {
+      'CallExpression[callee.name="shareReplay"]': (node: es.CallExpression) => {
+        checkCallExpression(node);
+      },
+      'CallExpression[callee.property.name="shareReplay"]': (node: es.CallExpression) => {
+        checkCallExpression(node);
+      },
+    };
+  },
+});

tests/rules/no-sharereplay-before-takeuntil.test.ts

@@ -0,0 +1,142 @@
+import { stripIndent } from 'common-tags';
+import { noSharereplayBeforeTakeuntilRule } from '../../src/rules/no-sharereplay-before-takeuntil';
+import { fromFixture } from '../etc';
+import { ruleTester } from '../rule-tester';
+
+ruleTester({ types: false }).run('no-sharereplay-before-takeuntil', noSharereplayBeforeTakeuntilRule, {
+  valid: [
+    stripIndent`
+      // with refCount true
+      import { of, takeUntil, shareReplay } from "rxjs";
+
+      const a = of("a");
+      const b = of("b");
+
+      a.pipe(shareReplay({ refCount: true }), takeUntil(b));
+    `,
+    stripIndent`
+      // with refCount true as const
+      import { of, takeUntil, shareReplay } from "rxjs";
+
+      const a = of("a");
+      const b = of("b");
+
+      const t = true as const;
+      a.pipe(shareReplay({ refCount: t }), takeUntil(b));
+    `,
+    stripIndent`
+      // refCount as variable not supported
+      import { of, takeUntil, shareReplay } from "rxjs";
+
+      const a = of("a");
+      const b = of("b");
+
+      const t = false;
+      a.pipe(shareReplay({ refCount: t }), takeUntil(b));
+    `,
+    stripIndent`
+      // composed observables not supported
+      import { of, takeUntil, shareReplay } from "rxjs";
+
+      const a = of("a");
+      const b = of("b");
+
+      const sr = shareReplay({ refCount: false });
+      a.pipe(sr, takeUntil(b));
+    `,
+    stripIndent`
+      // shareReplay after takeUntil
+      import { of, takeUntil, shareReplay } from "rxjs";
+
+      const a = of("a");
+      const b = of("b");
+
+      a.pipe(takeUntil(b), shareReplay({ refCount: false }));
+    `,
+    stripIndent`
+      // shareReplay after takeUntil with operators in between
+      import { of, takeUntil, shareReplay, map, filter } from "rxjs";
+
+      const a = of("a");
+      const b = of("b");
+
+      a.pipe(takeUntil(b), map(x => x), filter(x => !!x), shareReplay());
+    `,
+    stripIndent`
+      // namespace import with refCount true
+      import * as Rx from "rxjs";
+
+      const a = Rx.of("a");
+      const b = Rx.of("b");
+
+      a.pipe(Rx.shareReplay({ refCount: true }), Rx.takeUntil(b));
+    `,
+    stripIndent`
+      // shareReplay by itself
+      import { of, shareReplay } from "rxjs";
+
+      const a = of("a");
+
+      a.pipe(shareReplay());
+    `,
+    stripIndent`
+      // unrelated
+      import { of, takeUntil, toArray } from "rxjs";
+
+      const a = of("a");
+      const b = of("b");
+
+      a.pipe(takeUntil(b), toArray());
+    `,
+  ],
+  invalid: [
+    fromFixture(
+      stripIndent`
+        // without config
+        import { of, takeUntil, shareReplay } from "rxjs";
+
+        const a = of("a");
+        const b = of("b");
+
+        a.pipe(shareReplay(), takeUntil(b));
+               ~~~~~~~~~~~ [forbidden]
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // with refCount false
+        import { of, takeUntil, shareReplay } from "rxjs";
+
+        const a = of("a");
+        const b = of("b");
+
+        a.pipe(shareReplay({ refCount: false }), takeUntil(b));
+               ~~~~~~~~~~~ [forbidden]
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // with operators in between
+        import { of, takeUntil, shareReplay, map, filter } from "rxjs";
+
+        const a = of("a");
+        const b = of("b");
+
+        a.pipe(shareReplay(), map(x => x), filter(x => !!x), takeUntil(b));
+               ~~~~~~~~~~~ [forbidden]
+      `,
+    ),
+    fromFixture(
+      stripIndent`
+        // namespace import
+        import * as Rx from "rxjs";
+
+        const a = Rx.of("a");
+        const b = Rx.of("b");
+
+        a.pipe(Rx.shareReplay(), Rx.takeUntil(b));
+               ~~~~~~~~~~~~~~ [forbidden]
+      `,
+    ),
+  ],
+});