Skip to content

Commit eb979fb

Browse files
JasonWeinzierlJasonWeinzierl
authored
feat(throw-error): report subjects throwing non-Errors (#215)
This change enhances `throw-error` to report passing a non-Error into `Subject.error`. ```ts import { Subject } from 'rxjs'; const subject = new Subject<void>(); // This causes the same problems as doing `throwError("Kaboom!")`. subject.error("Kaboom!"); ``` Resolves #120
1 parent f3ab04d commit eb979fb

File tree

4 files changed

+177
-18
lines changed

4 files changed

+177
-18
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,6 @@ The package includes the following rules.
120120
| [prefer-observer](docs/rules/prefer-observer.md) | Disallow passing separate handlers to `subscribe` and `tap`. | ✅ 🔒 | 🔧 | 💡 | 💭 | |
121121
| [prefer-root-operators](docs/rules/prefer-root-operators.md) | Disallow importing operators from `rxjs/operators`. | ✅ 🔒 | 🔧 | 💡 | | |
122122
| [suffix-subjects](docs/rules/suffix-subjects.md) | Enforce the use of a suffix in subject identifiers. | | | | 💭 | |
123-
| [throw-error](docs/rules/throw-error.md) | Enforce passing only `Error` values to `throwError`. | ✅ 🔒 | | | 💭 | |
123+
| [throw-error](docs/rules/throw-error.md) | Enforce passing only `Error` values to `throwError` or `Subject.error`. | ✅ 🔒 | | | 💭 | |
124124

125125
<!-- end auto-generated rules list -->

‎docs/rules/throw-error.md‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,37 @@
1-
# Enforce passing only `Error` values to `throwError` (`rxjs-x/throw-error`)
1+
# Enforce passing only `Error` values to `throwError` or `Subject.error` (`rxjs-x/throw-error`)
22

33
💼 This rule is enabled in the following configs: ✅ `recommended`, 🔒 `strict`.
44

55
💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting).
66

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

9-
This rule forbids passing values that are not `Error` objects to `throwError`.
9+
This rule forbids passing values that are not `Error` objects to `throwError` or `Subject.error`.
1010
It's similar to the typescript-eslint [`only-throw-error`](https://typescript-eslint.io/rules/only-throw-error/) rule,
11-
but is for the `throwError` Observable creation function - not `throw` statements.
11+
but is for the `throwError` Observable creation function or the `Subject.error` method - not `throw` statements.
1212

1313
## Rule details
1414

1515
Examples of **incorrect** code for this rule:
1616

1717
```ts
18-
import { throwError } from "rxjs";
18+
import { throwError, Subject } from "rxjs";
1919

2020
throwError(() => "Kaboom!");
21+
22+
const subject = new Subject<void>();
23+
subject.error("Kaboom!");
2124
```
2225

2326
Examples of **correct** code for this rule:
2427

2528
```ts
26-
import { throwError } from "rxjs";
29+
import { throwError, Subject } from "rxjs";
2730

2831
throwError(() => new Error("Kaboom!"));
32+
33+
const subject = new Subject<void>();
34+
subject.error(new Error("Kaboom!"));
2935
```
3036

3137
## Options

‎src/rules/throw-error.ts‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { TSESTree as es, ESLintUtils } from '@typescript-eslint/utils';
22
import * as tsutils from 'ts-api-utils';
33
import ts from 'typescript';
4-
import { couldBeFunction, couldBeType, getTypeServices } from '../etc';
4+
import { couldBeFunction, couldBeType, getTypeServices, isMemberExpression } from '../etc';
55
import { ruleCreator } from '../utils';
66

