diff --git a/README.md b/README.md index 5e1bd97e..7eb0a38a 100644 --- a/README.md +++ b/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`. | | | | 💭 | | diff --git a/docs/rules/no-floating-observables.md b/docs/rules/no-floating-observables.md new file mode 100644 index 00000000..078e2afe --- /dev/null +++ b/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). + + + +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 + + + +| Name | Description | Type | Default | +| :----------- | :------------------------------------ | :------ | :------ | +| `ignoreVoid` | Whether to ignore `void` expressions. | Boolean | `true` | + + diff --git a/docs/rules/no-ignored-observable.md b/docs/rules/no-ignored-observable.md deleted file mode 100644 index 2b113752..00000000 --- a/docs/rules/no-ignored-observable.md +++ /dev/null @@ -1,25 +0,0 @@ -# Disallow ignoring observables returned by functions (`rxjs-x/no-ignored-observable`) - -💼 This rule is enabled in the 🔒 `strict` config. - -💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting). - - - -The effects failures if an observable returned by a function is neither assigned to a variable or property or passed to a function. - -## 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); -``` diff --git a/src/configs/strict.ts b/src/configs/strict.ts index 9fcfef6b..b10efa99 100644 --- a/src/configs/strict.ts +++ b/src/configs/strict.ts @@ -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', { diff --git a/src/etc/is.ts b/src/etc/is.ts index 7e3f4d6b..1171d708 100644 --- a/src/etc/is.ts +++ b/src/etc/is.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 { diff --git a/src/index.ts b/src/index.ts index d83cef9c..b0afe264 100644 --- a/src/index.ts +++ b/src/index.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, diff --git a/src/rules/no-floating-observables.ts b/src/rules/no-floating-observables.ts new file mode 100644 index 00000000..569354ec --- /dev/null +++ b/src/rules/no-floating-observables.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); + } + }); + }, + }; + }, +}); diff --git a/src/rules/no-ignored-observable.ts b/src/rules/no-ignored-observable.ts deleted file mode 100644 index e1b9e46b..00000000 --- a/src/rules/no-ignored-observable.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { TSESTree as es } from '@typescript-eslint/utils'; -import { getTypeServices } from '../etc'; -import { ruleCreator } from '../utils'; - -export const noIgnoredObservableRule = ruleCreator({ - defaultOptions: [], - meta: { - docs: { - description: 'Disallow ignoring observables returned by functions.', - recommended: 'strict', - requiresTypeChecking: true, - }, - messages: { - forbidden: 'Ignoring a returned Observable is forbidden.', - }, - schema: [], - type: 'problem', - }, - name: 'no-ignored-observable', - create: (context) => { - const { couldBeObservable } = getTypeServices(context); - - return { - 'ExpressionStatement > CallExpression': (node: es.CallExpression) => { - if (couldBeObservable(node)) { - context.report({ - messageId: 'forbidden', - node, - }); - } - }, - }; - }, -}); diff --git a/tests/rules/no-floating-observables.test.ts b/tests/rules/no-floating-observables.test.ts new file mode 100644 index 00000000..da588408 --- /dev/null +++ b/tests/rules/no-floating-observables.test.ts @@ -0,0 +1,131 @@ +import { stripIndent } from 'common-tags'; +import { noFloatingObservablesRule } from '../../src/rules/no-floating-observables'; +import { fromFixture } from '../etc'; +import { ruleTester } from '../rule-tester'; + +ruleTester({ types: true }).run('no-floating-observables', noFloatingObservablesRule, { + valid: [ + stripIndent` + // not ignored + import { Observable, of } from "rxjs"; + + function functionSource() { + return of(42); + } + + function sink(source: Observable) { + } + + functionSource().subscribe(); + const a = functionSource(); + const b = [functionSource()]; + [void functionSource()]; + sink(functionSource()); + void functionSource(); + `, + stripIndent` + // not ignored arrow + import { Observable, of } from "rxjs"; + + const arrowSource = () => of(42); + + function sink(source: Observable) { + } + + functionSource().subscribe(); + const a = arrowSource(); + const b = [arrowSource()]; + [void arrowSource()]; + sink(arrowSource()); + void functionSource(); + `, + stripIndent` + // unrelated + function foo() {} + + [1, 2, 3, 'foo']; + const a = [1]; + foo(); + [foo()]; + void foo(); + `, + ], + invalid: [ + fromFixture( + stripIndent` + // ignored + import { Observable, of } from "rxjs"; + + function functionSource() { + return of(42); + } + + functionSource(); + ~~~~~~~~~~~~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // ignored arrow + import { Observable, of } from "rxjs"; + + const arrowSource = () => of(42); + + arrowSource(); + ~~~~~~~~~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // chain expression + import { Observable, of } from "rxjs"; + + const arrowSource: null | (() => Observable) = () => of(42); + + arrowSource?.(); + ~~~~~~~~~~~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // array + import { of } from "rxjs"; + + [of(42)]; + ~~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // ignoreVoid false + import { Observable, of } from "rxjs"; + + function functionSource() { + return of(42); + } + + void functionSource(); + ~~~~~~~~~~~~~~~~ [forbiddenNoVoid] + void functionSource?.(); + ~~~~~~~~~~~~~~~~~~ [forbiddenNoVoid] + [void functionSource()]; + ~~~~~~~~~~~~~~~~ [forbiddenNoVoid] + [void functionSource?.()]; + ~~~~~~~~~~~~~~~~~~ [forbiddenNoVoid] + `, + { + options: [{ ignoreVoid: false }], + }, + ), + fromFixture( + stripIndent` + // sequence expression + import { of } from "rxjs"; + + of(42), of(42), void of(42); + ~~~~~~ [forbidden] + ~~~~~~ [forbidden] + `, + ), + ], +}); diff --git a/tests/rules/no-ignored-observable.test.ts b/tests/rules/no-ignored-observable.test.ts deleted file mode 100644 index 8c6b8b6a..00000000 --- a/tests/rules/no-ignored-observable.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { stripIndent } from 'common-tags'; -import { noIgnoredObservableRule } from '../../src/rules/no-ignored-observable'; -import { fromFixture } from '../etc'; -import { ruleTester } from '../rule-tester'; - -ruleTester({ types: true }).run('no-ignored-observable', noIgnoredObservableRule, { - valid: [ - stripIndent` - // not ignored - import { Observable, of } from "rxjs"; - - function functionSource() { - return of(42); - } - - function sink(source: Observable) { - } - - const a = functionSource(); - sink(functionSource()); - `, - stripIndent` - // not ignored arrow - import { Observable, of } from "rxjs"; - - const arrowSource = () => of(42); - - function sink(source: Observable) { - } - - const a = arrowSource(); - sink(arrowSource()); - `, - ], - invalid: [ - fromFixture( - stripIndent` - // ignored - import { Observable, of } from "rxjs"; - - function functionSource() { - return of(42); - } - - functionSource(); - ~~~~~~~~~~~~~~~~ [forbidden] - `, - ), - fromFixture( - stripIndent` - // ignored arrow - import { Observable, of } from "rxjs"; - - const arrowSource = () => of(42); - - arrowSource(); - ~~~~~~~~~~~~~ [forbidden] - `, - ), - ], -});