Skip to content

Commit ad628d2

Browse files
JasonWeinzierlJasonWeinzierl
committed
feat(no-misused-observables): void return return values
Also fixed invalid JSX.
1 parent 0a8af2c commit ad628d2

File tree

2 files changed

+193
-17
lines changed

2 files changed

+193
-17
lines changed

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

Lines changed: 117 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
1-
import { TSESTree as es, TSESLint as eslint, ESLintUtils } from '@typescript-eslint/utils';
1+
import { AST_NODE_TYPES, TSESTree as es, TSESLint as eslint, ESLintUtils } from '@typescript-eslint/utils';
22
import * as tsutils from 'ts-api-utils';
33
import ts from 'typescript';
4-
import { getTypeServices, isJSXExpressionContainer, isMethodDefinition, isPropertyDefinition } from '../etc';
4+
import {
5+
getTypeServices,
6+
isArrowFunctionExpression,
7+
isFunctionDeclaration,
8+
isFunctionExpression,
9+
isJSXExpressionContainer,
10+
isMethodDefinition,
11+
isPropertyDefinition,
12+
} from '../etc';
513
import { ruleCreator } from '../utils';
614

715
// The implementation of this rule is similar to typescript-eslint's no-misused-promises. MIT License.
@@ -55,7 +63,7 @@ export const noMisusedObservablesRule = ruleCreator({
5563
ClassExpression: checkClassLikeOrInterfaceNode,
5664
TSInterfaceDeclaration: checkClassLikeOrInterfaceNode,
5765
Property: checkProperty,
58-
// ReturnStatement: checkReturnStatement,
66+
ReturnStatement: checkReturnStatement,
5967
// AssignmentExpression: checkAssignment,
6068
// VariableDeclarator: checkVariableDeclarator,
6169
};
@@ -171,6 +179,50 @@ export const noMisusedObservablesRule = ruleCreator({
171179
});
172180
}
173181

182+
function checkReturnStatement(node: es.ReturnStatement): void {
183+
const tsNode = esTreeNodeToTSNodeMap.get(node);
184+
if (tsNode.expression === undefined || !node.argument) {
185+
return;
186+
}
187+
188+
// Optimization to avoid touching type info.
189+
function getFunctionNode() {
190+
let current: es.Node | undefined = node.parent;
191+
while (
192+
current
193+
&& !isArrowFunctionExpression(current)
194+
&& !isFunctionExpression(current)
195+
&& !isFunctionDeclaration(current)
196+
) {
197+
current = current.parent;
198+
}
199+
return current;
200+
}
201+
const functionNode = getFunctionNode();
202+
if (
203+
functionNode?.returnType
204+
&& !isPossiblyFunctionType(functionNode.returnType)
205+
) {
206+
return;
207+
}
208+
209+
const contextualType = checker.getContextualType(tsNode.expression);
210+
if (contextualType === undefined) {
211+
return;
212+
}
213+
if (!isVoidReturningFunctionType(contextualType)) {
214+
return;
215+
}
216+
if (!couldReturnObservable(node.argument)) {
217+
return;
218+
}
219+
220+
context.report({
221+
node: node.argument,
222+
messageId: 'forbiddenVoidReturnReturnValue',
223+
});
224+
}
225+
174226
return {
175227
...(checksVoidReturn ? voidReturnChecks : {}),
176228
...(checksSpreads ? spreadChecks : {}),
@@ -278,3 +330,65 @@ function getPropertyContextualType(
278330
return undefined;
279331
}
280332
}
333+
334+
/**
335+
* From no-misused-promises.
336+
*/
337+
function isPossiblyFunctionType(node: es.TSTypeAnnotation): boolean {
338+
switch (node.typeAnnotation.type) {
339+
case AST_NODE_TYPES.TSConditionalType:
340+
case AST_NODE_TYPES.TSConstructorType:
341+
case AST_NODE_TYPES.TSFunctionType:
342+
case AST_NODE_TYPES.TSImportType:
343+
case AST_NODE_TYPES.TSIndexedAccessType:
344+
case AST_NODE_TYPES.TSInferType:
345+
case AST_NODE_TYPES.TSIntersectionType:
346+
case AST_NODE_TYPES.TSQualifiedName:
347+
case AST_NODE_TYPES.TSThisType:
348+
case AST_NODE_TYPES.TSTypeOperator:
349+
case AST_NODE_TYPES.TSTypeQuery:
350+
case AST_NODE_TYPES.TSTypeReference:
351+
case AST_NODE_TYPES.TSUnionType:
352+
return true;
353+
354+
case AST_NODE_TYPES.TSTypeLiteral:
355+
return node.typeAnnotation.members.some(
356+
member =>
357+
member.type === AST_NODE_TYPES.TSCallSignatureDeclaration
358+
|| member.type === AST_NODE_TYPES.TSConstructSignatureDeclaration,
359+
);
360+
361+
case AST_NODE_TYPES.TSAbstractKeyword:
362+
case AST_NODE_TYPES.TSAnyKeyword:
363+
case AST_NODE_TYPES.TSArrayType:
364+
case AST_NODE_TYPES.TSAsyncKeyword:
365+
case AST_NODE_TYPES.TSBigIntKeyword:
366+
case AST_NODE_TYPES.TSBooleanKeyword:
367+
case AST_NODE_TYPES.TSDeclareKeyword:
368+
case AST_NODE_TYPES.TSExportKeyword:
369+
case AST_NODE_TYPES.TSIntrinsicKeyword:
370+
case AST_NODE_TYPES.TSLiteralType:
371+
case AST_NODE_TYPES.TSMappedType:
372+
case AST_NODE_TYPES.TSNamedTupleMember:
373+
case AST_NODE_TYPES.TSNeverKeyword:
374+
case AST_NODE_TYPES.TSNullKeyword:
375+
case AST_NODE_TYPES.TSNumberKeyword:
376+
case AST_NODE_TYPES.TSObjectKeyword:
377+
case AST_NODE_TYPES.TSOptionalType:
378+
case AST_NODE_TYPES.TSPrivateKeyword:
379+
case AST_NODE_TYPES.TSProtectedKeyword:
380+
case AST_NODE_TYPES.TSPublicKeyword:
381+
case AST_NODE_TYPES.TSReadonlyKeyword:
382+
case AST_NODE_TYPES.TSRestType:
383+
case AST_NODE_TYPES.TSStaticKeyword:
384+
case AST_NODE_TYPES.TSStringKeyword:
385+
case AST_NODE_TYPES.TSSymbolKeyword:
386+
case AST_NODE_TYPES.TSTemplateLiteralType:
387+
case AST_NODE_TYPES.TSTupleType:
388+
case AST_NODE_TYPES.TSTypePredicate:
389+
case AST_NODE_TYPES.TSUndefinedKeyword:
390+
case AST_NODE_TYPES.TSUnknownKeyword:
391+
case AST_NODE_TYPES.TSVoidKeyword:
392+
return false;
393+
}
394+
}

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