77
const defaultOptions: readonly {
@@ -14,7 +14,7 @@ export const throwErrorRule = ruleCreator({
1414
meta: {
1515
docs: {
1616
description:
17-
'Enforce passing only `Error` values to `throwError`.',
17+
'Enforce passing only `Error` values to `throwError` or `Subject.error`.',
1818
recommended: {
1919
recommended: true,
2020
strict: [{ allowThrowingAny: false, allowThrowingUnknown: false }],
@@ -38,15 +38,15 @@ export const throwErrorRule = ruleCreator({
3838
name: 'throw-error',
3939
create: (context) => {
4040
const { esTreeNodeToTSNodeMap, program, getTypeAtLocation } = ESLintUtils.getParserServices(context);
41-
const { couldBeObservable } = getTypeServices(context);
41+
const { couldBeObservable, couldBeSubject } = getTypeServices(context);
4242
const [config = {}] = context.options;
4343
const { allowThrowingAny = true, allowThrowingUnknown = true } = config;
4444

45-
function checkThrowArgument(node: es.Node) {
45+
function checkThrowArgument(node: es.Node, noFunction = false) {
4646
let type = getTypeAtLocation(node);
4747
let reportNode = node;
4848

49-
if (couldBeFunction(type)) {
49+
if (!noFunction && couldBeFunction(type)) {
5050
reportNode = (node as es.ArrowFunctionExpression).body ?? node;
5151

5252
const tsNode = esTreeNodeToTSNodeMap.get(node);
@@ -73,7 +73,7 @@ export const throwErrorRule = ruleCreator({
7373
});
7474
}
7575

76-
function checkNode(node: es.CallExpression) {
76+
function checkThrowError(node: es.CallExpression) {
7777
if (couldBeObservable(node)) {
7878
const [arg] = node.arguments;
7979
if (arg) {
@@ -82,13 +82,22 @@ export const throwErrorRule = ruleCreator({
8282
}
8383
}
8484

85+
function checkSubjectError(node: es.CallExpression) {
86+
if (isMemberExpression(node.callee) && couldBeSubject(node.callee.object)) {
87+
const [arg] = node.arguments;
88+
if (arg) {
89+
checkThrowArgument(arg, true);
90+
}
91+
}
92+
}
93+
8594
return {
86-
'CallExpression[callee.name=\'throwError\']': (node: es.CallExpression) => {
87-
checkNode(node);
88-
},
89-
'CallExpression[callee.property.name=\'throwError\']': (node: es.CallExpression) => {
90-
checkNode(node);
91-
},
95+
'CallExpression[callee.name=\'throwError\']':
96+
checkThrowError,
97+
'CallExpression[callee.property.name=\'throwError\']':
98+
checkThrowError,
99+
'CallExpression[callee.property.name=\'error\']':
100+
checkSubjectError,
92101
};
93102
},
94103
});

‎tests/rules/throw-error.test.ts‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { ruleTester } from '../rule-tester';
55

66
ruleTester({ types: true }).run('throw-error', throwErrorRule, {
77
valid: [
8+
// #region valid; throwError
89
stripIndent`
910
// Error
1011
import { throwError } from "rxjs";
@@ -130,6 +131,72 @@ ruleTester({ types: true }).run('throw-error', throwErrorRule, {
130131
);
131132
});
132133
`,
134+
// #endregion valid; throwError
135+
// #region valid; subject.error
136+
stripIndent`
137+
// Error
138+
import { Subject } from "rxjs";
139+
140+
const subject = new Subject<void>();
141+
142+
subject.error(new Error("Boom!"));
143+
`,
144+
stripIndent`
145+
// RangeError
146+
import { Subject } from "rxjs";
147+
148+
const subject = new Subject<void>();
149+
150+
subject.error(new RangeError("Boom!"));
151+
`,
152+
stripIndent`
153+
// DOMException
154+
/// <reference lib="dom" />
155+
import { Subject } from "rxjs";
156+
157+
const subject = new Subject<void>();
158+
159+
subject.error(new DOMException("Boom!"));
160+
`,
161+
stripIndent`
162+
// custom Error
163+
import { Subject } from "rxjs";
164+
165+
class MyFailure extends Error {}
166+
167+
const subject = new Subject<void>();
168+
169+
subject.error(new MyFailure("Boom!"));
170+
`,
171+
stripIndent`
172+
// any
173+
import { Subject } from "rxjs";
174+
175+
const subject = new Subject<void>();
176+
177+
subject.error("Boom!" as any);
178+
`,
179+
stripIndent`
180+
// unknown
181+
import { Subject } from "rxjs";
182+
183+
const subject = new Subject<void>();
184+
185+
subject.error("Boom!" as unknown);
186+
`,
187+
stripIndent`
188+
// Object.assign
189+
import { Subject } from "rxjs";
190+
191+
const subject = new Subject<void>();
192+
193+
subject.error(Object.assign(
194+
new Error("Not Found"),
195+
{ code: "NOT_FOUND" }
196+
));
197+
`,
198+
// #endregion valid; subject.error
199+
// #region valid; other
133200
stripIndent`
134201
// no signature
135202
// There will be no signature for callback and
@@ -165,8 +232,10 @@ ruleTester({ types: true }).run('throw-error', throwErrorRule, {
165232
throw error;
166233
}
167234
`,
235+
// #endregion valid; other
168236
],
169237
invalid: [
238+
// #region invalid; throwError
170239
fromFixture(
171240
stripIndent`
172241
// string
@@ -264,5 +333,80 @@ ruleTester({ types: true }).run('throw-error', throwErrorRule, {
264333
~~~~~~~ [forbidden]
265334
`,
266335
),
336+
// #endregion invalid; throwError
337+
// #region invalid; subject.error
338+
fromFixture(
339+
stripIndent`
340+
// string
341+
import { Subject } from "rxjs";
342+
343+
const subject = new Subject<void>();
344+
345+
subject.error("Boom!");
346+
~~~~~~~ [forbidden]
347+
`,
348+
),
349+
fromFixture(
350+
stripIndent`
351+
// any not allowed
352+
import { Subject } from "rxjs";
353+
354+
const subject = new Subject<void>();
355+
356+
subject.error("Boom!" as any);
357+
~~~~~~~~~~~~~~ [forbidden]
358+
`,
359+
{ options: [{ allowThrowingAny: false }] },
360+
),
361+
fromFixture(
362+
stripIndent`
363+
// unknown not allowed
364+
import { Subject } from "rxjs";
365+
366+
const subject = new Subject<void>();
367+
368+
subject.error("Boom!" as unknown);
369+
~~~~~~~~~~~~~~~~~~ [forbidden]
370+
`,
371+
{ options: [{ allowThrowingUnknown: false }] },
372+
),
373+
fromFixture(
374+
stripIndent`
375+
// falsy
376+
import { Subject } from "rxjs";
377+
378+
const subject = new Subject<void>();
379+
380+
subject.error(0);
381+
~ [forbidden]
382+
subject.error(false);
383+
~~~~~ [forbidden]
384+
subject.error(null);
385+
~~~~ [forbidden]
386+
subject.error(undefined);
387+
~~~~~~~~~ [forbidden]
388+
`,
389+
),
390+
fromFixture(
391+
stripIndent`
392+
// Object.assign with non-Error
393+
import { Subject } from "rxjs";
394+
395+
const subject = new Subject<void>();
396+
397+
subject.error(Object.assign({ message: "Not Found" }, { code: "NOT_FOUND" }));
398+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbidden]
399+
`,
400+
),
401+
fromFixture(
402+
stripIndent`
403+
// namespace import
404+
import * as Rx from "rxjs";
405+
const subject = new Rx.Subject<void>();
406+
subject.error("Boom!");
407+
~~~~~~~ [forbidden]
408+
`,
409+
),
410+
// #endregion invalid; subject.error
267411
],
268412
});

0 commit comments

Comments
 (0)