diff --git a/README.md b/README.md index 71f80f61..56b8cbc3 100644 --- a/README.md +++ b/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. | ✅ 🔒 | | | 💭 | | diff --git a/docs/rules/no-subscribe-in-pipe.md b/docs/rules/no-subscribe-in-pipe.md new file mode 100644 index 00000000..24a6352c --- /dev/null +++ b/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). + + + +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)); +``` diff --git a/src/configs/recommended.ts b/src/configs/recommended.ts index c29e47a7..5d08cb94 100644 --- a/src/configs/recommended.ts +++ b/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', diff --git a/src/configs/strict.ts b/src/configs/strict.ts index aca1e482..259ca297 100644 --- a/src/configs/strict.ts +++ b/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', diff --git a/src/index.ts b/src/index.ts index 3bab9b8d..cf693fa2 100644 --- a/src/index.ts +++ b/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, diff --git a/src/rules/no-subscribe-in-pipe.ts b/src/rules/no-subscribe-in-pipe.ts new file mode 100644 index 00000000..8f008fb1 --- /dev/null +++ b/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, + }); + } + }, + }; + }, +}); diff --git a/tests/rules/no-subscribe-in-pipe.test.ts b/tests/rules/no-subscribe-in-pipe.test.ts new file mode 100644 index 00000000..78a126ae --- /dev/null +++ b/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(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(); + `, + ), + ], +});