[no-invalid-void-type] Should allow this: void

[ghost]

Repro

{
  "rules": {
    "@typescript-eslint/no-invalid-void-type": ["error"]
  }
}

interface ClassWithStaticMethod {
    method(this: void): void;
}

Expected Result: Should not return an error here, as this: void clarifies that the method is static.

Actual Result: Throws an error saying that void is only valid as a return type or generic type variable.

Additional Info: TypeScript specially refers to "this in void context" which clarifies that void is the right type here, not undefined.

Versions

package	version
@typescript-eslint/eslint-plugin	2.34.0
@typescript-eslint/parser	2.34.0
TypeScript	3.9.3
ESLint	7.0.0
node	12.16.3
yarn	1.22.4

[bradzacher]

This code doesn't do what you think it does - defining an interface method with this: void enforces nothing on the implementing types:

interface ClassWithStaticMethod {
    method(this: void): void;
}

class Foo implements ClassWithStaticMethod {
    foo = 1;
    method() { // no error
        console.log(this.foo); // can still use `this`
    }
}

Foo.method(); // error - Property 'method' does not exist on type 'typeof Foo'.
(new Foo()).method();

class Bar implements ClassWithStaticMethod { // error - Property 'method' is missing in type 'Bar' but required in type 'ClassWithStaticMethod'.
    static foo = 1;
    static method() { // no error
        console.log(this.foo); // can still use `this`
    }
}

const x = {
    foo: 1,
    method() {
        console.log(this.foo);
    },
}

const y: ClassWithStaticMethod = x; // no error
y.method();

repl

[ghost]

Oh, huh.

I remember having cases where I had to use this: void but if that isn't one of them I'm not 100% sure any more. I think that they exist.

[bradzacher]

note that static methods still have access to a this context. In a static method, this references the class definition itself:

class Foo {
    static one() {
        this.two();
    }
    static two() {
        console.log('two');
    }
}

Foo.one(); // prints "two"

repl

TBH this: void isn't something you should be using - you should use the noImplicitThis compiler option instead. With this compiler option on, TS will automatically know that the this context doesn't exist, and will error appropriately.

Also, you should avoid this parameters unless you really, really need them as they can significantly slow down your typecheck times.

[yangmingshan]

Another use case:

interface Options {
    [key: string]: any
    setup: (this: void) => any
}

const defineComponent = (options: Options) => {
    const { setup } = options
    setup()
}

defineComponent({
    setup() {
        console.log(this.a) // won't report error without `this: void`
    }
})

[bradzacher]

TBF though - for that use case it wouldn't matter if the function was on a class or an object, it would error at runtime.

Additionally using this:void in this way provides a false sense of security.

Eg this is valid code, that will circumvent the this:void:

interface Options {
    [key: string]: any
    setup: (this: void) => any
}

const defineComponent = (options: Options) => {
    const { setup } = options
    setup()
}

class Foo implements Options {
    setup() {
        console.log(this.a);
    }

    [key: string]: any;
}

defineComponent(new Foo())

repl

IMO that use case is an edge case that should be covered with an eslint-disable explaining why it's valid, instead of allowing it always. In general this parameters are slow (from a compiler perspective), and do not enforce the correctness you want.

We've thought about introducing a lint rule to always require this parameters, but doing so essentially converts every function into a generic function, which causes significant compiler slowdown.

I just advise against ever using this parameters.

[ghost]

Personally, I think that this being slow is a compiler bug, not a reason to suggest against their use. That said, I do think that it providing a false sense of security is a fair reason to suggest against it in these cases. But I think that this should be a separate lint, and this still is technically a bug in this lint.

[bradzacher]

happy to accept a PR if someone wants to implement it.

[tadhgmister]

typescript explicitly recommends using this: void when the method in question does not rely on the this parameter.

If a subclass overrides a method without explicit this parameter you are right that it can lead to a false sense of security, but also an explicit this parameter does get correctly labeled as an error. I'm inclined to call the false sense of security more of a bug with typescript than anything else.

Also the implementation of typescript does use the void type internally when there is no this parameter so it definitely won't have any performance related issues. (raw file link)

// line 15677
if (sourceThisType && sourceThisType !== voidType){}

// line 25918
const thisArgumentType = thisArgumentNode ? checkExpression(thisArgumentNode) : voidType;

// line 26152
if (thisType && thisType !== voidType && ...){}

So highly in support of allowing void on the this parameter and will start working on a PR.

[papb]

Also, you should avoid this parameters unless you really, really need them as they can significantly slow down your typecheck times.

Hmm, interesting, do you know why this happens? Is this a bug?

[tadhgmister]

@papb no it's just that the logic to actually check the this parameter is comparatively time consuming, having this: this type on a class method means every time you call that method typescript has to first get the type of object it's called on, resolve the this type accordingly then check that the object does in fact conform to the given type.

using the this type is implemented internally to be similar to generics which is more costly to type check.

I'm sure it'd be possible for typescript to make optimizations to speed this up in trivial cases like this: this but since that is fairly uncommon it's not optimized. But like I said it treats no this parameter equivalently to having this: void so no performance hit would occur for the void case.

[bradzacher]

Also, you should avoid this parameters unless you really, really need them as they can significantly slow down your typecheck times.

Hmm, interesting, do you know why this happens? Is this a bug?

It's just a limitation of how TS handles this parameters.

IIRC - when you annotate a method with a this parameter, under the hood it essentially makes it a generic method. This means that every time you use that method TS has to perform the additional generic checks - which is a performance overhead that can impact your compile times.

I can't recall the issues off the top of my head, but I've seen it discussed in the past - people have asked for lint rules and TS features that make you always annotate the this parameter, but it's been shot down because of the performance implications.

[tadhgmister]

when you annotate a method with a this parameter, under the hood it essentially makes it a generic method

@bradzacher no you are thinking of the this type, using this parameter doesn't have the same implication.

When you use a class method like:

class Example {
  method(): this { return this; }
}

it internally handles it similar to:

class Example2 {
  method<T extends Example2>(this: T): T {return this;}
}

just adding a parameter for this (assuming it isn't the this type) adds the same amount of overhead as adding any other parameter.

particularly for this: void adds even less overhead because typescript internally uses void when there is no this parameter at all, as I mentioned above.

[bradzacher]

Found the issue: microsoft/TypeScript#35451 (comment)

Quoting the comment:

Adding a this parameter effectively makes the method generic; making all methods in classes generic by default has horrifying performance implications (think 20%+ increases in compilation times in class-heavy codebases).