Navigation Menu

Skip to content

Commit

Permalink
feat(eslint-plugin): create no-invalid-void-type rule (#1847)
Browse files Browse the repository at this point in the history
  • Loading branch information
G-Rath committed Apr 26, 2020
1 parent f91ff20 commit f667ff1
Show file tree
Hide file tree
Showing 7 changed files with 699 additions and 1 deletion.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -119,6 +119,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-implied-eval`](./docs/rules/no-implied-eval.md) | Disallow the use of `eval()`-like methods | | | :thought_balloon: |
| [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/no-invalid-void-type`](./docs/rules/no-invalid-void-type.md) | Disallows usage of `void` type outside of generic or return types | | | |
| [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor` | :heavy_check_mark: | | |
| [`@typescript-eslint/no-misused-promises`](./docs/rules/no-misused-promises.md) | Avoid using promises in places not designed to handle them | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces | :heavy_check_mark: | | |
Expand Down
3 changes: 2 additions & 1 deletion packages/eslint-plugin/ROADMAP.md
Expand Up @@ -18,7 +18,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th
| [`adjacent-overload-signatures`] || [`@typescript-eslint/adjacent-overload-signatures`] |
| [`ban-ts-ignore`] || [`@typescript-eslint/ban-ts-ignore`] |
| [`ban-types`] | 🌓 | [`@typescript-eslint/ban-types`]<sup>[1]</sup> |
| [`invalid-void`] | 🛑 | N/A |
| [`invalid-void`] | | [`@typescript-eslint/no-invalid-void-type`] |
| [`member-access`] || [`@typescript-eslint/explicit-member-accessibility`] |
| [`member-ordering`] || [`@typescript-eslint/member-ordering`] |
| [`no-any`] || [`@typescript-eslint/no-explicit-any`] |
Expand Down Expand Up @@ -623,6 +623,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@typescript-eslint/restrict-plus-operands`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/restrict-plus-operands.md
[`@typescript-eslint/strict-boolean-expressions`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md
[`@typescript-eslint/indent`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/indent.md
[`@typescript-eslint/no-invalid-void-type`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-invalid-void-type.md
[`@typescript-eslint/no-require-imports`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-require-imports.md
[`@typescript-eslint/array-type`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/array-type.md
[`@typescript-eslint/class-name-casing`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/class-name-casing.md
Expand Down
101 changes: 101 additions & 0 deletions packages/eslint-plugin/docs/rules/no-invalid-void-type.md
@@ -0,0 +1,101 @@
# 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.

## 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.

## Rule Details

This rule aims to ensure that the `void` type is only used in valid places.

The following patterns are considered warnings:

```ts
type PossibleValues = string | number | void;
type MorePossibleValues = string | ((number & any) | (string | void));

function logSomething(thing: void) {}
function printArg<T = void>(arg: T) {}

logAndReturn<void>(undefined);

interface Interface {
lambda: () => void;
prop: void;
}

class MyClass {
private readonly propName: void;
}
```

The following patterns are not considered warnings:

```ts
type NoOp = () => void;

function noop(): void {}

let trulyUndefined = void 0;

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

### Options

```ts
interface Options {
allowInGenericTypeArguments?: boolean | string[];
}

const defaultOptions: Options = {
allowInGenericTypeArguments: true,
};
```

#### `allowInGenericTypeArguments`

This option lets you control if `void` can be used as a valid value for generic type parameters.

Alternatively, you can provide an array of strings which whitelist which types may accept `void` as a generic type parameter.

This option is `true` by default.

The following patterns are considered warnings with `{ allowInGenericTypeArguments: false }`:

```ts
logAndReturn<void>(undefined);

let voidPromise: Promise<void> = new Promise<void>(() => {});
let voidMap: Map<string, void> = new Map<string, void>();
```

The following patterns are considered warnings with `{ allowInGenericTypeArguments: ['Ex.Mx.Tx'] }`:

```ts
logAndReturn<void>(undefined);

type NotAllowedVoid1 = Mx.Tx<void>;
type NotAllowedVoid2 = Tx<void>;
type NotAllowedVoid3 = Promise<void>;
```

The following patterns are not considered warnings with `{ allowInGenericTypeArguments: ['Ex.Mx.Tx'] }`:

```ts
type AllowedVoid = Ex.MX.Tx<void>;
```

## When Not To Use It

If you don't care about if `void` is used with other types,
or in invalid places, then you don't need this rule.

## Compatibility

- TSLint: [invalid-void](https://palantir.github.io/tslint/rules/invalid-void/)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -50,6 +50,7 @@
"@typescript-eslint/no-for-in-array": "error",
"@typescript-eslint/no-implied-eval": "error",
"@typescript-eslint/no-inferrable-types": "error",
"@typescript-eslint/no-invalid-void-type": "error",
"no-magic-numbers": "off",
"@typescript-eslint/no-magic-numbers": "error",
"@typescript-eslint/no-misused-new": "error",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -40,6 +40,7 @@ import noFloatingPromises from './no-floating-promises';
import noForInArray from './no-for-in-array';
import noImpliedEval from './no-implied-eval';
import noInferrableTypes from './no-inferrable-types';
import noInvalidVoidType from './no-invalid-void-type';
import noMagicNumbers from './no-magic-numbers';
import noMisusedNew from './no-misused-new';
import noMisusedPromises from './no-misused-promises';
Expand Down Expand Up @@ -142,6 +143,7 @@ export default {
'no-for-in-array': noForInArray,
'no-implied-eval': noImpliedEval,
'no-inferrable-types': noInferrableTypes,
'no-invalid-void-type': noInvalidVoidType,
'no-magic-numbers': noMagicNumbers,
'no-misused-new': noMisusedNew,
'no-misused-promises': noMisusedPromises,
Expand Down
116 changes: 116 additions & 0 deletions packages/eslint-plugin/src/rules/no-invalid-void-type.ts
@@ -0,0 +1,116 @@
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import * as util from '../util';

interface Options {
allowInGenericTypeArguments: boolean | string[];
}

type MessageIds =
| 'invalidVoidForGeneric'
| 'invalidVoidNotReturnOrGeneric'
| 'invalidVoidNotReturn';

export default util.createRule<[Options], MessageIds>({
name: 'no-invalid-void-type',
meta: {
type: 'problem',
docs: {
description:
'Disallows usage of `void` type outside of generic or return types',
category: 'Best Practices',
recommended: false,
},
messages: {
invalidVoidForGeneric:
'{{ generic }} may not have void as a type variable',
invalidVoidNotReturnOrGeneric:
'void is only valid as a return type or generic type variable',
invalidVoidNotReturn: 'void is only valid as a return type',
},
schema: [
{
type: 'object',
properties: {
allowInGenericTypeArguments: {
oneOf: [
{ type: 'boolean' },
{
type: 'array',
items: { type: 'string' },
minLength: 1,
},
],
},
},
additionalProperties: false,
},
],
},
defaultOptions: [{ allowInGenericTypeArguments: true }],
create(context, [{ allowInGenericTypeArguments }]) {
const validParents: AST_NODE_TYPES[] = [
AST_NODE_TYPES.TSTypeAnnotation, //
];
const invalidGrandParents: AST_NODE_TYPES[] = [
AST_NODE_TYPES.TSPropertySignature,
AST_NODE_TYPES.CallExpression,

This comment was marked as spam.

Copy link
@jtbandes

jtbandes Jul 6, 2021

Contributor

Just curious: why does this rule disallow void in generic parameters to function calls? Such as the example logAndReturn<void>(undefined). This makes it difficult to adopt the rule in places where a function's generic parameter ends up as a return type. Like if you have wrapInPromise<T>(fn: () => T): Promise<T>, then wrapInPromise<void>() is disallowed.

Would it make sense to change this behavior of the rule by default, or add a configuration option to change it?

This comment has been minimized.

Copy link
@bradzacher

bradzacher Jul 6, 2021

Member

Please don't comment on commits.
Commit comments are impossible to keep track of as they they are on a specific commit, so you have to keep returning to that commit in the log to get to them.
Additionally they are not discoverable for other contributors - there's no search that I know of for finding them.

Just raise an issue and link the file + relevant lines in your comment.
If you include the commit hash in place of master in the linked URL, github will even render the code inline.

https://github.com/typescript-eslint/typescript-eslint/blob/master/CONTRIBUTING.md#commenting

AST_NODE_TYPES.ClassProperty,
AST_NODE_TYPES.Identifier,
];

if (allowInGenericTypeArguments === true) {
validParents.push(AST_NODE_TYPES.TSTypeParameterInstantiation);
}

return {
TSVoidKeyword(node: TSESTree.TSVoidKeyword): void {
/* istanbul ignore next */
if (!node.parent?.parent) {
return;
}

if (
validParents.includes(node.parent.type) &&
!invalidGrandParents.includes(node.parent.parent.type)
) {
return;
}

if (
node.parent.type === AST_NODE_TYPES.TSTypeParameterInstantiation &&
node.parent.parent.type === AST_NODE_TYPES.TSTypeReference &&
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;
}

context.report({
messageId: allowInGenericTypeArguments
? 'invalidVoidNotReturnOrGeneric'
: 'invalidVoidNotReturn',
node,
});
},
};
},
});

0 comments on commit f667ff1

Please sign in to comment.