feat(eslint-plugin): [no-invalid-void-type] allow union of void and allowInGenericTypeArguments

packages/eslint-plugin/docs/rules/no-invalid-void-type.md

@@ -1,13 +1,14 @@
 # Disallows usage of `void` type outside of generic or return types (`no-invalid-void-type`)
 
 Disallows usage of `void` type outside of return types or generic type arguments.
-If `void` is used as return type, it shouldn’t be a part of intersection/union type.
+If `void` is used as return type, it shouldn’t be a part of intersection/union type with most other types.
 
 ## Rationale
 
 The `void` type means “nothing” or that a function does not return any value,
 in contrast with implicit `undefined` type which means that a function returns a value `undefined`.
-So “nothing” cannot be mixed with any other types. If you need this - use the `undefined` type instead.
+So “nothing” cannot be mixed with any other types, other than `never`, which accepts all types.
+If you need this - use the `undefined` type instead.
 
 ## Rule Details
 
@@ -44,6 +45,8 @@ function noop(): void {}
 let trulyUndefined = void 0;
 
 async function promiseMeSomething(): Promise<void> {}
+
+type stillVoid = void | never;
 ```
 
 ### Options
@@ -64,6 +67,8 @@ This option lets you control if `void` can be used as a valid value for generic
 
 Alternatively, you can provide an array of strings which whitelist which types may accept `void` as a generic type parameter.
 
+Any types considered valid by this option will be considered valid as part of a union type with `void`.
+
 This option is `true` by default.
 
 The following patterns are considered warnings with `{ allowInGenericTypeArguments: false }`:
@@ -88,7 +93,8 @@ type NotAllowedVoid3 = Promise<void>;
 The following patterns are not considered warnings with `{ allowInGenericTypeArguments: ['Ex.Mx.Tx'] }`:
 
 ```ts