Lines changed: 76 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,11 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
4747
import React, { FC } from "react";
4848
4949
const Component: FC<{ foo: () => void }> = () => <div />;
50-
return (
51-
<Component foo={() => of(42)} />
52-
);
50+
const App = () => {
51+
return (
52+
<Component foo={() => of(42)} />
53+
);
54+
};
5355
`,
5456
options: [{ checksVoidReturn: false }],
5557
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
@@ -60,9 +62,11 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
6062
import React, { FC } from "react";
6163
6264
const Component: FC<{ foo: () => void }> = () => <div />;
63-
return (
64-
<Component foo={() => 42} />
65-
);
65+
const App = () => {
66+
return (
67+
<Component foo={() => 42} />
68+
);
69+
};
6670
`,
6771
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
6872
},
@@ -171,6 +175,36 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
171175
};
172176
`,
173177
// #endregion valid; void return property
178+
// #region valid; void return return value
179+
{
180+
code: stripIndent`
181+
// void return return value; explicitly allowed
182+
import { Observable, of } from "rxjs";
183+
184+
function foo(): () => void {
185+
return (): Observable<number> => of(42);
186+
}
187+
`,
188+
options: [{ checksVoidReturn: false }],
189+
},
190+
stripIndent`
191+
// void return return value; not void
192+
import { Observable, of } from "rxjs";
193+
194+
function foo(): () => Observable<number> {
195+
return (): Observable<number> => of(42);
196+
}
197+
`,
198+
stripIndent`
199+
// void return return value; unrelated
200+
function foo(): () => number {
201+
return (): number => 42;
202+
}
203+
function bar(): () => void {
204+
return (): number => 42;
205+
}
206+
`,
207+
// #endregion valid; void return return value
174208
// #region valid; spread
175209
{
176210
code: stripIndent`
@@ -248,10 +282,12 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
248282
import React, { FC } from "react";
249283
250284
const Component: FC<{ foo: () => void }> = () => <div />;
251-
return (
252-
<Component foo={(): Observable<number> => { return of(42); }} />
253-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnAttribute]
254-
);
285+
const App = () => {
286+
return (
287+
<Component foo={(): Observable<number> => { return of(42); }} />
288+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnAttribute]
289+
);
290+
};
255291
`,
256292
{
257293
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
@@ -264,10 +300,12 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
264300
import React, { FC } from "react";
265301
266302
const Component: FC<{ foo: () => void }> = () => <div />;
267-
return (
268-
<Component foo={() => of(42)} />
269-
~~~~~~~~~~~~~~ [forbiddenVoidReturnAttribute]
270-
);
303+
const App = () => {
304+
return (
305+
<Component foo={() => of(42)} />
306+
~~~~~~~~~~~~~~ [forbiddenVoidReturnAttribute]
307+
);
308+
};
271309
`,
272310
{
273311
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
@@ -659,6 +697,30 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
659697
`,
660698
),
661699
// #endregion invalid; void return property
700+
// #region invalid; void return return value
701+
fromFixture(
702+
stripIndent`
703+
// void return return value; arrow function
704+
import { Observable, of } from "rxjs";
705+
706+
function foo(): () => void {
707+
return (): Observable<number> => of(42);
708+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnReturnValue]
709+
}
710+
`,
711+
),
712+
fromFixture(
713+
stripIndent`
714+
// void return return value; function
715+
import { Observable, of } from "rxjs";
716+
717+
function foo(): () => void {
718+
return function(): Observable<number> { return of(42); };
719+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnReturnValue]
720+
}
721+
`,
722+
),
723+
// #endregion invalid; void return return value
662724
// #region invalid; spread
663725
fromFixture(
664726
stripIndent`

0 commit comments

Comments
 (0)