Skip to content

Commit 46e105f

Browse files
fix(no-ignored-error): check observer object too (#27)
Fixes lack of error when passing an observer object into `subscribe` that's lacking an `error` property. Previously this rule would only fire if passing in separate callbacks. Resolves upstream cartant#94
1 parent bda8626 commit 46e105f

File tree

2 files changed

+74
-4
lines changed

2 files changed

+74
-4
lines changed

src/rules/no-ignored-error.ts

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { TSESTree as es } from '@typescript-eslint/utils';
2-
import { getTypeServices } from '../etc';
1+
import { TSESTree as es, ESLintUtils } from '@typescript-eslint/utils';
2+
import { getTypeServices, isIdentifier, isMemberExpression, isObjectExpression, isProperty } from '../etc';
33
import { ruleCreator } from '../utils';
44

55
export const noIgnoredErrorRule = ruleCreator({
@@ -18,18 +18,59 @@ export const noIgnoredErrorRule = ruleCreator({
1818
},
1919
name: 'no-ignored-error',
2020
create: (context) => {
21+
const { getTypeAtLocation } = ESLintUtils.getParserServices(context);
2122
const { couldBeObservable, couldBeFunction } = getTypeServices(context);
2223

24+
function isMissingErrorCallback(callExpression: es.CallExpression): boolean {
25+
if (callExpression.arguments.length >= 2) {
26+
return false;
27+
}
28+
return couldBeFunction(callExpression.arguments[0]);
29+
}
30+
31+
function isObjMissingError(arg: es.ObjectExpression): boolean {
32+
return !arg.properties.some(
33+
property =>
34+
isProperty(property)
35+
&& isIdentifier(property.key)
36+
&& property.key.name === 'error',
37+
);
38+
}
39+
40+
function isTypeMissingError(arg: es.Identifier): boolean {
41+
const argType = getTypeAtLocation(arg);
42+
return !argType?.getProperties().some(p => p.name === 'error');
43+
}
44+
45+
function isMissingErrorProperty(callExpression: es.CallExpression): boolean {
46+
if (callExpression.arguments.length !== 1) {
47+
return false;
48+
}
49+
50+
const [arg] = callExpression.arguments;
51+
52+
if (isObjectExpression(arg)) {
53+
return isObjMissingError(arg);
54+
}
55+
if (isIdentifier(arg)) {
56+
return isTypeMissingError(arg);
57+
}
58+
if (isMemberExpression(arg) && isIdentifier(arg.property)) {
59+
return isTypeMissingError(arg.property);
60+
}
61+
return false;
62+
}
63+
2364
return {
2465
'CallExpression[arguments.length > 0] > MemberExpression > Identifier[name=\'subscribe\']':
2566
(node: es.Identifier) => {
2667
const memberExpression = node.parent as es.MemberExpression;
2768
const callExpression = memberExpression.parent as es.CallExpression;
2869

2970
if (
30-
callExpression.arguments.length < 2
71+
(isMissingErrorCallback(callExpression)
72+
|| isMissingErrorProperty(callExpression))
3173
&& couldBeObservable(memberExpression.object)
32-
&& couldBeFunction(callExpression.arguments[0])
3374
) {
3475
context.report({
3576
messageId: 'forbidden',

tests/rules/no-ignored-error.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,19 @@ ruleTester({ types: true }).run('no-ignored-error', noIgnoredErrorRule, {
1111
const observable = of([1, 2]);
1212
observable.subscribe(() => {}, () => {});
1313
`,
14+
stripIndent`
15+
// observer argument
16+
import { of } from "rxjs";
17+
18+
const observable = of([1, 2]);
19+
observable.subscribe({ next: () => {}, error: () => {} });
20+
21+
const observer1 = { next: () => {}, error: () => {} };
22+
observable.subscribe(observer1);
23+
24+
const obj = { observer: observer1 };
25+
observable.subscribe(obj.observer);
26+
`,
1427
stripIndent`
1528
// subject
1629
import { Subject } from "rxjs";
@@ -85,5 +98,21 @@ ruleTester({ types: true }).run('no-ignored-error', noIgnoredErrorRule, {
8598
}
8699
`,
87100
),
101+
fromFixture(
102+
stripIndent`
103+
// observer argument
104+
import { of } from "rxjs";
105+
106+
const observable = of([1, 2]);
107+
observable.subscribe({ next: () => {} });
108+
~~~~~~~~~ [forbidden]
109+
const observer1 = { next: () => {} };
110+
observable.subscribe(observer1);
111+
~~~~~~~~~ [forbidden]
112+
const obj = { observer: observer1 };
113+
observable.subscribe(obj.observer1);
114+
~~~~~~~~~ [forbidden]
115+
`,
116+
),
88117
],
89118
});

0 commit comments

Comments
 (0)