Skip to content

Commit d5348ad

Browse files
JasonWeinzierlJasonWeinzierl
committed
feat(no-misused-observables): check class declarations
1 parent 1735e04 commit d5348ad

File tree

3 files changed

+250
-2
lines changed

3 files changed

+250
-2
lines changed

‎src/etc/is.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ export function isMemberExpression(node: TSESTree.Node): node is TSESTree.Member
9090
return node.type === AST_NODE_TYPES.MemberExpression;
9191
}
9292

93+
export function isMethodDefinition(node: TSESTree.Node): node is TSESTree.MethodDefinition {
94+
return node.type === AST_NODE_TYPES.MethodDefinition;
95+
}
96+
9397
export function isNewExpression(node: TSESTree.Node): node is TSESTree.NewExpression {
9498
return node.type === AST_NODE_TYPES.NewExpression;
9599
}
@@ -110,6 +114,10 @@ export function isProperty(node: TSESTree.Node): node is TSESTree.Property {
110114
return node.type === AST_NODE_TYPES.Property;
111115
}
112116

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

‎src/rules/no-misused-observables.ts‎

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

77
// The implementation of this rule is similar to typescript-eslint's no-misused-promises. MIT License.
@@ -51,7 +51,7 @@ export const noMisusedObservablesRule = ruleCreator({
5151
CallExpression: checkArguments,
5252
NewExpression: checkArguments,
5353
JSXAttribute: checkJSXAttribute,
54-
// ClassDeclaration: checkClassLikeOrInterfaceNode,
54+
ClassDeclaration: checkClassLikeOrInterfaceNode,
5555
// ClassExpression: checkClassLikeOrInterfaceNode,
5656
// TSInterfaceDeclaration: checkClassLikeOrInterfaceNode,
5757
// Property: checkProperty,
@@ -105,6 +105,51 @@ export const noMisusedObservablesRule = ruleCreator({
105105
}
106106
}
107107

108+
function checkClassLikeOrInterfaceNode(
109+
node: es.ClassDeclaration | es.ClassExpression | es.TSInterfaceDeclaration,
110+
): void {
111+
const tsNode = esTreeNodeToTSNodeMap.get(node);
112+
113+
const heritageTypes = getHeritageTypes(checker, tsNode);
114+
if (!heritageTypes?.length) {
115+
return;
116+
}
117+
118+
for (const element of node.body.body) {
119+
const tsElement = esTreeNodeToTSNodeMap.get(element);
120+
const memberName = tsElement?.name?.getText();
121+
if (memberName === undefined) {
122+
// See comment in typescript-eslint no-misused-promises for why.
123+
continue;
124+
}
125+
126+
if (!couldReturnObservable(element)) {
127+
continue;
128+
}
129+
130+
if (isStaticMember(element)) {
131+
continue;
132+
}
133+
134+
for (const heritageType of heritageTypes) {
135+
const heritageMember = getMemberIfExists(heritageType, memberName);
136+
if (heritageMember === undefined) {
137+
continue;
138+
}
139+
const memberType = checker.getTypeOfSymbolAtLocation(heritageMember, tsElement);
140+
if (!isVoidReturningFunctionType(memberType)) {
141+
continue;
142+
}
143+
144+
context.report({
145+
messageId: 'forbiddenVoidReturnInheritedMethod',
146+
node: element,
147+
data: { heritageTypeName: checker.typeToString(heritageType) },
148+
});
149+
}
150+
}
151+
}
152+
108153
return {
109154
...(checksVoidReturn ? voidReturnChecks : {}),
110155
...(checksSpreads ? spreadChecks : {}),
@@ -156,3 +201,26 @@ function isVoidReturningFunctionType(
156201

157202
return hasVoidReturn;
158203
}
204+
205+
function getHeritageTypes(
206+
checker: ts.TypeChecker,
207+
tsNode: ts.ClassDeclaration | ts.ClassExpression | ts.InterfaceDeclaration,
208+
): ts.Type[] | undefined {
209+
return tsNode.heritageClauses
210+
?.flatMap(clause => clause.types)
211+
.map(typeExpressions => checker.getTypeAtLocation(typeExpressions));
212+
}
213+
214+
function getMemberIfExists(
215+
type: ts.Type,
216+
memberName: string,
217+
): ts.Symbol | undefined {
218+
const escapedMemberName = ts.escapeLeadingUnderscores(memberName);
219+
const symbolMemberMatch = type.getSymbol()?.members?.get(escapedMemberName);
220+
return symbolMemberMatch ?? tsutils.getPropertyOfType(type, escapedMemberName);
221+
}
222+
223+
function isStaticMember(node: es.Node): boolean {
224+
return (isMethodDefinition(node) || isPropertyDefinition(node))
225+
&& node.static;
226+
}

‎tests/rules/no-misused-observables.test.ts‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,33 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
6767
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
6868
},
6969
// #endregion valid; void return attribute
70+
// #region valid; void return inherited method
71+
{
72+
code: stripIndent`
73+
// void return inherited method; explicitly allowed
74+
import { Observable, of } from "rxjs";
75+
76+
class Foo {
77+
foo(): void {}
78+
}
79+
80+
class Bar extends Foo {
81+
foo(): Observable<number> { return of(42); }
82+
}
83+
`,
84+
options: [{ checksVoidReturn: false }],
85+
},
86+
stripIndent`
87+
// void return inherited method; unrelated
88+
class Foo {
89+
foo(): void {}
90+
}
91+
92+
class Bar extends Foo {
93+
foo(): number { return 42; }
94+
}
95+
`,
96+
// #endregion valid; void return inherited method
7097
// #region valid; spread
7198
{
7299
code: stripIndent`
@@ -170,6 +197,151 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
170197
},
171198
),
172199
// #endregion invalid; void return attribute
200+
// #region invalid; void return inherited method
201+
fromFixture(
202+
stripIndent`
203+
// void return inherited method; class declaration; extends
204+
import { Observable, of } from "rxjs";
205+
206+
class Foo {
207+
foo(): void {}
208+
}
209+
210+
class Bar extends Foo {
211+
foo(): Observable<number> { return of(42); }
212+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Foo" }]
213+
}
214+
`,
215+
),
216+
fromFixture(
217+
stripIndent`
218+
// void return inherited method; class declaration; abstract extends
219+
import { Observable } from "rxjs";
220+
221+
class Foo {
222+
foo(): void {}
223+
}
224+
225+
abstract class Bar extends Foo {
226+
abstract foo(): Observable<number>;
227+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Foo" }]
228+
}
229+
`,
230+
),
231+
fromFixture(
232+
stripIndent`
233+
// void return inherited method; class declaration; extends abstract
234+
import { Observable, of } from "rxjs";
235+
236+
abstract class Foo {
237+
abstract foo(): void;
238+
}
239+
240+
class Bar extends Foo {
241+
foo(): Observable<number> { return of(42); }
242+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Foo" }]
243+
}
244+
`,
245+
),
246+
fromFixture(
247+
stripIndent`
248+
// void return inherited method; class declaration; abstract extends abstract
249+
import { Observable } from "rxjs";
250+
251+
abstract class Foo {
252+
abstract foo(): void;
253+
}
254+
255+
abstract class Bar extends Foo {
256+
abstract foo(): Observable<number>;
257+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Foo" }]
258+
}
259+
`,
260+
),
261+
fromFixture(
262+
stripIndent`
263+
// void return inherited method; class declaration; implements
264+
import { Observable, of } from "rxjs";
265+
266+
interface Foo {
267+
foo(): void;
268+
}
269+
270+
class Bar implements Foo {
271+
foo(): Observable<number> { return of(42); }
272+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Foo" }]
273+
}
274+
`,
275+
),
276+
fromFixture(
277+
stripIndent`
278+
// void return inherited method; class declaration; abstract implements
279+
import { Observable } from "rxjs";
280+
281+
interface Foo {
282+
foo(): void;
283+
}
284+
285+
abstract class Bar implements Foo {
286+
abstract foo(): Observable<number>;
287+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Foo" }]
288+
}
289+
`,
290+
),
291+
fromFixture(
292+
stripIndent`
293+
// void return inherited method; class declaration; implements type intersection
294+
import { Observable, of } from "rxjs";
295+
296+
type Foo = { foo(): void } & { bar(): void };
297+
298+
class Bar implements Foo {
299+
foo(): Observable<number> { return of(42); }
300+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Foo" }]
301+
bar(): void {}
302+
}
303+
`,
304+
),
305+
fromFixture(
306+
stripIndent`
307+
// void return inherited method; class declaration; extends and implements
308+
import { Observable, of } from "rxjs";
309+
310+
interface Foo {
311+
foo(): Observable<void>;
312+
}
313+
314+
interface Bar {
315+
foo(): void;
316+
}
317+
318+
class Baz {
319+
foo(): void {}
320+
}
321+
322+
class Qux extends Baz implements Foo, Bar {
323+
foo(): Observable<void> { return of(42); }
324+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Baz" }]
325+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Bar" }]
326+
}
327+
`,
328+
),
329+
fromFixture(
330+
stripIndent`
331+
// void return inherited method; class declaration; extends class expression
332+
import { Observable, of } from "rxjs";
333+
334+
const Foo = class {
335+
foo(): void {}
336+
}
337+
338+
class Bar extends Foo {
339+
foo(): Observable<number> { return of(42); }
340+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnInheritedMethod { "heritageTypeName": "Foo" }]
341+
}
342+
`,
343+
),
344+
// #endregion invalid; void return inherited method
173345
// #region invalid; spread
174346
fromFixture(
175347
stripIndent`

0 commit comments

Comments
 (0)