Skip to content

Commit e2e9ffc

Browse files
ronamiJoshuaKGoldberg
ronami
and
JoshuaKGoldberg
authored
feat(eslint-plugin): [no-confusing-void-expression] add an option to ignore void<->void (typescript-eslint#10067)
* initial implementation * enable rule * improvements and fixes * use checker.getContextualType over getContextualType * more fixes * improve implementation * add docs * update snapshots * enable rule with option * rename function names to be a bit less confusing * reword game-plan * move rule def to be under its proper title * remove invalid tests * add more tests * add more tests * add more tests * refactor tests * refactor tests * wording * Update packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> * Update packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> * Update packages/eslint-plugin/src/rules/no-confusing-void-expression.ts Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> * format * format * add missing test * remove code to handle multiple call signatures * figure out the case of multiple call signatures in a function expression --------- Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
1 parent ac1f632 commit e2e9ffc

File tree

9 files changed

+869
-33
lines changed

9 files changed

+869
-33
lines changed

‎eslint.config.mjs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,14 @@ export default tseslint.config(
105105
},
106106
linterOptions: { reportUnusedDisableDirectives: 'error' },
107107
rules: {
108-
// TODO: https://github.com/typescript-eslint/typescript-eslint/issues/8538
109-
'@typescript-eslint/no-confusing-void-expression': 'off',
110-
111108
//
112109
// our plugin :D
113110
//
114111

112+
'@typescript-eslint/no-confusing-void-expression': [
113+
'error',
114+
{ ignoreVoidReturningFunctions: true },
115+
],
115116
'@typescript-eslint/ban-ts-comment': [
116117
'error',
117118
{

‎packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,28 @@ function doSomething() {
123123
console.log(void alert('Hello, world!'));
124124
```
125125

126+
### `ignoreVoidReturningFunctions`
127+
128+
Whether to ignore returns from functions with `void` return types when inside a function with a `void` return type.
129+
130+
Some projects prefer allowing functions that explicitly return `void` to return `void` expressions. Doing so allows more writing more succinct functions.
131+
132+
:::note
133+
This is technically risky as the `void`-returning function might actually be returning a value not seen by the type system.
134+
:::
135+
136+
```ts option='{ "ignoreVoidReturningFunctions": true }' showPlaygroundButton
137+
function foo(): void {
138+
return console.log();
139+
}
140+
141+
function onError(callback: () => void): void {
142+
callback();
143+
}
144+
145+
onError(() => console.log('oops'));
146+
```
147+
126148
## When Not To Use It
127149

128150
The return type of a function can be inspected by going to its definition or hovering over it in an IDE.

‎packages/eslint-plugin/src/rules/no-confusing-void-expression.ts

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@ import {
1616
nullThrows,
1717
NullThrowsReasons,
1818
} from '../util';
19+
import { getParentFunctionNode } from '../util/getParentFunctionNode';
1920

2021
export type Options = [
2122
{
2223
ignoreArrowShorthand?: boolean;
2324
ignoreVoidOperator?: boolean;
25+
ignoreVoidReturningFunctions?: boolean;
2426
},
2527
];
2628

@@ -86,21 +88,28 @@ export default createRule<Options, MessageId>({
8688
description:
8789
'Whether to ignore returns that start with the `void` operator.',
8890
},
91+
ignoreVoidReturningFunctions: {
92+
type: 'boolean',
93+
description:
94+
'Whether to ignore returns from functions with explicit `void` return types and functions with contextual `void` return types.',
95+
},
8996
},
9097
},
9198
],
9299
},
93100
defaultOptions: [{ ignoreArrowShorthand: false, ignoreVoidOperator: false }],
94101

95102
create(context, [options]) {
103+
const services = getParserServices(context);
104+
const checker = services.program.getTypeChecker();
105+
96106
return {
97107
'AwaitExpression, CallExpression, TaggedTemplateExpression'(
98108
node:
99109
| TSESTree.AwaitExpression
100110
| TSESTree.CallExpression
101111
| TSESTree.TaggedTemplateExpression,
102112
): void {
103-
const services = getParserServices(context);
104113
const type = getConstrainedTypeAtLocation(services, node);
105114
if (!tsutils.isTypeFlagSet(type, ts.TypeFlags.VoidLike)) {
106115
// not a void expression
@@ -122,6 +131,14 @@ export default createRule<Options, MessageId>({
122131
if (invalidAncestor.type === AST_NODE_TYPES.ArrowFunctionExpression) {
123132
// handle arrow function shorthand
124133

134+
if (options.ignoreVoidReturningFunctions) {
135+
const returnsVoid = isVoidReturningFunctionNode(invalidAncestor);
136+
137+
if (returnsVoid) {
138+
return;
139+
}
140+
}
141+
125142
if (options.ignoreVoidOperator) {
126143
// handle wrapping with `void`
127144
return context.report({
@@ -177,6 +194,18 @@ export default createRule<Options, MessageId>({
177194
if (invalidAncestor.type === AST_NODE_TYPES.ReturnStatement) {
178195
// handle return statement
179196

197+
if (options.ignoreVoidReturningFunctions) {
198+
const functionNode = getParentFunctionNode(invalidAncestor);
199+
200+
if (functionNode) {
201+
const returnsVoid = isVoidReturningFunctionNode(functionNode);
202+
203+
if (returnsVoid) {
204+
return;
205+
}
206+
}
207+
}
208+
180209
if (options.ignoreVoidOperator) {
181210
// handle wrapping with `void`
182211
return context.report({
@@ -380,8 +409,6 @@ export default createRule<Options, MessageId>({
380409
function canFix(
381410
node: ReturnStatementWithArgument | TSESTree.ArrowFunctionExpression,
382411
): boolean {
383-
const services = getParserServices(context);
384-
385412
const targetNode =
386413
node.type === AST_NODE_TYPES.ReturnStatement
387414
? node.argument
@@ -390,5 +417,53 @@ export default createRule<Options, MessageId>({
390417
const type = getConstrainedTypeAtLocation(services, targetNode);
391418
return tsutils.isTypeFlagSet(type, ts.TypeFlags.VoidLike);
392419
}
420+
421+
function isFunctionReturnTypeIncludesVoid(functionType: ts.Type): boolean {
422+
const callSignatures = tsutils.getCallSignaturesOfType(functionType);
423+
424+
return callSignatures.some(signature => {
425+
const returnType = signature.getReturnType();
426+
427+
return tsutils
428+
.unionTypeParts(returnType)
429+
.some(tsutils.isIntrinsicVoidType);
430+
});
431+
}
432+
433+
function isVoidReturningFunctionNode(
434+
functionNode:
435+
| TSESTree.ArrowFunctionExpression
436+
| TSESTree.FunctionDeclaration
437+
| TSESTree.FunctionExpression,
438+
): boolean {
439+
// Game plan:
440+
// - If the function node has a type annotation, check if it includes `void`.
441+
// - If it does then the function is safe to return `void` expressions in.
442+
// - Otherwise, check if the function is a function-expression or an arrow-function.
443+
// - If it is, get its contextual type and bail if we cannot.
444+
// - Return based on whether the contextual type includes `void` or not
445+
446+
const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode);
447+
448+
if (functionTSNode.type) {
449+
const returnType = checker.getTypeFromTypeNode(functionTSNode.type);
450+
451+
return tsutils
452+
.unionTypeParts(returnType)
453+
.some(tsutils.isIntrinsicVoidType);
454+
}
455+
456+
if (ts.isExpression(functionTSNode)) {
457+
const functionType = checker.getContextualType(functionTSNode);
458+
459+
if (functionType) {
460+
return tsutils
461+
.unionTypeParts(functionType)
462+
.some(isFunctionReturnTypeIncludesVoid);
463+
}
464+
}
465+
466+
return false;
467+
}
393468
},
394469
});

‎packages/eslint-plugin/src/rules/no-unsafe-return.ts

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { TSESTree } from '@typescript-eslint/utils';
22

3-
import { AST_NODE_TYPES } from '@typescript-eslint/utils';
43
import * as tsutils from 'ts-api-utils';
54
import * as ts from 'typescript';
65

@@ -18,6 +17,7 @@ import {
1817
isTypeUnknownType,
1918
isUnsafeAssignment,
2019
} from '../util';
20+
import { getParentFunctionNode } from '../util/getParentFunctionNode';
2121

2222
export default createRule({
2323
name: 'no-unsafe-return',
@@ -49,31 +49,6 @@ export default createRule({
4949
'noImplicitThis',
5050
);
5151

52-
function getParentFunctionNode(
53-
node: TSESTree.Node,
54-
):
55-
| TSESTree.ArrowFunctionExpression
56-
| TSESTree.FunctionDeclaration
57-
| TSESTree.FunctionExpression
58-
| null {
59-
let current = node.parent;
60-
while (current) {
61-
if (
62-
current.type === AST_NODE_TYPES.ArrowFunctionExpression ||
63-
current.type === AST_NODE_TYPES.FunctionDeclaration ||
64-
current.type === AST_NODE_TYPES.FunctionExpression
65-
) {
66-
return current;
67-
}
68-
69-
current = current.parent;
70-
}
71-
72-
// this shouldn't happen in correct code, but someone may attempt to parse bad code
73-
// the parser won't error, so we shouldn't throw here
74-
/* istanbul ignore next */ return null;
75-
}
76-
7752
function checkReturn(
7853
returnNode: TSESTree.Node,
7954
reportingNode: TSESTree.Node = returnNode,

‎packages/eslint-plugin/src/util/getParentFunctionNode.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import type { TSESTree } from '@typescript-eslint/utils';
2+
3+
import { AST_NODE_TYPES } from '@typescript-eslint/utils';
4+
5+
export function getParentFunctionNode(
6+
node: TSESTree.Node,
7+
):
8+
| TSESTree.ArrowFunctionExpression
9+
| TSESTree.FunctionDeclaration
10+
| TSESTree.FunctionExpression
11+
| null {
12+
let current = node.parent;
13+
while (current) {
14+
if (
15+
current.type === AST_NODE_TYPES.ArrowFunctionExpression ||
16+
current.type === AST_NODE_TYPES.FunctionDeclaration ||
17+
current.type === AST_NODE_TYPES.FunctionExpression
18+
) {
19+
return current;
20+
}
21+
22+
current = current.parent;
23+
}
24+
25+
// this shouldn't happen in correct code, but someone may attempt to parse bad code
26+
// the parser won't error, so we shouldn't throw here
27+
/* istanbul ignore next */ return null;
28+
}

‎packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-void-expression.shot

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)