feat(no-floating-observables)!: replace no-ignored-observable (#55)

BREAKING CHANGE: `no-ignored-observable` is removed; use
`no-floating-observables` instead.

- Enhance floating observable logic to handle more than just call
expressions.
- Now that `no-ignored-observable` is included in the `strict` config,
enhancing it will be more widely useful.
- Add `ignoreVoid` option. When `false`, this rule will reach past the
`void` operator and check its argument. Defaults to `true` (i.e. `void`
can be used to ignore floating observables).
- Rename rule from `no-ignored-observable` to `no-floating-observables`.
- As mentioned in #43 , we should take advantage of this project being a
fork to clarify rule names, and typescript-eslint has an existing
convention for promises that would be nice to imitate for observables:
one rule covers floating, the other covers misused.
- An alternative is keeping the old rule as-is and deprecating it.
However, this forked project is not v1 yet, so we should take advantage
of prerelease to make breaking changes early instead of putting them off
for a later, more painful date.

Author: JasonWeinzierl

README.md

@@ -17,6 +17,7 @@ There are some breaking changes:
   - If you need to continue using this old format, use the original `eslint-plugin-rxjs` or a different fork.
 - The plugin namespace specified in the `recommended` config was changed from `rxjs` to `rxjs-x`.
   - e.g. In your ESLint config, `rxjs/no-subject-value` should be renamed to `rxjs-x/no-subject-value`.
+- The rule `rxjs/no-ignored-observable` is renamed to `rxjs-x/no-floating-observables`.
 
 A complete description of all changes are documented in the [CHANGELOG](CHANGELOG.md) file.
 
@@ -88,10 +89,10 @@ The package includes the following rules.
 | [no-explicit-generics](docs/rules/no-explicit-generics.md)             | Disallow unnecessary explicit generic type arguments.                                                     | 🔒   |    |    |    |    |
 | [no-exposed-subjects](docs/rules/no-exposed-subjects.md)               | Disallow public and protected subjects.                                                                   | 🔒   |    |    | 💭 |    |
 | [no-finnish](docs/rules/no-finnish.md)                                 | Disallow Finnish notation.                                                                                |      |    |    | 💭 |    |
+| [no-floating-observables](docs/rules/no-floating-observables.md)       | Require Observables to be handled appropriately.                                                          | 🔒   |    |    | 💭 |    |
 | [no-ignored-default-value](docs/rules/no-ignored-default-value.md)     | Disallow using `firstValueFrom`, `lastValueFrom`, `first`, and `last` without specifying a default value. | 🔒   |    |    | 💭 |    |
 | [no-ignored-error](docs/rules/no-ignored-error.md)                     | Disallow calling `subscribe` without specifying an error handler.                                         | 🔒   |    |    | 💭 |    |
 | [no-ignored-notifier](docs/rules/no-ignored-notifier.md)               | Disallow observables not composed from the `repeatWhen` or `retryWhen` notifier.                          | ✅ 🔒 |    |    | 💭 |    |
-| [no-ignored-observable](docs/rules/no-ignored-observable.md)           | Disallow ignoring observables returned by functions.                                                      | 🔒   |    |    | 💭 |    |
 | [no-ignored-replay-buffer](docs/rules/no-ignored-replay-buffer.md)     | Disallow using `ReplaySubject`, `publishReplay` or `shareReplay` without specifying the buffer size.      | ✅ 🔒 |    |    |    |    |
 | [no-ignored-subscribe](docs/rules/no-ignored-subscribe.md)             | Disallow calling `subscribe` without specifying arguments.                                                |      |    |    | 💭 |    |
 | [no-ignored-subscription](docs/rules/no-ignored-subscription.md)       | Disallow ignoring the subscription returned by `subscribe`.                                               |      |    |    | 💭 |    |

docs/rules/no-floating-observables.md

@@ -0,0 +1,47 @@
+# Require Observables to be handled appropriately (`rxjs-x/no-floating-observables`)
+
+💼 This rule is enabled in the 🔒 `strict` config.
+
+💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting).
+
+<!-- end auto-generated rule header -->
+
+A "floating" observable is one that is created without any code set up to handle any errors it might emit.
+Like a floating Promise, floating observables can cause several issues, such as ignored errors, unhandled cold observables, and more.
+
+This rule is like [no-floating-promises](https://typescript-eslint.io/rules/no-floating-promises/) but for Observables.
+This rule will report observable-valued statements that are not treated in one of the following ways:
+
+- Calling its `.subscribe()`
+- `return`ing it
+- Wrapping it in `lastValueFrom` or `firstValueFrom` and `await`ing it
+- [`void`ing it](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/void)
+
+> [!TIP]
+> `no-floating-observables` only detects apparently unhandled observable _statements_.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```ts
+import { of } from "rxjs";
+of(42, 54);
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+import { of } from "rxjs";
+const answers = of(42, 54);
+```
+
+## Options
+
+<!-- begin auto-generated rule options list -->
+
+| Name         | Description                           | Type    | Default |
+| :----------- | :------------------------------------ | :------ | :------ |
+| `ignoreVoid` | Whether to ignore `void` expressions. | Boolean | `true`  |
+
+<!-- end auto-generated rule options list -->

docs/rules/no-ignored-observable.md

@@ -11,10 +11,10 @@ export const createStrictConfig = (
     'rxjs-x/no-create': 'error',
     'rxjs-x/no-explicit-generics': 'error',
     'rxjs-x/no-exposed-subjects': 'error',
+    'rxjs-x/no-floating-observables': 'error',
     'rxjs-x/no-ignored-default-value': 'error',
     'rxjs-x/no-ignored-error': 'error',
     'rxjs-x/no-ignored-notifier': 'error',
-    'rxjs-x/no-ignored-observable': 'error',
     'rxjs-x/no-ignored-replay-buffer': 'error',
     'rxjs-x/no-ignored-takewhile-value': 'error',
     'rxjs-x/no-implicit-any-catch': ['error', {

src/configs/strict.ts

@@ -34,6 +34,10 @@ export function isCallExpression(node: TSESTree.Node): node is TSESTree.CallExpr
   return node.type === AST_NODE_TYPES.CallExpression;
 }
 
+export function isChainExpression(node: TSESTree.Node): node is TSESTree.ChainExpression {
+  return node.type === AST_NODE_TYPES.ChainExpression;
+}
+
 export function isExportNamedDeclaration(
   node: TSESTree.Node,
 ): node is TSESTree.ExportNamedDeclaration {
@@ -124,6 +128,10 @@ export function isTSTypeReference(node: TSESTree.Node): node is TSESTree.TSTypeR
   return node.type === AST_NODE_TYPES.TSTypeReference;
 }
 
+export function isUnaryExpression(node: TSESTree.Node): node is TSESTree.UnaryExpression {
+  return node.type === AST_NODE_TYPES.UnaryExpression;
+}
+
 export function isVariableDeclarator(
   node: TSESTree.Node,
 ): node is TSESTree.VariableDeclarator {

src/etc/is.ts

@@ -16,10 +16,10 @@ import { noCyclicActionRule } from './rules/no-cyclic-action';
 import { noExplicitGenericsRule } from './rules/no-explicit-generics';
 import { noExposedSubjectsRule } from './rules/no-exposed-subjects';
 import { noFinnishRule } from './rules/no-finnish';
+import { noFloatingObservablesRule } from './rules/no-floating-observables';
 import { noIgnoredDefaultValueRule } from './rules/no-ignored-default-value';
 import { noIgnoredErrorRule } from './rules/no-ignored-error';
 import { noIgnoredNotifierRule } from './rules/no-ignored-notifier';
-import { noIgnoredObservableRule } from './rules/no-ignored-observable';
 import { noIgnoredReplayBufferRule } from './rules/no-ignored-replay-buffer';
 import { noIgnoredSubscribeRule } from './rules/no-ignored-subscribe';
 import { noIgnoredSubscriptionRule } from './rules/no-ignored-subscription';
@@ -63,10 +63,10 @@ const plugin = {
     'no-explicit-generics': noExplicitGenericsRule,
     'no-exposed-subjects': noExposedSubjectsRule,
     'no-finnish': noFinnishRule,
+    'no-floating-observables': noFloatingObservablesRule,
     'no-ignored-default-value': noIgnoredDefaultValueRule,
     'no-ignored-error': noIgnoredErrorRule,
     'no-ignored-notifier': noIgnoredNotifierRule,
-    'no-ignored-observable': noIgnoredObservableRule,
     'no-ignored-replay-buffer': noIgnoredReplayBufferRule,
     'no-ignored-subscribe': noIgnoredSubscribeRule,
     'no-ignored-subscription': noIgnoredSubscriptionRule,

src/index.ts

@@ -0,0 +1,97 @@
+import { TSESTree as es } from '@typescript-eslint/utils';
+import { getTypeServices, isCallExpression, isChainExpression, isUnaryExpression } from '../etc';
+import { ruleCreator } from '../utils';
+
+const defaultOptions: readonly {
+  ignoreVoid?: boolean;
+}[] = [];
+
+const messageBase
+  = 'Observables must be subscribed to, returned, converted to a promise and awaited, '
+  + 'or be explicitly marked as ignored with the `void` operator.';
+
+const messageBaseNoVoid
+    = 'Observables must be subscribed to, returned, or converted to a promise and awaited.';
+
+export const noFloatingObservablesRule = ruleCreator({
+  defaultOptions,
+  meta: {
+    docs: {
+      description: 'Require Observables to be handled appropriately.',
+      recommended: 'strict',
+      requiresTypeChecking: true,
+    },
+    messages: {
+      forbidden: messageBase,
+      forbiddenNoVoid: messageBaseNoVoid,
+    },
+    schema: [
+      {
+        properties: {
+          ignoreVoid: { type: 'boolean', default: true, description: 'Whether to ignore `void` expressions.' },
+        },
+        type: 'object',
+      },
+    ],
+    type: 'problem',
+  },
+  name: 'no-floating-observables',
+  create: (context) => {
+    const { couldBeObservable } = getTypeServices(context);
+    const [config = {}] = context.options;
+    const { ignoreVoid = true } = config;
+
+    function checkNode(node: es.CallExpression) {
+      if (couldBeObservable(node)) {
+        context.report({
+          messageId: ignoreVoid ? 'forbidden' : 'forbiddenNoVoid',
+          node,
+        });
+      }
+    }
+
+    function checkVoid(node: es.UnaryExpression) {
+      if (ignoreVoid) return;
+      if (node.operator !== 'void') return;
+
+      let expression = node.argument;
+      if (isChainExpression(expression)) {
+        expression = expression.expression;
+      }
+
+      if (!isCallExpression(expression)) return;
+      checkNode(expression);
+    }
+
+    return {
+      'ExpressionStatement > CallExpression': (node: es.CallExpression) => {
+        checkNode(node);
+      },
+      'ExpressionStatement > UnaryExpression': (node: es.UnaryExpression) => {
+        checkVoid(node);
+      },
+      'ExpressionStatement > ChainExpression': (node: es.ChainExpression) => {
+        if (!isCallExpression(node.expression)) return;
+
+        checkNode(node.expression);
+      },
+      'ExpressionStatement > SequenceExpression': (node: es.SequenceExpression) => {
+        node.expressions.forEach(expression => {
+          if (isCallExpression(expression)) {
+            checkNode(expression);
+          }
+        });
+      },
+      'ExpressionStatement > ArrayExpression': (node: es.ArrayExpression) => {
+        node.elements.forEach(expression => {
+          if (!expression) return;
+          if (isCallExpression(expression)) {
+            checkNode(expression);
+          } else if (isUnaryExpression(expression)) {
+            checkVoid(expression);
+          }
+        });
+      },
+    };
+  },
+});