Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): [no-invalid-void-type] add option to allow this: void #2481

Merged
merged 6 commits into from Sep 28, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions packages/eslint-plugin/docs/rules/no-invalid-void-type.md
Expand Up @@ -54,10 +54,12 @@ type stillVoid = void | never;
```ts
interface Options {
allowInGenericTypeArguments?: boolean | string[];
allowAsThisParameter?: boolean;
}

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

Expand Down Expand Up @@ -97,6 +99,23 @@ type AllowedVoid = Ex.Mx.Tx<void>;
type AllowedVoidUnion = void | Ex.Mx.Tx<void>;
```

#### `allowAsThisParameter`

This option allows specifying a `this` parameter of a function to be `void` when set to `true`.
TypeScript [recommends using this pattern](https://www.typescriptlang.org/docs/handbook/functions.html#this-parameters-in-callbacks) to explicitly label function types that do not use a this argument.
tadhgmister marked this conversation as resolved.
Show resolved Hide resolved

This option is `false` by default.

The following patterns are considered warnings with `{ allowAsThisParameter: false }` but valid with `{ allowAsThisParameter: true }`:

```ts
function doThing(this: void) {}
class Example {
static helper(this: void) {}
callback(this: void) {}
}
```

## When Not To Use It

If you don't care about if `void` is used with other types,
Expand Down
45 changes: 37 additions & 8 deletions packages/eslint-plugin/src/rules/no-invalid-void-type.ts
Expand Up @@ -5,13 +5,16 @@ import {
import * as util from '../util';

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

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

export default util.createRule<[Options], MessageIds>({
name: 'no-invalid-void-type',
Expand All @@ -29,6 +32,10 @@ export default util.createRule<[Options], MessageIds>({
invalidVoidNotReturnOrGeneric:
'void is only valid as a return type or generic type variable',
invalidVoidNotReturn: 'void is only valid as a return type',
invalidVoidNotReturnOrThisParam:
'void is only valid as return type or type of this parameter',
tadhgmister marked this conversation as resolved.
Show resolved Hide resolved
invalidVoidNotReturnOrThisParamOrGeneric:
'void is only valid as a return type or generic type variable or in this parameter',
tadhgmister marked this conversation as resolved.
Show resolved Hide resolved
},
schema: [
{
Expand All @@ -44,13 +51,18 @@ export default util.createRule<[Options], MessageIds>({
},
],
},
allowAsThisParameter: {
type: 'boolean',
},
},
additionalProperties: false,
},
],
},
defaultOptions: [{ allowInGenericTypeArguments: true }],
create(context, [{ allowInGenericTypeArguments }]) {
defaultOptions: [
{ allowInGenericTypeArguments: true, allowAsThisParameter: false },
],
create(context, [{ allowInGenericTypeArguments, allowAsThisParameter }]) {
const validParents: AST_NODE_TYPES[] = [
AST_NODE_TYPES.TSTypeAnnotation, //
];
Expand Down Expand Up @@ -110,7 +122,9 @@ export default util.createRule<[Options], MessageIds>({

if (!allowInGenericTypeArguments) {
context.report({
messageId: 'invalidVoidNotReturn',
messageId: allowAsThisParameter
? 'invalidVoidNotReturnOrThisParam'
: 'invalidVoidNotReturn',
node,
});
}
Expand Down Expand Up @@ -159,6 +173,16 @@ export default util.createRule<[Options], MessageIds>({
return;
}

// this parameter is ok to be void.
if (
allowAsThisParameter &&
node.parent.type === AST_NODE_TYPES.TSTypeAnnotation &&
node.parent.parent.type === AST_NODE_TYPES.Identifier &&
node.parent.parent.name === 'this'
) {
return;
}

// default cases
if (
validParents.includes(node.parent.type) &&
Expand All @@ -168,9 +192,14 @@ export default util.createRule<[Options], MessageIds>({
}

context.report({
messageId: allowInGenericTypeArguments
? 'invalidVoidNotReturnOrGeneric'
: 'invalidVoidNotReturn',
messageId:
allowInGenericTypeArguments && allowAsThisParameter
? 'invalidVoidNotReturnOrThisParamOrGeneric'
: allowInGenericTypeArguments
? 'invalidVoidNotReturnOrGeneric'
: allowAsThisParameter
? 'invalidVoidNotReturnOrThisParam'
: 'invalidVoidNotReturn',
node,
});
},
Expand Down
53 changes: 53 additions & 0 deletions packages/eslint-plugin/tests/rules/no-invalid-void-type.test.ts
Expand Up @@ -538,3 +538,56 @@ async function foo(bar: () => void | Promise<void>) {
},
],
});

ruleTester.run('allowAsThisParameter: true', rule, {
valid: [
{
code: 'function f(this: void) {}',
options: [{ allowAsThisParameter: true }],
},
{
code: `
class Test {
public static helper(this: void) {}
method(this: void) {}
}
`,
options: [{ allowAsThisParameter: true }],
},
],
invalid: [
{
code: 'type alias = void;',
options: [
{ allowAsThisParameter: true, allowInGenericTypeArguments: true },
],
errors: [
{
messageId: 'invalidVoidNotReturnOrThisParamOrGeneric',
},
],
},
{
code: 'type alias = void;',
options: [
{ allowAsThisParameter: true, allowInGenericTypeArguments: false },
],
errors: [
{
messageId: 'invalidVoidNotReturnOrThisParam',
},
],
},
{
code: 'type alias = Array<void>;',
options: [
{ allowAsThisParameter: true, allowInGenericTypeArguments: false },
],
errors: [
{
messageId: 'invalidVoidNotReturnOrThisParam',
},
],
},
],
});