Skip to content

Commit

Permalink
fix(eslint-plugin): [no-invalid-void-type] don't flag function overlo…
Browse files Browse the repository at this point in the history
…ading with `void` in return type annotation (#10422)

* initial implementation

* add block test

* refactor

* refactor

* use a recursive function to handle wrapped return types

* report on method definitions that aren't overloads

* handle export default declaration

* handle named export declaration

* refactor

* fix test

* use nullThrows instead of non-null assertions

* add stricter parent types for various ast nodes and refactor accordingly

* fix existing missing code coverage

* refactor

* move and rename hasOverloadSignatures

* test void in non-implementation position

* adjust hasOverloadSignatures to require a method definition with a body

* expand comment a bit

* Revert "adjust hasOverloadSignatures to require a method definition with a body"

This reverts commit 8dd4707.

* remove unnecessary comment

* adjust hasOverloadSignatures to require a method definition with a body

* Revert "adjust hasOverloadSignatures to require a method definition with a body"

This reverts commit 350e82f.

* refactor
  • Loading branch information
ronami authored Feb 24, 2025
1 parent f22de04 commit b688380
Show file tree
Hide file tree
Showing 4 changed files with 357 additions and 2 deletions.
38 changes: 37 additions & 1 deletion packages/eslint-plugin/src/rules/no-invalid-void-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { TSESTree } from '@typescript-eslint/utils';

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

import { createRule } from '../util';
import { createRule, hasOverloadSignatures } from '../util';

export interface Options {
allowAsThisParameter?: boolean;
Expand Down Expand Up @@ -203,6 +203,20 @@ export default createRule<[Options], MessageIds>({
return;
}

// using `void` as part of the return type of function overloading implementation
if (node.parent.type === AST_NODE_TYPES.TSUnionType) {
const declaringFunction = getParentFunctionDeclarationNode(
node.parent,
);

if (
declaringFunction &&
hasOverloadSignatures(declaringFunction, context)
) {
return;
}
}

// this parameter is ok to be void.
if (
allowAsThisParameter &&
Expand Down Expand Up @@ -246,3 +260,25 @@ function getNotReturnOrGenericMessageId(
? 'invalidVoidUnionConstituent'
: 'invalidVoidNotReturnOrGeneric';
}

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

if (
current.type === AST_NODE_TYPES.MethodDefinition &&
current.value.body != null
) {
return current;
}

current = current.parent;
}

return null;
}
72 changes: 72 additions & 0 deletions packages/eslint-plugin/src/util/hasOverloadSignatures.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import type { TSESTree } from '@typescript-eslint/utils';
import type { RuleContext } from '@typescript-eslint/utils/ts-eslint';

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

import { getStaticMemberAccessValue } from './misc';

/**
* @return `true` if the function or method node has overload signatures.
*/
export function hasOverloadSignatures(
node: TSESTree.FunctionDeclaration | TSESTree.MethodDefinition,
context: RuleContext<string, unknown[]>,
): boolean {
// `export default function () {}`
if (node.parent.type === AST_NODE_TYPES.ExportDefaultDeclaration) {
return node.parent.parent.body.some(member => {
return (
member.type === AST_NODE_TYPES.ExportDefaultDeclaration &&
member.declaration.type === AST_NODE_TYPES.TSDeclareFunction
);
});
}

// `export function f() {}`
if (node.parent.type === AST_NODE_TYPES.ExportNamedDeclaration) {
return node.parent.parent.body.some(member => {
return (
member.type === AST_NODE_TYPES.ExportNamedDeclaration &&
member.declaration?.type === AST_NODE_TYPES.TSDeclareFunction &&
getFunctionDeclarationName(member.declaration, context) ===
getFunctionDeclarationName(node, context)
);
});
}

// either:
// - `function f() {}`
// - `class T { foo() {} }`

const nodeKey = getFunctionDeclarationName(node, context);

return node.parent.body.some(member => {
return (
(member.type === AST_NODE_TYPES.TSDeclareFunction ||
(member.type === AST_NODE_TYPES.MethodDefinition &&
member.value.body == null)) &&
nodeKey === getFunctionDeclarationName(member, context)
);
});
}

function getFunctionDeclarationName(
node:
| TSESTree.FunctionDeclaration
| TSESTree.MethodDefinition
| TSESTree.TSDeclareFunction,
context: RuleContext<string, unknown[]>,
): string | symbol | undefined {
if (
node.type === AST_NODE_TYPES.FunctionDeclaration ||
node.type === AST_NODE_TYPES.TSDeclareFunction
) {
// For a `FunctionDeclaration` or `TSDeclareFunction` this may be `null` if
// and only if the parent is an `ExportDefaultDeclaration`.
//
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return node.id!.name;
}

return getStaticMemberAccessValue(node, context);
}
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export * from './getStringLength';
export * from './getTextWithParentheses';
export * from './getThisExpression';
export * from './getWrappingFixer';
export * from './hasOverloadSignatures';
export * from './isArrayMethodCallWithPredicate';
export * from './isAssignee';
export * from './isNodeEqual';
Expand Down
Loading

0 comments on commit b688380

Please sign in to comment.