Skip to content

Commit 41c7be8

Browse files
feat(no-misused-observables): new rule (#58)
New rule based on `@typescript-eslint/no-misused-promises`. Ensures you don't return an Observable where void is expected, or try to spread an Observable. This rule is added to the `strict` configuration. Resolves #43 . Note: `checksVoidReturn` is noticeably slow, specifically stemming from the `arguments`, `properties`, and `variables` sub-options. It's still faster than `@typescript-eslint/no-deprecated` and some `import-x` rules, but it's in the top 10 with `TIMING=1` enabled. But since the implementation is similar to `@typescript-eslint/no-misused-promises` which has similar performance, we'll allow it.
1 parent b122a8f commit 41c7be8

File tree

10 files changed

+1482
-8
lines changed

10 files changed

+1482
-8
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ The package includes the following rules.
100100
| [no-implicit-any-catch](docs/rules/no-implicit-any-catch.md) | Disallow implicit `any` error parameters in `catchError` operators. | ✅ 🔒 | 🔧 | 💡 | 💭 | |
101101
| [no-index](docs/rules/no-index.md) | Disallow importing index modules. | ✅ 🔒 | | | | |
102102
| [no-internal](docs/rules/no-internal.md) | Disallow importing internal modules. | ✅ 🔒 | 🔧 | 💡 | | |
103+
| [no-misused-observables](docs/rules/no-misused-observables.md) | Disallow Observables in places not designed to handle them. | 🔒 | | | 💭 | |
103104
| [no-nested-subscribe](docs/rules/no-nested-subscribe.md) | Disallow calling `subscribe` within a `subscribe` callback. | ✅ 🔒 | | | 💭 | |
104105
| [no-redundant-notify](docs/rules/no-redundant-notify.md) | Disallow sending redundant notifications from completed or errored observables. | ✅ 🔒 | | | 💭 | |
105106
| [no-sharereplay](docs/rules/no-sharereplay.md) | Disallow unsafe `shareReplay` usage. | ✅ 🔒 | | | | |

docs/rules/no-floating-observables.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ This rule will report observable-valued statements that are not treated in one o
1919

2020
> [!TIP]
2121
> `no-floating-observables` only detects apparently unhandled observable _statements_.
22+
> See [`no-misused-observables`](./no-misused-observables.md) for detecting code that provides observables to _logical_ locations
2223
2324
## Rule details
2425

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# Disallow Observables in places not designed to handle them (`rxjs-x/no-misused-observables`)
2+
3+
💼 This rule is enabled in the 🔒 `strict` config.
4+
5+
💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting).
6+
7+
<!-- end auto-generated rule header -->
8+
9+
This rule forbids providing observables to logical locations where the TypeScript compiler allows them but they are not handled properly.
10+
These situations can often arise due to a misunderstanding of the way observables are handled.
11+
12+
> [!TIP]
13+
> `no-misused-observables` only detects code that provides observables to incorrect _logical_ locations.
14+
> See [`no-floating-observables`](./no-floating-observables.md) for detecting unhandled observable _statements_.
15+
16+
This rule is like [no-misused-promises](https://typescript-eslint.io/rules/no-misused-promises) but for Observables.
17+
18+
> [!NOTE]
19+
> Unlike `@typescript-eslint/no-misused-promises`, this rule does not check conditionals like `if` statements.
20+
> Use `@typescript-eslint/no-unnecessary-condition` for linting those situations.
21+
22+
## Rule details
23+
24+
Examples of **incorrect** code for this rule:
25+
26+
```ts
27+
import { of } from "rxjs";
28+
29+
[1, 2, 3].forEach(i => of(i));
30+
31+
interface MySyncInterface {
32+
foo(): void;
33+
}
34+
class MyRxClass implements MySyncInterface {
35+
foo(): Observable<number> {
36+
return of(42);
37+
}
38+
}
39+
40+
const a = of(42);
41+
const b = { ...b };
42+
```
43+
44+
Examples of **correct** code for this rule:
45+
46+
```ts
47+
import { of } from "rxjs";
48+
49+
[1, 2, 3].map(i => of(i));
50+
51+
interface MyRxInterface {
52+
foo(): Observable<number>;
53+
}
54+
class MyRxClass implements MyRxInterface {
55+
foo(): Observable<number> {
56+
return of(42);
57+
}
58+
}
59+
```
60+
61+
## Options
62+
63+
<!-- WARNING: not auto-generated! -->
64+
65+
| Name | Description | Type | Default |
66+
| :----------------- | :-------------------------------------------------------------------------- | :------ | :------ |
67+
| `checksSpreads` | Disallow `...` spreading an Observable. | Boolean | `true` |
68+
| `checksVoidReturn` | Disallow returning an Observable from a function typed as returning `void`. | Object | `true` |
69+
70+
### `checksVoidReturn`
71+
72+
You can disable selective parts of the `checksVoidReturn` option. The following sub-options are supported:
73+
74+
| Name | Description | Type | Default |
75+
| :----------------- | :--------------------------------------------------------------------------------------------------------------------------------------- | :------ | :------ |
76+
| `arguments` | Disallow passing an Observable-returning function as an argument where the parameter type expects a function that returns `void`. | Boolean | `true` |
77+
| `attributes` | Disallow passing an Observable-returning function as a JSX attribute expected to be a function that returns `void`. | Boolean | `true` |
78+
| `inheritedMethods` | Disallow providing an Observable-returning function where a function that returns `void` is expected by an extended or implemented type. | Boolean | `true` |
79+
| `properties` | Disallow providing an Observable-returning function where a function that returns `void` is expected by a property. | Boolean | `true` |
80+
| `returns` | Disallow returning an Observable-returning function where a function that returns `void` is expected. | Boolean | `true` |
81+
| `variables` | Disallow assigning or declaring an Observable-returning function where a function that returns `void` is expected. | Boolean | `true` |
82+
83+
## Further reading
84+
85+
- [TypeScript void function assignability](https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-functions-returning-non-void-assignable-to-function-returning-void)

src/configs/strict.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export const createStrictConfig = (
2222
}],
2323
'rxjs-x/no-index': 'error',
2424
'rxjs-x/no-internal': 'error',
25+
'rxjs-x/no-misused-observables': 'error',
2526
'rxjs-x/no-nested-subscribe': 'error',
2627
'rxjs-x/no-redundant-notify': 'error',
2728
'rxjs-x/no-sharereplay': 'error',

