Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. | ✅ 🔒 | | | 💭 | |
Expand Down
59 changes: 59 additions & 0 deletions docs/rules/no-unnecessary-collection.md
Original file line number Diff line number Diff line change
@@ -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.

<!-- end auto-generated rule header -->

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)
1 change: 1 addition & 0 deletions src/configs/strict.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
13 changes: 13 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
107 changes: 107 additions & 0 deletions src/rules/no-unnecessary-collection.ts
Original file line number Diff line number Diff line change
@@ -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<string, (node: es.CallExpression) => 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;
},
});
Loading