-type AllowedVoid = Ex.MX.Tx<void>;
+type AllowedVoid = Ex.Mx.Tx<void>;
+type AllowedVoidUnion = void | Ex.Mx.Tx<void>;
 ```
 
 ## When Not To Use It

packages/eslint-plugin/src/rules/no-invalid-void-type.ts

@@ -60,47 +60,110 @@ export default util.createRule<[Options], MessageIds>({
       AST_NODE_TYPES.ClassProperty,
       AST_NODE_TYPES.Identifier,
     ];
+    const validUnionMembers: AST_NODE_TYPES[] = [
+      AST_NODE_TYPES.TSVoidKeyword,
+      AST_NODE_TYPES.TSNeverKeyword,
+    ];
 
     if (allowInGenericTypeArguments === true) {
       validParents.push(AST_NODE_TYPES.TSTypeParameterInstantiation);
     }
 
+    /**
+     * @brief check if the given void keyword is used as a valid generic type
+     *
+     * reports if the type parametrized by void is not in the whitelist, or
+     * allowInGenericTypeArguments is false.
+     * no-op if the given void keyword is not used as generic type
+     */
+    function checkGenericTypeArgument(node: TSESTree.TSVoidKeyword): void {
+      // only matches T<..., void, ...>
+      // extra check for precaution
+      /* istanbul ignore next */
+      if (
+        node.parent?.type !== AST_NODE_TYPES.TSTypeParameterInstantiation ||
+        node.parent.parent?.type !== AST_NODE_TYPES.TSTypeReference
+      ) {
+        return;
+      }
+
+      // check whitelist
+      if (Array.isArray(allowInGenericTypeArguments)) {
+        const sourceCode = context.getSourceCode();
+        const fullyQualifiedName = sourceCode
+          .getText(node.parent.parent.typeName)
+          .replace(/ /gu, '');
+
+        if (
+          !allowInGenericTypeArguments
+            .map(s => s.replace(/ /gu, ''))
+            .includes(fullyQualifiedName)
+        ) {
+          context.report({
+            messageId: 'invalidVoidForGeneric',
+            data: { generic: fullyQualifiedName },
+            node,
+          });
+        }
+        return;
+      }
+
+      if (!allowInGenericTypeArguments) {
+        context.report({
+          messageId: 'invalidVoidNotReturn',
+          node,
+        });
+      }
+    }
+
+    /**
+     * @brief checks that a union containing void is valid
+     * @return true if every member of the union is specified as a valid type in
+     * validUnionMembers, or is a valid generic type parametrized by void
+     */
+    function isValidUnionType(node: TSESTree.TSUnionType): boolean {
+      return node.types.every(
+        member =>
+          validUnionMembers.includes(member.type) ||
+          // allows any T<..., void, ...> here, checked by checkGenericTypeArgument
+          (member.type === AST_NODE_TYPES.TSTypeReference &&
+            member.typeParameters?.type ===
+              AST_NODE_TYPES.TSTypeParameterInstantiation &&
+            member.typeParameters?.params
+              .map(param => param.type)
+              .includes(AST_NODE_TYPES.TSVoidKeyword)),
+      );
+    }
+
     return {
       TSVoidKeyword(node: TSESTree.TSVoidKeyword): void {
         /* istanbul ignore next */
         if (!node.parent?.parent) {
           return;
         }
 
+        // checks T<..., void, ...> against specification of allowInGenericArguments option
         if (
-          validParents.includes(node.parent.type) &&
-          !invalidGrandParents.includes(node.parent.parent.type)
+          node.parent.type === AST_NODE_TYPES.TSTypeParameterInstantiation &&
+          node.parent.parent.type === AST_NODE_TYPES.TSTypeReference
         ) {
+          checkGenericTypeArgument(node);
           return;
         }
 
+        // union w/ void must contain types from validUnionMembers, or a valid generic void type
         if (
-          node.parent.type === AST_NODE_TYPES.TSTypeParameterInstantiation &&
-          node.parent.parent.type === AST_NODE_TYPES.TSTypeReference &&
-          Array.isArray(allowInGenericTypeArguments)
+          node.parent.type === AST_NODE_TYPES.TSUnionType &&
+          isValidUnionType(node.parent)
         ) {
-          const sourceCode = context.getSourceCode();
-          const fullyQualifiedName = sourceCode
-            .getText(node.parent.parent.typeName)
-            .replace(/ /gu, '');
-
-          if (
-            !allowInGenericTypeArguments
-              .map(s => s.replace(/ /gu, ''))
-              .includes(fullyQualifiedName)
-          ) {
-            context.report({
-              messageId: 'invalidVoidForGeneric',
-              data: { generic: fullyQualifiedName },
-              node,
-            });
-          }
+          return;
+        }
 
+        // default cases
+        if (
+          validParents.includes(node.parent.type) &&
+          !invalidGrandParents.includes(node.parent.parent.type)
+        ) {
           return;
         }
 

packages/eslint-plugin/tests/rules/no-invalid-void-type.test.ts

@@ -11,6 +11,23 @@ ruleTester.run('allowInGenericTypeArguments: false', rule, {
       code: 'type Generic<T> = [T];',
       options: [{ allowInGenericTypeArguments: false }],
     },
+    {
+      // https://github.com/typescript-eslint/typescript-eslint/issues/1946
+      code: `
+function foo(): void | never {
+  throw new Error('Test');
+}
+      `,
+      options: [{ allowInGenericTypeArguments: false }],
+    },
+    {
+      code: 'type voidNeverUnion = void | never;',
+      options: [{ allowInGenericTypeArguments: false }],
+    },
+    {
+      code: 'type neverVoidUnion = never | void;',
+      options: [{ allowInGenericTypeArguments: false }],
+    },
   ],
   invalid: [
     {
@@ -67,6 +84,17 @@ ruleTester.run('allowInGenericTypeArguments: false', rule, {
         },
       ],
     },
+    {
+      code: 'type invalidVoidUnion = void | number;',
+      options: [{ allowInGenericTypeArguments: false }],
+      errors: [
+        {
+          messageId: 'invalidVoidNotReturn',
+          line: 1,
+          column: 25,
+        },
+      ],
+    },
   ],
 });
 
@@ -89,6 +117,8 @@ ruleTester.run('allowInGenericTypeArguments: true', rule, {
     'type UnionType = string | number;',
     'type GenericVoid = Generic<void>;',
     'type Generic<T> = [T];',
+    'type voidPromiseUnion = void | Promise<void>;',
+    'type promiseNeverUnion = Promise<void> | never;',
   ],
   invalid: [
     {
@@ -315,7 +345,6 @@ ruleTester.run('allowInGenericTypeArguments: true', rule, {
     {
       code: `
         type VoidType = void;
-
         class OtherClassName {
           private propName: VoidType;
         }
@@ -406,6 +435,16 @@ ruleTester.run('allowInGenericTypeArguments: true', rule, {
         },
       ],
     },
+    {
+      code: 'type invalidVoidUnion = void | Map<string, number>;',
+      errors: [
+        {
+          messageId: 'invalidVoidNotReturnOrGeneric',
+          line: 1,
+          column: 25,
+        },
+      ],
+    },
   ],
 });
 
@@ -435,6 +474,31 @@ ruleTester.run('allowInGenericTypeArguments: whitelist', rule, {
       code: 'type AllowedVoid = Ex.Mx.Tx<void>;',
       options: [{ allowInGenericTypeArguments: ['Ex . Mx . Tx'] }],
     },
+    {
+      code: 'type voidPromiseUnion = void | Promise<void>;',
+      options: [{ allowInGenericTypeArguments: ['Promise'] }],
+    },
+    {
+      code: 'type promiseVoidUnion = Promise<void> | void;',
+      options: [{ allowInGenericTypeArguments: ['Promise'] }],
+    },
+    {
+      // https://github.com/typescript-eslint/typescript-eslint/issues/1956
+      code: `
+async function foo(bar: () => void | Promise<void>) {
+  await bar();
+}
+      `,
+      options: [{ allowInGenericTypeArguments: ['Promise'] }],
+    },
+    {
+      code: 'type promiseNeverUnion = Promise<void> | never;',
+      options: [{ allowInGenericTypeArguments: ['Promise'] }],
+    },
+    {
+      code: 'type voidPromiseNeverUnion = void | Promise<void> | never;',
+      options: [{ allowInGenericTypeArguments: ['Promise'] }],
+    },
   ],
   invalid: [
     {