From 27126ca29b48b8bd505a97dd8434be4310a7b442 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 10:01:55 +0300 Subject: [PATCH 01/27] initial implementation --- .../src/rules/no-confusing-void-expression.ts | 93 ++++++++++++++++++- 1 file changed, 90 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts index ff66138dcf9d..b50e08162b4c 100644 --- a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -7,6 +7,7 @@ import type { MakeRequired } from '../util'; import { createRule, getConstrainedTypeAtLocation, + getContextualType, getParserServices, isClosingParenToken, isOpeningParenToken, @@ -91,6 +92,77 @@ export default createRule({ defaultOptions: [{ ignoreArrowShorthand: false, ignoreVoidOperator: false }], create(context, [options]) { + const services = getParserServices(context); + const checker = services.program.getTypeChecker(); + + 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; + } + + return null; + } + + function functionTypeReturnsVoid(functionType: ts.Type): boolean { + const callSignatures = tsutils.getCallSignaturesOfType(functionType); + + return callSignatures.some(signature => { + const returnType = signature.getReturnType(); + + return tsutils + .unionTypeParts(returnType) + .some(part => tsutils.isTypeFlagSet(part, ts.TypeFlags.VoidLike)); + }); + } + + function functionReturnsVoid( + functionNode: + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression, + ): boolean { + const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); + + let functionType = + ts.isFunctionExpression(functionTSNode) || + ts.isArrowFunction(functionTSNode) + ? getContextualType(checker, functionTSNode) + : services.getTypeAtLocation(functionNode); + + if (!functionType) { + functionType = services.getTypeAtLocation(functionNode); + } + + const functionTypeParts = tsutils.unionTypeParts(functionType); + + const returnsVoid = functionTypeParts.some(part => { + if (tsutils.isIntersectionType(part)) { + return tsutils + .intersectionTypeParts(part) + .every(functionTypeReturnsVoid); + } + + return functionTypeReturnsVoid(part); + }); + + return returnsVoid; + } + return { 'AwaitExpression, CallExpression, TaggedTemplateExpression'( node: @@ -98,7 +170,6 @@ export default createRule({ | 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 @@ -120,6 +191,12 @@ export default createRule({ if (invalidAncestor.type === AST_NODE_TYPES.ArrowFunctionExpression) { // handle arrow function shorthand + const returnsVoid = functionReturnsVoid(invalidAncestor); + + if (returnsVoid) { + return; + } + if (options.ignoreVoidOperator) { // handle wrapping with `void` return context.report({ @@ -175,6 +252,18 @@ export default createRule({ if (invalidAncestor.type === AST_NODE_TYPES.ReturnStatement) { // handle return statement + const functionNode = getParentFunctionNode(invalidAncestor); + + if (!functionNode) { + return; + } + + const returnsVoid = functionReturnsVoid(functionNode); + + if (returnsVoid) { + return; + } + if (options.ignoreVoidOperator) { // handle wrapping with `void` return context.report({ @@ -378,8 +467,6 @@ export default createRule({ function canFix( node: ReturnStatementWithArgument | TSESTree.ArrowFunctionExpression, ): boolean { - const services = getParserServices(context); - const targetNode = node.type === AST_NODE_TYPES.ReturnStatement ? node.argument From 98830b9518d2bf50956e280799f3f901ffc41c1a Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 13:32:10 +0300 Subject: [PATCH 02/27] enable rule --- eslint.config.mjs | 3 +-- .../typescript-estree/tests/lib/getParsedConfigFile.test.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index c87098636b74..2de30a38110b 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -96,8 +96,7 @@ 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', + '@typescript-eslint/no-confusing-void-expression': 'error', // // our plugin :D diff --git a/packages/typescript-estree/tests/lib/getParsedConfigFile.test.ts b/packages/typescript-estree/tests/lib/getParsedConfigFile.test.ts index a1aa1a2c04e4..49610b99e89b 100644 --- a/packages/typescript-estree/tests/lib/getParsedConfigFile.test.ts +++ b/packages/typescript-estree/tests/lib/getParsedConfigFile.test.ts @@ -81,7 +81,7 @@ describe('getParsedConfigFile', () => { typeof ts.getParsedCommandLineOfConfigFile > ) => { - return host.onUnRecoverableConfigFileDiagnostic({ + host.onUnRecoverableConfigFileDiagnostic({ category: ts.DiagnosticCategory.Error, code: 1234, file: ts.createSourceFile('./tsconfig.json', '', { From 3830a24a7b945f20e441a8c8d2b4200435924e89 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 13:39:30 +0300 Subject: [PATCH 03/27] improvements and fixes --- .../src/rules/no-confusing-void-expression.ts | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts index b50e08162b4c..1951927fa7de 100644 --- a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -118,19 +118,25 @@ export default createRule({ return null; } - function functionTypeReturnsVoid(functionType: ts.Type): boolean { + function isVoidReturningFunctionType(functionType: ts.Type): boolean { const callSignatures = tsutils.getCallSignaturesOfType(functionType); return callSignatures.some(signature => { const returnType = signature.getReturnType(); + if (tsutils.isIntersectionType(returnType)) { + return tsutils + .unionTypeParts(returnType) + .every(tsutils.isIntrinsicVoidType); + } + return tsutils .unionTypeParts(returnType) - .some(part => tsutils.isTypeFlagSet(part, ts.TypeFlags.VoidLike)); + .some(tsutils.isIntrinsicVoidType); }); } - function functionReturnsVoid( + function isVoidReturningFunctionNode( functionNode: | TSESTree.ArrowFunctionExpression | TSESTree.FunctionDeclaration @@ -138,29 +144,35 @@ export default createRule({ ): boolean { const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); - let functionType = + const functionType = ts.isFunctionExpression(functionTSNode) || ts.isArrowFunction(functionTSNode) ? getContextualType(checker, functionTSNode) : services.getTypeAtLocation(functionNode); if (!functionType) { - functionType = services.getTypeAtLocation(functionNode); + if (!functionTSNode.type) { + return false; + } + + const returnType = checker.getTypeFromTypeNode(functionTSNode.type); + + return tsutils + .intersectionTypeParts(returnType) + .some(tsutils.isIntrinsicVoidType); } const functionTypeParts = tsutils.unionTypeParts(functionType); - const returnsVoid = functionTypeParts.some(part => { + return functionTypeParts.some(part => { if (tsutils.isIntersectionType(part)) { return tsutils .intersectionTypeParts(part) - .every(functionTypeReturnsVoid); + .every(isVoidReturningFunctionType); } - return functionTypeReturnsVoid(part); + return isVoidReturningFunctionType(part); }); - - return returnsVoid; } return { @@ -191,7 +203,7 @@ export default createRule({ if (invalidAncestor.type === AST_NODE_TYPES.ArrowFunctionExpression) { // handle arrow function shorthand - const returnsVoid = functionReturnsVoid(invalidAncestor); + const returnsVoid = isVoidReturningFunctionNode(invalidAncestor); if (returnsVoid) { return; @@ -258,7 +270,7 @@ export default createRule({ return; } - const returnsVoid = functionReturnsVoid(functionNode); + const returnsVoid = isVoidReturningFunctionNode(functionNode); if (returnsVoid) { return; From 5b9655df40215029ae44af897682bcbc99a712a1 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 14:40:53 +0300 Subject: [PATCH 04/27] use checker.getContextualType over getContextualType --- .../eslint-plugin/src/rules/no-confusing-void-expression.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts index 1951927fa7de..8c6a1faaaaf3 100644 --- a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -7,7 +7,6 @@ import type { MakeRequired } from '../util'; import { createRule, getConstrainedTypeAtLocation, - getContextualType, getParserServices, isClosingParenToken, isOpeningParenToken, @@ -147,7 +146,7 @@ export default createRule({ const functionType = ts.isFunctionExpression(functionTSNode) || ts.isArrowFunction(functionTSNode) - ? getContextualType(checker, functionTSNode) + ? checker.getContextualType(functionTSNode) : services.getTypeAtLocation(functionNode); if (!functionType) { From e47395ed49e89f43ae8511be24f9b9d81f867730 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 14:52:08 +0300 Subject: [PATCH 05/27] more fixes --- .../src/rules/no-confusing-void-expression.ts | 53 ++++++++++--------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts index 8c6a1faaaaf3..d92b90b5cf2a 100644 --- a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -143,17 +143,7 @@ export default createRule({ ): boolean { const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); - const functionType = - ts.isFunctionExpression(functionTSNode) || - ts.isArrowFunction(functionTSNode) - ? checker.getContextualType(functionTSNode) - : services.getTypeAtLocation(functionNode); - - if (!functionType) { - if (!functionTSNode.type) { - return false; - } - + if (functionTSNode.type) { const returnType = checker.getTypeFromTypeNode(functionTSNode.type); return tsutils @@ -161,17 +151,30 @@ export default createRule({ .some(tsutils.isIntrinsicVoidType); } - const functionTypeParts = tsutils.unionTypeParts(functionType); + if ( + ts.isFunctionExpression(functionTSNode) || + ts.isArrowFunction(functionTSNode) + ) { + const functionType = checker.getContextualType(functionTSNode); - return functionTypeParts.some(part => { - if (tsutils.isIntersectionType(part)) { - return tsutils - .intersectionTypeParts(part) - .every(isVoidReturningFunctionType); + if (!functionType) { + return false; } - return isVoidReturningFunctionType(part); - }); + const functionTypeParts = tsutils.unionTypeParts(functionType); + + return functionTypeParts.some(part => { + if (tsutils.isIntersectionType(part)) { + return tsutils + .intersectionTypeParts(part) + .every(isVoidReturningFunctionType); + } + + return isVoidReturningFunctionType(part); + }); + } + + return false; } return { @@ -265,14 +268,12 @@ export default createRule({ const functionNode = getParentFunctionNode(invalidAncestor); - if (!functionNode) { - return; - } - - const returnsVoid = isVoidReturningFunctionNode(functionNode); + if (functionNode) { + const returnsVoid = isVoidReturningFunctionNode(functionNode); - if (returnsVoid) { - return; + if (returnsVoid) { + return; + } } if (options.ignoreVoidOperator) { From 4d0bf7ca0aade67ee88634ff4d88e0d006b8964a Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 15:46:05 +0300 Subject: [PATCH 06/27] improve implementation --- .../src/rules/no-confusing-void-expression.ts | 161 ++++++++---------- .../src/rules/no-unsafe-return.ts | 27 +-- .../src/util/getParentFunctionNode.ts | 27 +++ .../no-confusing-void-expression.test.ts | 159 +++++++++++++++++ 4 files changed, 257 insertions(+), 117 deletions(-) create mode 100644 packages/eslint-plugin/src/util/getParentFunctionNode.ts diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts index d92b90b5cf2a..27a51921989e 100644 --- a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -14,11 +14,13 @@ import { nullThrows, NullThrowsReasons, } from '../util'; +import { getParentFunctionNode } from '../util/getParentFunctionNode'; export type Options = [ { ignoreArrowShorthand?: boolean; ignoreVoidOperator?: boolean; + ignoreVoidReturningFunctions?: boolean; }, ]; @@ -80,6 +82,11 @@ export default createRule({ 'Whether to ignore returns that start with the `void` operator.', type: 'boolean', }, + ignoreVoidReturningFunctions: { + description: + 'Whether to ignore returns from functions with explicit `void` return types and functions with contextual `void` return types', + type: 'boolean', + }, }, additionalProperties: false, }, @@ -94,89 +101,6 @@ export default createRule({ const services = getParserServices(context); const checker = services.program.getTypeChecker(); - 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; - } - - return null; - } - - function isVoidReturningFunctionType(functionType: ts.Type): boolean { - const callSignatures = tsutils.getCallSignaturesOfType(functionType); - - return callSignatures.some(signature => { - const returnType = signature.getReturnType(); - - if (tsutils.isIntersectionType(returnType)) { - return tsutils - .unionTypeParts(returnType) - .every(tsutils.isIntrinsicVoidType); - } - - return tsutils - .unionTypeParts(returnType) - .some(tsutils.isIntrinsicVoidType); - }); - } - - function isVoidReturningFunctionNode( - functionNode: - | TSESTree.ArrowFunctionExpression - | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression, - ): boolean { - const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); - - if (functionTSNode.type) { - const returnType = checker.getTypeFromTypeNode(functionTSNode.type); - - return tsutils - .intersectionTypeParts(returnType) - .some(tsutils.isIntrinsicVoidType); - } - - if ( - ts.isFunctionExpression(functionTSNode) || - ts.isArrowFunction(functionTSNode) - ) { - const functionType = checker.getContextualType(functionTSNode); - - if (!functionType) { - return false; - } - - const functionTypeParts = tsutils.unionTypeParts(functionType); - - return functionTypeParts.some(part => { - if (tsutils.isIntersectionType(part)) { - return tsutils - .intersectionTypeParts(part) - .every(isVoidReturningFunctionType); - } - - return isVoidReturningFunctionType(part); - }); - } - - return false; - } - return { 'AwaitExpression, CallExpression, TaggedTemplateExpression'( node: @@ -205,10 +129,12 @@ export default createRule({ if (invalidAncestor.type === AST_NODE_TYPES.ArrowFunctionExpression) { // handle arrow function shorthand - const returnsVoid = isVoidReturningFunctionNode(invalidAncestor); + if (options.ignoreVoidReturningFunctions) { + const returnsVoid = isVoidReturningFunctionNode(invalidAncestor); - if (returnsVoid) { - return; + if (returnsVoid) { + return; + } } if (options.ignoreVoidOperator) { @@ -266,13 +192,15 @@ export default createRule({ if (invalidAncestor.type === AST_NODE_TYPES.ReturnStatement) { // handle return statement - const functionNode = getParentFunctionNode(invalidAncestor); + if (options.ignoreVoidReturningFunctions) { + const functionNode = getParentFunctionNode(invalidAncestor); - if (functionNode) { - const returnsVoid = isVoidReturningFunctionNode(functionNode); + if (functionNode) { + const returnsVoid = isVoidReturningFunctionNode(functionNode); - if (returnsVoid) { - return; + if (returnsVoid) { + return; + } } } @@ -487,5 +415,56 @@ export default createRule({ const type = getConstrainedTypeAtLocation(services, targetNode); return tsutils.isTypeFlagSet(type, ts.TypeFlags.VoidLike); } + + function isVoidReturningFunctionType(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 has a type annotation, check if it includes `void` + // - Otherwise, check if a function is a function expression or an arrow function + // - If it is, get its contextual type, bail if we cannot get its contextual type + // - Return based on whether the contextual type includes `void` + // - Otherwise, report the function as a non `void` returning function + + const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); + + if (functionTSNode.type) { + const returnType = checker.getTypeFromTypeNode(functionTSNode.type); + + return tsutils + .unionTypeParts(returnType) + .some(tsutils.isIntrinsicVoidType); + } + + if ( + ts.isFunctionExpression(functionTSNode) || + ts.isArrowFunction(functionTSNode) + ) { + const functionType = checker.getContextualType(functionTSNode); + + if (functionType) { + return tsutils + .unionTypeParts(functionType) + .some(isVoidReturningFunctionType); + } + } + + return false; + } }, }); diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 1b89ffbf8568..9afb4eaaa90e 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -1,5 +1,4 @@ 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'; @@ -17,6 +16,7 @@ import { isTypeUnknownType, isUnsafeAssignment, } from '../util'; +import { getParentFunctionNode } from '../util/getParentFunctionNode'; export default createRule({ name: 'no-unsafe-return', @@ -48,31 +48,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, diff --git a/packages/eslint-plugin/src/util/getParentFunctionNode.ts b/packages/eslint-plugin/src/util/getParentFunctionNode.ts new file mode 100644 index 000000000000..86264fac8b6a --- /dev/null +++ b/packages/eslint-plugin/src/util/getParentFunctionNode.ts @@ -0,0 +1,27 @@ +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; +} diff --git a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts index 24301c213bce..6430bc54efac 100644 --- a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts @@ -120,6 +120,101 @@ function cool(input: string) { } `, }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +function f(): void { + return console.log('bar'); +} + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +const f = (): void => { + return console.log('bar'); +}; + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +const f = (): void => console.log('bar'); + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +type Foo = () => void; +const test: Foo = () => console.log('foo'); + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +declare function foo(arg: () => void): void; +foo(() => console.log()); + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +declare function foo(arg: (() => void) | (() => string)): void; +foo(() => console.log()); + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +declare function foo(arg: (() => void) | (() => string) | string): void; +foo(() => console.log()); + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +declare function foo(arg: () => void | string): void; +foo(() => console.log()); + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +declare function foo(options: { cb: () => void }): void; +foo({ cb: () => console.log() }); + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +const q = { + foo: { bar: () => console.log() }, +} as { + foo: { bar: () => void }; +}; + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +interface Foo { + foo: () => void; +} + +function bar(): Foo { + return { + foo: () => console.log(), + }; +} + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +type HigherOrderType = () => () => () => void; +const x: HigherOrderType = () => () => () => console.log(); + `, + }, ], invalid: [ @@ -494,5 +589,69 @@ function notcool(input: string) { }, ], }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` + function f(): any { + return console.log('bar'); + } + `, + output: ` + function f(): any { + console.log('bar'); + } + `, + errors: [{ column: 18, messageId: 'invalidVoidExprReturnLast' }], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +declare function foo(arg: () => any): void; +foo(() => console.log()); + `, + output: ` +declare function foo(arg: () => any): void; +foo(() => { console.log(); }); + `, + errors: [{ column: 11, messageId: 'invalidVoidExprArrow' }], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` + function f() { + return console.log('bar'); + } + `, + output: ` + function f() { + console.log('bar'); + } + `, + errors: [{ column: 18, messageId: 'invalidVoidExprReturnLast' }], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` + const f = () => { + return console.log('bar'); + }; + `, + output: ` + const f = () => { + console.log('bar'); + }; + `, + errors: [{ column: 18, messageId: 'invalidVoidExprReturnLast' }], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` + const f = () => console.log('bar'); + `, + output: ` + const f = () => { console.log('bar'); }; + `, + errors: [{ column: 25, messageId: 'invalidVoidExprArrow' }], + }, ], }); From 117d244aa7c4fe86e1c37fb9ddfa07d38cd48985 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 19:00:45 +0300 Subject: [PATCH 07/27] add docs --- .../rules/no-confusing-void-expression.mdx | 18 ++++++++++++++++++ .../src/rules/no-confusing-void-expression.ts | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx b/packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx index b779558319b7..23056383931a 100644 --- a/packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx +++ b/packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx @@ -119,6 +119,24 @@ function doSomething() { console.log(void alert('Hello, world!')); ``` +### `ignoreVoidReturningFunctions` + +Whether to ignore returns from functions with explicit `void` return types and functions with contextual `void` return types. + +It may be preferable to allow functions that explicitly return `void` to return `void` expressions. This exception will also apply to functions with an implicit contextual return type of `void`. + +```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. diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts index 27a51921989e..31d0933169d0 100644 --- a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -84,7 +84,7 @@ export default createRule({ }, ignoreVoidReturningFunctions: { description: - 'Whether to ignore returns from functions with explicit `void` return types and functions with contextual `void` return types', + 'Whether to ignore returns from functions with explicit `void` return types and functions with contextual `void` return types.', type: 'boolean', }, }, From b4be799bc96f87af65714d319b91b3c644ccb37a Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 19:03:24 +0300 Subject: [PATCH 08/27] update snapshots --- .../no-confusing-void-expression.shot | 15 +++++++++++++++ .../no-confusing-void-expression.shot | 6 ++++++ 2 files changed, 21 insertions(+) diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-void-expression.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-void-expression.shot index bf20e61945c3..db49175d4373 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-void-expression.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-void-expression.shot @@ -80,3 +80,18 @@ function doSomething() { console.log(void alert('Hello, world!')); " `; + +exports[`Validating rule docs no-confusing-void-expression.mdx code examples ESLint output 5`] = ` +"Options: { "ignoreVoidReturningFunctions": true } + +function foo(): void { + return console.log(); +} + +function onError(callback: () => void): void { + callback(); +} + +onError(() => console.log('oops')); +" +`; diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-confusing-void-expression.shot b/packages/eslint-plugin/tests/schema-snapshots/no-confusing-void-expression.shot index 33d620951ee6..2ce5fcfafca8 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-confusing-void-expression.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-confusing-void-expression.shot @@ -15,6 +15,10 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos "ignoreVoidOperator": { "description": "Whether to ignore returns that start with the \`void\` operator.", "type": "boolean" + }, + "ignoreVoidReturningFunctions": { + "description": "Whether to ignore returns from functions with explicit \`void\` return types and functions with contextual \`void\` return types.", + "type": "boolean" } }, "type": "object" @@ -30,6 +34,8 @@ type Options = [ ignoreArrowShorthand?: boolean; /** Whether to ignore returns that start with the \`void\` operator. */ ignoreVoidOperator?: boolean; + /** Whether to ignore returns from functions with explicit \`void\` return types and functions with contextual \`void\` return types. */ + ignoreVoidReturningFunctions?: boolean; }, ]; " From 1c4978055d3da8bfb5d1eb29fd0fbd2c4da7e31e Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 19:23:09 +0300 Subject: [PATCH 09/27] enable rule with option --- eslint.config.mjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 2de30a38110b..f09bd76bb583 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -96,7 +96,10 @@ export default tseslint.config( }, linterOptions: { reportUnusedDisableDirectives: 'error' }, rules: { - '@typescript-eslint/no-confusing-void-expression': 'error', + '@typescript-eslint/no-confusing-void-expression': [ + 'error', + { ignoreVoidReturningFunctions: true }, + ], // // our plugin :D From 5dc76002e91e4274e9079689cf1b92de6c98bc5c Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 19:29:53 +0300 Subject: [PATCH 10/27] rename function names to be a bit less confusing --- .../eslint-plugin/src/rules/no-confusing-void-expression.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts index 31d0933169d0..fb8f58679d22 100644 --- a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -416,7 +416,7 @@ export default createRule({ return tsutils.isTypeFlagSet(type, ts.TypeFlags.VoidLike); } - function isVoidReturningFunctionType(functionType: ts.Type): boolean { + function isFunctionReturnTypeIncludesVoid(functionType: ts.Type): boolean { const callSignatures = tsutils.getCallSignaturesOfType(functionType); return callSignatures.some(signature => { @@ -460,7 +460,7 @@ export default createRule({ if (functionType) { return tsutils .unionTypeParts(functionType) - .some(isVoidReturningFunctionType); + .some(isFunctionReturnTypeIncludesVoid); } } From 2c26323e92e01058eada0775170fbc3e76a79f09 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 19:34:46 +0300 Subject: [PATCH 11/27] reword game-plan --- .../src/rules/no-confusing-void-expression.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts index fb8f58679d22..893d02a8e5ba 100644 --- a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts +++ b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts @@ -435,11 +435,11 @@ export default createRule({ | TSESTree.FunctionExpression, ): boolean { // Game plan: - // - If the function has a type annotation, check if it includes `void` - // - Otherwise, check if a function is a function expression or an arrow function - // - If it is, get its contextual type, bail if we cannot get its contextual type - // - Return based on whether the contextual type includes `void` - // - Otherwise, report the function as a non `void` returning function + // - 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 get it. + // - Return based on whether the contextual type includes `void` or not const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); From 6800dba8cd99a18492480081a0ba84e48f0904aa Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 19:35:33 +0300 Subject: [PATCH 12/27] move rule def to be under its proper title --- eslint.config.mjs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index f09bd76bb583..236c3a34014b 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -96,15 +96,14 @@ export default tseslint.config( }, linterOptions: { reportUnusedDisableDirectives: 'error' }, rules: { - '@typescript-eslint/no-confusing-void-expression': [ - 'error', - { ignoreVoidReturningFunctions: true }, - ], - // // our plugin :D // + '@typescript-eslint/no-confusing-void-expression': [ + 'error', + { ignoreVoidReturningFunctions: true }, + ], '@typescript-eslint/ban-ts-comment': [ 'error', { From 5f7833fa103af64ef086d167da0a338cc16409ff Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 19:38:50 +0300 Subject: [PATCH 13/27] remove invalid tests --- .../no-confusing-void-expression.test.ts | 64 ------------------- 1 file changed, 64 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts index 6430bc54efac..452e9560efb7 100644 --- a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts @@ -589,69 +589,5 @@ function notcool(input: string) { }, ], }, - { - options: [{ ignoreVoidReturningFunctions: true }], - code: ` - function f(): any { - return console.log('bar'); - } - `, - output: ` - function f(): any { - console.log('bar'); - } - `, - errors: [{ column: 18, messageId: 'invalidVoidExprReturnLast' }], - }, - { - options: [{ ignoreVoidReturningFunctions: true }], - code: ` -declare function foo(arg: () => any): void; -foo(() => console.log()); - `, - output: ` -declare function foo(arg: () => any): void; -foo(() => { console.log(); }); - `, - errors: [{ column: 11, messageId: 'invalidVoidExprArrow' }], - }, - { - options: [{ ignoreVoidReturningFunctions: true }], - code: ` - function f() { - return console.log('bar'); - } - `, - output: ` - function f() { - console.log('bar'); - } - `, - errors: [{ column: 18, messageId: 'invalidVoidExprReturnLast' }], - }, - { - options: [{ ignoreVoidReturningFunctions: true }], - code: ` - const f = () => { - return console.log('bar'); - }; - `, - output: ` - const f = () => { - console.log('bar'); - }; - `, - errors: [{ column: 18, messageId: 'invalidVoidExprReturnLast' }], - }, - { - options: [{ ignoreVoidReturningFunctions: true }], - code: ` - const f = () => console.log('bar'); - `, - output: ` - const f = () => { console.log('bar'); }; - `, - errors: [{ column: 25, messageId: 'invalidVoidExprArrow' }], - }, ], }); From 37201b83dc9142c0a9dd5f75c54e3a3c50f33d5a Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 20:08:26 +0300 Subject: [PATCH 14/27] add more tests --- .../no-confusing-void-expression.test.ts | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts index 452e9560efb7..b29fadd657ae 100644 --- a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts @@ -215,6 +215,31 @@ type HigherOrderType = () => () => () => void; const x: HigherOrderType = () => () => () => console.log(); `, }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +function test(): void & void { + return console.log('foo'); +} + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +type Foo = void; +declare function foo(): Foo; +function test(): Foo { + return foo(); +} + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +type Foo = void; +const test = (): Foo => console.log('err'); + `, + }, ], invalid: [ @@ -589,5 +614,113 @@ function notcool(input: string) { }, ], }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +type Foo = () => void; + +const test1: Foo = function () { + function testtt() { + return console.log(); + } +}; + `, + output: ` +type Foo = () => void; + +const test1: Foo = function () { + function testtt() { + console.log(); + } +}; + `, + errors: [ + { + line: 6, + column: 12, + messageId: 'invalidVoidExprReturnLast', + }, + ], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +const test2 = function () { + function testtt() { + return console.log(); + } +}; + `, + output: ` +const test2 = function () { + function testtt() { + console.log(); + } +}; + `, + errors: [ + { + line: 4, + column: 12, + messageId: 'invalidVoidExprReturnLast', + }, + ], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +return console.log('foo'); + `, + output: ` +{ console.log('foo'); return; } + `, + errors: [ + { + line: 2, + column: 8, + messageId: 'invalidVoidExprReturn', + }, + ], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +function foo(): void { + () => () => console.log(); +} + `, + output: ` +function foo(): void { + () => () => { console.log(); }; +} + `, + errors: [ + { + line: 3, + column: 15, + messageId: 'invalidVoidExprArrow', + }, + ], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +function foo(): any { + () => () => console.log(); +} + `, + output: ` +function foo(): any { + () => () => { console.log(); }; +} + `, + errors: [ + { + line: 3, + column: 15, + messageId: 'invalidVoidExprArrow', + }, + ], + }, ], }); From 5e235c85e077ae41d3d5d41ea35a5b60a2e9e1b3 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Fri, 27 Sep 2024 20:16:05 +0300 Subject: [PATCH 15/27] add more tests --- .../no-confusing-void-expression.test.ts | 158 +++++++++++++++++- 1 file changed, 151 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts index b29fadd657ae..f601165bd232 100644 --- a/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-confusing-void-expression.test.ts @@ -145,10 +145,161 @@ const f = (): void => console.log('bar'); { options: [{ ignoreVoidReturningFunctions: true }], code: ` +function test(): void { + { + return console.log('foo'); + } +} + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +const data = { + test(): void { + return console.log('foo'); + }, +}; + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +class Foo { + test(): void { + return console.log('foo'); + } +} + `, + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` +function test() { + function nestedTest(): void { + return console.log('foo'); + } +} + `, + }, + { + code: ` +type Foo = () => void; +const test = (() => console.log()) as Foo; + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +type Foo = { + foo: () => void; +}; +const test: Foo = { + foo: () => console.log(), +}; + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +const test = { + foo: () => console.log(), +} as { + foo: () => void; +}; + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +const test: { + foo: () => void; +} = { + foo: () => console.log(), +}; + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +type Foo = { + foo: { bar: () => void }; +}; +const test = { + foo: { bar: () => console.log() }, +} as Foo; + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +type Foo = { + foo: { bar: () => void }; +}; +const test: Foo = { + foo: { bar: () => console.log() }, +}; + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +type MethodType = () => void; +class App { + private method: MethodType = () => console.log(); +} + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +interface Foo { + foo: () => void; +} +function bar(): Foo { + return { + foo: () => console.log(), + }; +} + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +type HigherOrderType = () => () => () => void; +const x: HigherOrderType = () => () => () => console.log(); + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + code: ` +type Foo = { + foo: () => void; +}; +const test = { + foo: () => console.log(), +} as Foo; + `, + options: [{ ignoreVoidReturningFunctions: true }], + }, + { + options: [{ ignoreVoidReturningFunctions: true }], + code: ` type Foo = () => void; const test: Foo = () => console.log('foo'); `, }, + { + code: 'const foo =