Skip to content

feat(eslint-plugin): [no-confusing-void-expression] add an option to ignore void<->void #10067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
27126ca
initial implementation
ronami Sep 27, 2024
98830b9
enable rule
ronami Sep 27, 2024
3830a24
improvements and fixes
ronami Sep 27, 2024
5b9655d
use checker.getContextualType over getContextualType
ronami Sep 27, 2024
e47395e
more fixes
ronami Sep 27, 2024
4d0bf7c
improve implementation
ronami Sep 27, 2024
117d244
add docs
ronami Sep 27, 2024
b4be799
update snapshots
ronami Sep 27, 2024
1c49780
enable rule with option
ronami Sep 27, 2024
5dc7600
rename function names to be a bit less confusing
ronami Sep 27, 2024
2c26323
reword game-plan
ronami Sep 27, 2024
6800dba
move rule def to be under its proper title
ronami Sep 27, 2024
5f7833f
remove invalid tests
ronami Sep 27, 2024
37201b8
add more tests
ronami Sep 27, 2024
5e235c8
add more tests
ronami Sep 27, 2024
a8ca492
add more tests
ronami Sep 27, 2024
60dc04c
refactor tests
ronami Sep 27, 2024
3b66043
refactor tests
ronami Sep 27, 2024
b788954
wording
ronami Sep 27, 2024
d735247
merge conflicts
ronami Oct 11, 2024
c5f5e12
Update packages/eslint-plugin/docs/rules/no-confusing-void-expression…
ronami Oct 11, 2024
b61ab1f
Update packages/eslint-plugin/docs/rules/no-confusing-void-expression…
ronami Oct 11, 2024
af9bf87
Update packages/eslint-plugin/src/rules/no-confusing-void-expression.ts
ronami Oct 11, 2024
7ac4a37
format
ronami Oct 11, 2024
dcbb392
format
ronami Oct 11, 2024
1a52c22
add missing test
ronami Oct 11, 2024
f190b15
remove code to handle multiple call signatures
ronami Oct 11, 2024
8b85c14
figure out the case of multiple call signatures in a function expression
ronami Oct 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,14 @@ export default tseslint.config(
},
linterOptions: { reportUnusedDisableDirectives: 'error' },
rules: {
// TODO: https://github.com/typescript-eslint/typescript-eslint/issues/8538
'@typescript-eslint/no-confusing-void-expression': 'off',

//
// our plugin :D
//

'@typescript-eslint/no-confusing-void-expression': [
'error',
{ ignoreVoidReturningFunctions: true },
],
'@typescript-eslint/ban-ts-comment': [
'error',
{
Expand Down
22 changes: 22 additions & 0 deletions packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,28 @@ function doSomething() {
console.log(void alert('Hello, world!'));
```

### `ignoreVoidReturningFunctions`

Whether to ignore returns from functions with `void` return types when inside a function with a `void` return type.

Some projects prefer allowing functions that explicitly return `void` to return `void` expressions. Doing so allows more writing more succinct functions.

:::note
This is technically risky as the `void`-returning function might actually be returning a value not seen by the type system.
:::

```ts option='{ "ignoreVoidReturningFunctions": true }' showPlaygroundButton
function foo(): void {
return console.log();
}

function onError(callback: () => void): void {
callback();
}

onError(() => console.log('oops'));
```

## When Not To Use It

The return type of a function can be inspected by going to its definition or hovering over it in an IDE.
Expand Down
81 changes: 78 additions & 3 deletions packages/eslint-plugin/src/rules/no-confusing-void-expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ import {
nullThrows,
NullThrowsReasons,
} from '../util';
import { getParentFunctionNode } from '../util/getParentFunctionNode';

export type Options = [
{
ignoreArrowShorthand?: boolean;
ignoreVoidOperator?: boolean;
ignoreVoidReturningFunctions?: boolean;
},
];

Expand Down Expand Up @@ -86,21 +88,28 @@ export default createRule<Options, MessageId>({
description:
'Whether to ignore returns that start with the `void` operator.',
},
ignoreVoidReturningFunctions: {
type: 'boolean',
description:
'Whether to ignore returns from functions with explicit `void` return types and functions with contextual `void` return types.',
},
},
},
],
},
defaultOptions: [{ ignoreArrowShorthand: false, ignoreVoidOperator: false }],

create(context, [options]) {
const services = getParserServices(context);
const checker = services.program.getTypeChecker();

return {
'AwaitExpression, CallExpression, TaggedTemplateExpression'(
node:
| TSESTree.AwaitExpression
| TSESTree.CallExpression
| TSESTree.TaggedTemplateExpression,
): void {
const services = getParserServices(context);
const type = getConstrainedTypeAtLocation(services, node);
if (!tsutils.isTypeFlagSet(type, ts.TypeFlags.VoidLike)) {
// not a void expression
Expand All @@ -122,6 +131,14 @@ export default createRule<Options, MessageId>({
if (invalidAncestor.type === AST_NODE_TYPES.ArrowFunctionExpression) {
// handle arrow function shorthand

if (options.ignoreVoidReturningFunctions) {
const returnsVoid = isVoidReturningFunctionNode(invalidAncestor);

if (returnsVoid) {
return;
}
}

if (options.ignoreVoidOperator) {
// handle wrapping with `void`
return context.report({
Expand Down Expand Up @@ -177,6 +194,18 @@ export default createRule<Options, MessageId>({
if (invalidAncestor.type === AST_NODE_TYPES.ReturnStatement) {
// handle return statement

if (options.ignoreVoidReturningFunctions) {
const functionNode = getParentFunctionNode(invalidAncestor);

if (functionNode) {
const returnsVoid = isVoidReturningFunctionNode(functionNode);

if (returnsVoid) {
return;
}
}
}

if (options.ignoreVoidOperator) {
// handle wrapping with `void`
return context.report({
Expand Down Expand Up @@ -380,8 +409,6 @@ export default createRule<Options, MessageId>({
function canFix(
node: ReturnStatementWithArgument | TSESTree.ArrowFunctionExpression,
): boolean {
const services = getParserServices(context);

const targetNode =
node.type === AST_NODE_TYPES.ReturnStatement
? node.argument
Expand All @@ -390,5 +417,53 @@ export default createRule<Options, MessageId>({
const type = getConstrainedTypeAtLocation(services, targetNode);
return tsutils.isTypeFlagSet(type, ts.TypeFlags.VoidLike);
}

function isFunctionReturnTypeIncludesVoid(functionType: ts.Type): boolean {
const callSignatures = tsutils.getCallSignaturesOfType(functionType);

return callSignatures.some(signature => {
const returnType = signature.getReturnType();

return tsutils
.unionTypeParts(returnType)
.some(tsutils.isIntrinsicVoidType);
});
}

function isVoidReturningFunctionNode(
functionNode:
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression,
): boolean {
// Game plan:
// - If the function node has a type annotation, check if it includes `void`.
// - If it does then the function is safe to return `void` expressions in.
// - Otherwise, check if the function is a function-expression or an arrow-function.
// - If it is, get its contextual type and bail if we cannot.
// - Return based on whether the contextual type includes `void` or not

const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode);

if (functionTSNode.type) {
const returnType = checker.getTypeFromTypeNode(functionTSNode.type);

return tsutils
.unionTypeParts(returnType)
.some(tsutils.isIntrinsicVoidType);
}

if (ts.isExpression(functionTSNode)) {
const functionType = checker.getContextualType(functionTSNode);

if (functionType) {
return tsutils
.unionTypeParts(functionType)
.some(isFunctionReturnTypeIncludesVoid);
}
}

return false;
}
},
});
27 changes: 1 addition & 26 deletions packages/eslint-plugin/src/rules/no-unsafe-return.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { TSESTree } from '@typescript-eslint/utils';

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

Expand All @@ -18,6 +17,7 @@ import {
isTypeUnknownType,
isUnsafeAssignment,
} from '../util';
import { getParentFunctionNode } from '../util/getParentFunctionNode';

export default createRule({
name: 'no-unsafe-return',
Expand Down Expand Up @@ -49,31 +49,6 @@ export default createRule({
'noImplicitThis',
);

function getParentFunctionNode(
node: TSESTree.Node,
):
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression
| null {
let current = node.parent;
while (current) {
if (
current.type === AST_NODE_TYPES.ArrowFunctionExpression ||
current.type === AST_NODE_TYPES.FunctionDeclaration ||
current.type === AST_NODE_TYPES.FunctionExpression
) {
return current;
}

current = current.parent;
}

// this shouldn't happen in correct code, but someone may attempt to parse bad code
// the parser won't error, so we shouldn't throw here
/* istanbul ignore next */ return null;
}

function checkReturn(
returnNode: TSESTree.Node,
reportingNode: TSESTree.Node = returnNode,
Expand Down
28 changes: 28 additions & 0 deletions packages/eslint-plugin/src/util/getParentFunctionNode.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Praise] I like the reuse here 🙂 nice.

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import type { TSESTree } from '@typescript-eslint/utils';

import { AST_NODE_TYPES } from '@typescript-eslint/utils';

export function getParentFunctionNode(
node: TSESTree.Node,
):
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression
| null {
let current = node.parent;
while (current) {
if (
current.type === AST_NODE_TYPES.ArrowFunctionExpression ||
current.type === AST_NODE_TYPES.FunctionDeclaration ||
current.type === AST_NODE_TYPES.FunctionExpression
) {
return current;
}

current = current.parent;
}

// this shouldn't happen in correct code, but someone may attempt to parse bad code
// the parser won't error, so we shouldn't throw here
/* istanbul ignore next */ return null;
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading