Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(eslint-plugin): [no-invalid-void-type] allow union of void and `…
…allowInGenericTypeArguments` (#1960)
  • Loading branch information
sinyovercosy committed May 17, 2020
1 parent 1af59ba commit 1bc105a
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 25 deletions.
12 changes: 9 additions & 3 deletions 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

Expand Down Expand Up @@ -44,6 +45,8 @@ function noop(): void {}
let trulyUndefined = void 0;

async function promiseMeSomething(): Promise<void> {}

type stillVoid = void | never;
```

### Options
Expand All @@ -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 }`:
Expand All @@ -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
Expand Down
105 changes: 84 additions & 21 deletions packages/eslint-plugin/src/rules/no-invalid-void-type.ts
Expand Up @@ -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;
}

Expand Down
66 changes: 65 additions & 1 deletion packages/eslint-plugin/tests/rules/no-invalid-void-type.test.ts
Expand Up @@ -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: [
{
Expand Down Expand Up @@ -67,6 +84,17 @@ ruleTester.run('allowInGenericTypeArguments: false', rule, {
},
],
},
{
code: 'type invalidVoidUnion = void | number;',
options: [{ allowInGenericTypeArguments: false }],
errors: [
{
messageId: 'invalidVoidNotReturn',
line: 1,
column: 25,
},
],
},
],
});

Expand All @@ -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: [
{
Expand Down Expand Up @@ -315,7 +345,6 @@ ruleTester.run('allowInGenericTypeArguments: true', rule, {
{
code: `
type VoidType = void;
class OtherClassName {
private propName: VoidType;
}
Expand Down Expand Up @@ -406,6 +435,16 @@ ruleTester.run('allowInGenericTypeArguments: true', rule, {
},
],
},
{
code: 'type invalidVoidUnion = void | Map<string, number>;',
errors: [
{
messageId: 'invalidVoidNotReturnOrGeneric',
line: 1,
column: 25,
},
],
},
],
});

Expand Down Expand Up @@ -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: [
{
Expand Down

0 comments on commit 1bc105a

Please sign in to comment.