diff --git a/README.md b/README.md index d8657b7c..e61fb658 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,7 @@ The package includes the following rules. | [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. | ✅ 🔒 | | | 💭 | | +| [no-unnecessary-collection](docs/rules/no-unnecessary-collection.md) | Disallow unnecessary usage of collection arguments with single values. | 🔒 | | | | | | [no-unsafe-catch](docs/rules/no-unsafe-catch.md) | Disallow unsafe `catchError` usage in effects and epics. | | | | 💭 | | | [no-unsafe-first](docs/rules/no-unsafe-first.md) | Disallow unsafe `first`/`take` usage in effects and epics. | | | | 💭 | | | [no-unsafe-subject-next](docs/rules/no-unsafe-subject-next.md) | Disallow unsafe optional `next` calls. | ✅ 🔒 | | | 💭 | | diff --git a/docs/rules/no-unnecessary-collection.md b/docs/rules/no-unnecessary-collection.md new file mode 100644 index 00000000..616fc7ba --- /dev/null +++ b/docs/rules/no-unnecessary-collection.md @@ -0,0 +1,59 @@ +# Disallow unnecessary usage of collection arguments with single values (`rxjs-x/no-unnecessary-collection`) + +💼 This rule is enabled in the 🔒 `strict` config. + + + +This rule effects failures when passing a collection (object or array) containing a single observable +to the static observable creators that accept multiple observables +(`combineLatest`, `forkJoin`, `merge`, `zip`, `concat`, `race`). +Use of these creator functions with only a single observable +can be replaced with direct usage of the observable itself. + +## Rule details + +Examples of **incorrect** code for this rule: + +```ts +import { combineLatest, forkJoin, merge, zip, concat, race, of } from "rxjs"; + +// These incorrect example can be simplified: +const a$ = combineLatest([of(1)]); +const b$ = forkJoin([of('data')]); + +const c$ = combineLatest({ value: of(1) }); +const d$ = forkJoin({ data: of('hello') }); + +const e$ = merge(of(1)); +const f$ = zip(of(1)); +const g$ = concat(of(1)); +const h$ = race(of(1)); +``` + +Examples of **correct** code for this rule: + +```ts +import { of, map } from "rxjs"; + +// These are equivalent to the previous examples: +const a$ = of(1); +const b$ = of('data').pipe(map(x => [x])); + +const c$ = of(1).pipe(map(x => ({ value: x }))) +const d$ = of('hello').pipe(map(x => ({ data: x }))); + +const e$ = of(1); +const f$ = of(1).pipe(map(x => [x])); +const g$ = of(1); +const h$ = of(1); +``` + +## When Not To Use It + +If you don't care about unnecessary usage of static observable creators, +then you don't need this rule. + +## Resources + +- [Rule source](/src/rules/no-unnecessary-collection.ts) +- [Test source](/tests/rules/no-unnecessary-collection.test.ts) diff --git a/src/configs/strict.ts b/src/configs/strict.ts index 53ad79d3..80456db9 100644 --- a/src/configs/strict.ts +++ b/src/configs/strict.ts @@ -33,6 +33,7 @@ export const createStrictConfig = ( 'rxjs-x/no-subscribe-in-pipe': 'error', 'rxjs-x/no-topromise': 'error', 'rxjs-x/no-unbound-methods': 'error', + 'rxjs-x/no-unnecessary-collection': 'error', 'rxjs-x/no-unsafe-subject-next': 'error', 'rxjs-x/no-unsafe-takeuntil': 'error', 'rxjs-x/prefer-observer': 'error', diff --git a/src/constants.ts b/src/constants.ts index a431977c..a9c241de 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -9,6 +9,19 @@ export const SOURCES_OBJECT_ACCEPTING_STATIC_OBSERVABLE_CREATORS = [ 'forkJoin', ]; +/** + * The names of static observable creators + * that accept a collection (array or object) of multiple observables as input. + */ +export const MULTIPLE_OBSERVABLE_ACCEPTING_STATIC_OBSERVABLE_CREATORS = [ + 'combineLatest', + 'forkJoin', + 'merge', + 'zip', + 'concat', + 'race', +]; + /** * The names of types that are allowed to be passed unbound. */ diff --git a/src/index.ts b/src/index.ts index 6d9ddaf6..b5ea3ee0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -41,6 +41,7 @@ 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'; +import { noUnnecessaryCollectionRule } from './rules/no-unnecessary-collection'; import { noUnsafeCatchRule } from './rules/no-unsafe-catch'; import { noUnsafeFirstRule } from './rules/no-unsafe-first'; import { noUnsafeSubjectNext } from './rules/no-unsafe-subject-next'; @@ -89,6 +90,7 @@ const allRules = { 'no-tap': noTapRule, 'no-topromise': noTopromiseRule, 'no-unbound-methods': noUnboundMethodsRule, + 'no-unnecessary-collection': noUnnecessaryCollectionRule, 'no-unsafe-catch': noUnsafeCatchRule, 'no-unsafe-first': noUnsafeFirstRule, 'no-unsafe-subject-next': noUnsafeSubjectNext, diff --git a/src/rules/no-unnecessary-collection.ts b/src/rules/no-unnecessary-collection.ts new file mode 100644 index 00000000..1b6c7e80 --- /dev/null +++ b/src/rules/no-unnecessary-collection.ts @@ -0,0 +1,107 @@ +import { + TSESTree as es, +} from '@typescript-eslint/utils'; +import { MULTIPLE_OBSERVABLE_ACCEPTING_STATIC_OBSERVABLE_CREATORS, SOURCES_OBJECT_ACCEPTING_STATIC_OBSERVABLE_CREATORS } from '../constants'; +import { getTypeServices, isArrayExpression, isObjectExpression, isProperty } from '../etc'; +import { ruleCreator } from '../utils'; + +export const noUnnecessaryCollectionRule = ruleCreator({ + defaultOptions: [], + meta: { + docs: { + description: 'Disallow unnecessary usage of collection arguments with single values.', + recommended: 'strict', + requiresTypeChecking: false, + }, + messages: { + forbidden: 'Unnecessary {{operator}} with {{inputType}}. Use the observable directly instead.', + }, + schema: [], + type: 'suggestion', + }, + name: 'no-unnecessary-collection', + create: (context) => { + const { couldBeType, couldBeObservable } = getTypeServices(context); + + function couldBeFromRxjs(node: es.Node, operatorName: string): boolean { + return couldBeType(node, operatorName, { name: /[/\\]rxjs[/\\]/ }); + } + + function checkCallExpression(node: es.CallExpression, operatorName: string): void { + const args = node.arguments; + + if (args.length === 0) { + return; + } + + const firstArg = args[0]; + + // Single-valued array. + if (isArrayExpression(firstArg)) { + const nonNullElements = firstArg.elements.filter(element => element !== null); + if (nonNullElements.length === 1 && couldBeObservable(nonNullElements[0])) { + context.report({ + messageId: 'forbidden', + node: node.callee, + data: { + operator: operatorName, + inputType: 'single-valued array', + }, + }); + } + return; + } + + // Single-property object. + if (isObjectExpression(firstArg) && SOURCES_OBJECT_ACCEPTING_STATIC_OBSERVABLE_CREATORS.includes(operatorName)) { + if (firstArg.properties.length === 1 && isProperty(firstArg.properties[0])) { + const property = firstArg.properties[0]; + if (property.value && couldBeObservable(property.value)) { + context.report({ + messageId: 'forbidden', + node: node.callee, + data: { + operator: operatorName, + inputType: 'single-property object', + }, + }); + } + } + return; + } + + // Single rest parameter argument. + if (args.length === 1 && couldBeObservable(firstArg)) { + context.report({ + messageId: 'forbidden', + node: node.callee, + data: { + operator: operatorName, + inputType: 'single argument', + }, + }); + } + } + + const callExpressionVisitors: Record void> = {}; + + for (const operator of MULTIPLE_OBSERVABLE_ACCEPTING_STATIC_OBSERVABLE_CREATORS) { + callExpressionVisitors[`CallExpression[callee.name="${operator}"]`] = (node: es.CallExpression) => { + if (couldBeFromRxjs(node.callee, operator)) { + checkCallExpression(node, operator); + } + }; + } + + for (const operator of MULTIPLE_OBSERVABLE_ACCEPTING_STATIC_OBSERVABLE_CREATORS) { + callExpressionVisitors[`CallExpression[callee.type="MemberExpression"][callee.property.name="${operator}"]`] = (node: es.CallExpression) => { + const memberExpr = node.callee as es.MemberExpression; + if (couldBeFromRxjs(memberExpr.property, operator)) { + checkCallExpression(node, operator); + } + }; + } + + return callExpressionVisitors; + }, +}); diff --git a/tests/rules/no-unnecessary-collection.test.ts b/tests/rules/no-unnecessary-collection.test.ts new file mode 100644 index 00000000..6fe5ca66 --- /dev/null +++ b/tests/rules/no-unnecessary-collection.test.ts @@ -0,0 +1,407 @@ +import { stripIndent } from 'common-tags'; +import { noUnnecessaryCollectionRule } from '../../src/rules/no-unnecessary-collection'; +import { fromFixture } from '../etc'; +import { ruleTester } from '../rule-tester'; + +ruleTester({ types: true }).run('no-unnecessary-collection', noUnnecessaryCollectionRule, { + valid: [ + // #region valid; multiple observables in array + stripIndent` + // combineLatest with multiple observables in array + import { combineLatest, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const combined$ = combineLatest([a$, b$]); + `, + stripIndent` + // combineLatest with multiple observables in sparse array + import { combineLatest, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const combined$ = combineLatest([, a$, b$]); + `, + stripIndent` + // forkJoin with multiple observables in array + import { forkJoin, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const combined$ = forkJoin([a$, b$]); + `, + stripIndent` + // merge with multiple observables + import { merge, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const merged$ = merge(a$, b$); + `, + stripIndent` + // zip with multiple observables + import { zip, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const zipped$ = zip(a$, b$); + `, + stripIndent` + // concat with multiple observables + import { concat, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const concatenated$ = concat(a$, b$); + `, + stripIndent` + // race with multiple observables + import { race, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const raced$ = race(a$, b$); + `, + // #endregion + + // #region valid; multiple observables in object + stripIndent` + // combineLatest with multiple observables in object + import { combineLatest, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const combined$ = combineLatest({ a: a$, b: b$ }); + `, + stripIndent` + // forkJoin with multiple observables in object + import { forkJoin, of } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const combined$ = forkJoin({ a: a$, b: b$ }); + `, + stripIndent` + // forkJoin with spread object properties + import { forkJoin, of } from "rxjs"; + + const source1$ = of(1); + const source2$ = of(2); + const sources = { first: source1$, second: source2$ }; + const combined$ = forkJoin({ ...sources }); + `, + // #endregion + + // #region valid; no arguments or empty + stripIndent` + // combineLatest with no arguments + import { combineLatest } from "rxjs"; + + const combined$ = combineLatest(); + `, + stripIndent` + // merge with no arguments + import { merge } from "rxjs"; + + const merged$ = merge(); + `, + stripIndent` + // combineLatest with empty array + import { combineLatest } from "rxjs"; + + const combined$ = combineLatest([]); + `, + stripIndent` + // forkJoin with empty object + import { forkJoin } from "rxjs"; + + const combined$ = forkJoin({}); + `, + // #endregion + + // #region valid; non-rxjs functions with same names + stripIndent` + // non-RxJS combineLatest function + function combineLatest(observables: any[]) { + return observables[0]; + } + + const result = combineLatest([someValue]); + `, + stripIndent` + // method call on different object + const someObject = { + forkJoin(sources: any) { return sources; } + }; + + const result = someObject.forkJoin([singleValue]); + `, + // #endregion + + // #region valid; dynamic arrays are not supported + stripIndent` + // combineLatest with variable array + import { combineLatest, of } from "rxjs"; + + const observables = [of(1)]; + const combined$ = combineLatest(observables); + `, + stripIndent` + // forkJoin with computed array + import { forkJoin, of } from "rxjs"; + + const sources = getSources(); // could return array of any length + const combined$ = forkJoin(sources); + `, + // #endregion + + // #region valid; namespace imports + stripIndent` + // namespace import with multiple observables in array + import * as Rx from "rxjs"; + + const a$ = Rx.of(1); + const b$ = Rx.of(2); + const combined$ = Rx.combineLatest([a$, b$]); + `, + stripIndent` + // namespace import with multiple observables in object + import * as Rx from "rxjs"; + + const source1$ = Rx.of('a'); + const source2$ = Rx.of('b'); + const result$ = Rx.forkJoin({ first: source1$, second: source2$ }); + `, + // #endregion + + // #region valid; aliased imports are not supported + stripIndent` + // aliased combineLatest with single observable + import { combineLatest as combine, of } from "rxjs"; + + const a$ = of(1); + const combined$ = combine([a$]); + `, + stripIndent` + // aliased forkJoin with single observable in object + import { forkJoin as fork, of } from "rxjs"; + + const source$ = of('test'); + const result$ = fork({ single: source$ }); + `, + // #endregion + + // #region valid; unrelated + stripIndent` + // unrelated code + import { of, map, from, timer, raceWith } from "rxjs"; + + const a$ = of(1); + const b$ = of(2); + const sum$ = a$.pipe( + map(x => x + 10), + ); + const raced$ = a$.pipe( + raceWith(b$), + ); + + const c$ = from([1]); + + const d$ = timer(1000); + `, + // #endregion + ], + + invalid: [ + // #region invalid; single observable in array + fromFixture( + stripIndent` + // combineLatest with single observable in array + import { combineLatest, of } from "rxjs"; + + const a$ = of(1); + const combined$ = combineLatest([a$]); + ~~~~~~~~~~~~~ [forbidden { "operator": "combineLatest", "inputType": "single-valued array" }] + `, + ), + fromFixture( + stripIndent` + // forkJoin with single observable in array + import { forkJoin, of } from "rxjs"; + + const a$ = of(1); + const combined$ = forkJoin([a$]); + ~~~~~~~~ [forbidden { "operator": "forkJoin", "inputType": "single-valued array" }] + `, + ), + fromFixture( + stripIndent` + // merge with single observable + import { merge, of } from "rxjs"; + + const a$ = of(1); + const merged$ = merge(a$); + ~~~~~ [forbidden { "operator": "merge", "inputType": "single argument" }] + `, + ), + fromFixture( + stripIndent` + // zip with single observable + import { zip, of } from "rxjs"; + + const a$ = of(1); + const zipped$ = zip(a$); + ~~~ [forbidden { "operator": "zip", "inputType": "single argument" }] + `, + ), + fromFixture( + stripIndent` + // concat with single observable + import { concat, of } from "rxjs"; + + const a$ = of(1); + const concatenated$ = concat(a$); + ~~~~~~ [forbidden { "operator": "concat", "inputType": "single argument" }] + `, + ), + fromFixture( + stripIndent` + // race with single observable + import { race, of } from "rxjs"; + + const a$ = of(1); + const raced$ = race(a$); + ~~~~ [forbidden { "operator": "race", "inputType": "single argument" }] + `, + ), + // #endregion + + // #region invalid; single observable in object + fromFixture( + stripIndent` + // combineLatest with single property in object + import { combineLatest, of } from "rxjs"; + + const a$ = of(1); + const combined$ = combineLatest({ a: a$ }); + ~~~~~~~~~~~~~ [forbidden { "operator": "combineLatest", "inputType": "single-property object" }] + `, + ), + fromFixture( + stripIndent` + // forkJoin with single property in object + import { forkJoin, of } from "rxjs"; + + const a$ = of(1); + const combined$ = forkJoin({ result: a$ }); + ~~~~~~~~ [forbidden { "operator": "forkJoin", "inputType": "single-property object" }] + `, + ), + // #endregion + + // #region invalid; inline observables + fromFixture( + stripIndent` + // combineLatest with inline single observable + import { combineLatest, of } from "rxjs"; + + const combined$ = combineLatest([of(1)]); + ~~~~~~~~~~~~~ [forbidden { "operator": "combineLatest", "inputType": "single-valued array" }] + `, + ), + fromFixture( + stripIndent` + // forkJoin with inline single observable in object + import { forkJoin, of } from "rxjs"; + + const combined$ = forkJoin({ data: of('hello') }); + ~~~~~~~~ [forbidden { "operator": "forkJoin", "inputType": "single-property object" }] + `, + ), + // #endregion + + // #region invalid; sparse arrays + fromFixture( + stripIndent` + // combineLatest with sparse array + import { combineLatest, of } from "rxjs"; + + const a$ = of(1); + const combined$ = combineLatest([, a$]); + ~~~~~~~~~~~~~ [forbidden { "operator": "combineLatest", "inputType": "single-valued array" }] + `, + ), + fromFixture( + stripIndent` + // forkJoin with sparse array + import { forkJoin, of } from "rxjs"; + + const a$ = of(1); + const combined$ = forkJoin([, a$]); + ~~~~~~~~ [forbidden { "operator": "forkJoin", "inputType": "single-valued array" }] + `, + ), + // #endregion + + // #region invalid; getter object member + fromFixture( + stripIndent` + // combineLatest with non-property object member + import { combineLatest, of } from "rxjs"; + + const combined$ = combineLatest({ get data() { return of(1); } }); + ~~~~~~~~~~~~~ [forbidden { "operator": "combineLatest", "inputType": "single-property object" }] + `, + ), + // #endregion + + // #region invalid; namespace imports + fromFixture( + stripIndent` + // combineLatest from namespace import with single observable array + import * as Rx from "rxjs"; + + const a$ = Rx.of(1); + const combined$ = Rx.combineLatest([a$]); + ~~~~~~~~~~~~~~~~ [forbidden { "operator": "combineLatest", "inputType": "single-valued array" }] + `, + ), + fromFixture( + stripIndent` + // forkJoin from namespace import with single observable object + import * as Rx from "rxjs"; + + const source$ = Rx.of('data'); + const result$ = Rx.forkJoin({ item: source$ }); + ~~~~~~~~~~~ [forbidden { "operator": "forkJoin", "inputType": "single-property object" }] + `, + ), + // #endregion + + // #region invalid; complex expressions with single elements + fromFixture( + stripIndent` + // combineLatest with single complex expression + import { combineLatest, of } from "rxjs"; + import { map } from "rxjs/operators"; + + const combined$ = combineLatest([of(1).pipe(map(x => x * 2))]); + ~~~~~~~~~~~~~ [forbidden { "operator": "combineLatest", "inputType": "single-valued array" }] + `, + ), + fromFixture( + stripIndent` + // forkJoin with single method call result + import { forkJoin, Observable } from "rxjs"; + + function getObservable(): Observable { + return of(42); + } + + const combined$ = forkJoin([getObservable()]); + ~~~~~~~~ [forbidden { "operator": "forkJoin", "inputType": "single-valued array" }] + `, + ), + // #endregion + ], +});