src/etc/get-type-services.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,16 @@ export function getTypeServices<
3838
|| ts.isMethodDeclaration(tsNode)
3939
|| ts.isFunctionExpression(tsNode)
4040
) {
41-
tsTypeNode = tsNode.type ?? tsNode.body;
41+
tsTypeNode = tsNode.type ?? tsNode.body; // TODO(#57): this doesn't work for Block bodies.
4242
} else if (
4343
ts.isCallSignatureDeclaration(tsNode)
4444
|| ts.isMethodSignature(tsNode)
4545
) {
4646
tsTypeNode = tsNode.type;
47+
} else if (
48+
ts.isPropertySignature(tsNode)
49+
) {
50+
// TODO(#66): this doesn't work for functions assigned to class properties, variables, params.
4751
}
4852
return Boolean(
4953
tsTypeNode

src/etc/is.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ export function isImportSpecifier(node: TSESTree.Node): node is TSESTree.ImportS
7878
return node.type === AST_NODE_TYPES.ImportSpecifier;
7979
}
8080

81+
export function isJSXExpressionContainer(node: TSESTree.Node): node is TSESTree.JSXExpressionContainer {
82+
return node.type === AST_NODE_TYPES.JSXExpressionContainer;
83+
}
84+
8185
export function isLiteral(node: TSESTree.Node): node is TSESTree.Literal {
8286
return node.type === AST_NODE_TYPES.Literal;
8387
}
@@ -86,6 +90,10 @@ export function isMemberExpression(node: TSESTree.Node): node is TSESTree.Member
8690
return node.type === AST_NODE_TYPES.MemberExpression;
8791
}
8892

93+
export function isMethodDefinition(node: TSESTree.Node): node is TSESTree.MethodDefinition {
94+
return node.type === AST_NODE_TYPES.MethodDefinition;
95+
}
96+
8997
export function isNewExpression(node: TSESTree.Node): node is TSESTree.NewExpression {
9098
return node.type === AST_NODE_TYPES.NewExpression;
9199
}
@@ -106,6 +114,10 @@ export function isProperty(node: TSESTree.Node): node is TSESTree.Property {
106114
return node.type === AST_NODE_TYPES.Property;
107115
}
108116

117+
export function isPropertyDefinition(node: TSESTree.Node): node is TSESTree.PropertyDefinition {
118+
return node.type === AST_NODE_TYPES.PropertyDefinition;
119+
}
120+
109121
export function isPrivateIdentifier(
110122
node: TSESTree.Node,
111123
): node is TSESTree.PrivateIdentifier {

src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { noIgnoredTakewhileValueRule } from './rules/no-ignored-takewhile-value'
2727
import { noImplicitAnyCatchRule } from './rules/no-implicit-any-catch';
2828
import { noIndexRule } from './rules/no-index';
2929
import { noInternalRule } from './rules/no-internal';
30+
import { noMisusedObservablesRule } from './rules/no-misused-observables';
3031
import { noNestedSubscribeRule } from './rules/no-nested-subscribe';
3132
import { noRedundantNotifyRule } from './rules/no-redundant-notify';
3233
import { noSharereplayRule } from './rules/no-sharereplay';
@@ -74,6 +75,7 @@ const plugin = {
7475
'no-implicit-any-catch': noImplicitAnyCatchRule,
7576
'no-index': noIndexRule,
7677
'no-internal': noInternalRule,
78+
'no-misused-observables': noMisusedObservablesRule,
7779
'no-nested-subscribe': noNestedSubscribeRule,
7880
'no-redundant-notify': noRedundantNotifyRule,
7981
'no-sharereplay': noSharereplayRule,

0 commit comments

Comments
 (0)