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>
peanutenthusiast · Nov 4, 2024 · e2e9ffc · e2e9ffc
1 parent ac1f632
commit e2e9ffc
Show file tree

Hide file tree

Showing 9 changed files with 869 additions and 33 deletions.
diff --git a/eslint.config.mjs b/eslint.config.mjs
@@ -105,13 +105,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',
         {

diff --git a/packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx b/packages/eslint-plugin/docs/rules/no-confusing-void-expression.mdx
@@ -123,6 +123,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.

diff --git a/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts b/packages/eslint-plugin/src/rules/no-confusing-void-expression.ts
@@ -16,11 +16,13 @@ import {
   nullThrows,
   NullThrowsReasons,
 } from '../util';
+import { getParentFunctionNode } from '../util/getParentFunctionNode';
 
 export type Options = [
   {
     ignoreArrowShorthand?: boolean;
     ignoreVoidOperator?: boolean;
+    ignoreVoidReturningFunctions?: boolean;
   },
 ];
 
@@ -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
@@ -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({
@@ -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({
@@ -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
@@ -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;
+    }
   },
 });
diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts
@@ -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';
 
@@ -18,6 +17,7 @@ import {
   isTypeUnknownType,
   isUnsafeAssignment,
 } from '../util';
+import { getParentFunctionNode } from '../util/getParentFunctionNode';
 
 export default createRule({
   name: 'no-unsafe-return',
@@ -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,

diff --git a/packages/eslint-plugin/src/util/getParentFunctionNode.ts b/packages/eslint-plugin/src/util/getParentFunctionNode.ts
@@ -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;
+}
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