Typescript-eslint: [unbound-method] support binding methods inside constructor

Created on 22 Jun 2019  路  3Comments  路  Source: typescript-eslint/typescript-eslint

Repro

{
  "rules": {
    "@typescript-eslint/unbound-method": "error"
  }
}
class MyClass {
    constructor() {
        // will permanently bind and should not be an error (but currently is)
        this.log = this.log.bind(this)
    }

    public log(): void {
        console.log(this);
    }
}

const instance = new MyClass();

// this should not error because it is bound in the class's constructor so it will log the expected this
const myLog = instance.log;
myLog();

// This log might later be called with an incorrect scope
const { log } = instance;

Expected Result
No @typescript-eslint/unbound-method errors.

Actual Result
Two @typescript-eslint/unbound-method errors.

Additional Info

Versions

| package | version |
| ---------------------------------- | ------- |
| @typescript-eslint/eslint-plugin | 1.10.2 |
| @typescript-eslint/parser | 1.10.2 |
| TypeScript | 3.5.2 |
| ESLint | 5.16.0 |
| node | 10.14.1 |
| npm | 6.9.0 |

plugin rule option eslint-plugin

All 3 comments

I successfully replicated this.

Any news?

happy to accept a PR!

However, it will not be easy to implement.
The solution to this would have to be done completely manually, because AFAIK TypeScript does not track binding information anywhere.


_Take this aside with a pinch of salt, because even a naive solution is an improvement over the base implementation, and no doubt the below is an edge case that can be ignored._

As an aside, this isn't really possible to do 100% correctly, because there's the problem of things like reassignment outside of the constructor (because methods are not readonly).
So you'd need to do some global control-flow analysis shenanigans, which wouldn't work.

class MyClass {
    constructor() {
        // will permanently bind and should not be an error (but currently is)
        this.log = this.log.bind(this)
    }

    public log(): void {
        console.log(this);
    }
}

const instance = new MyClass();

// this should not error, because the method is bound
const myLog1 = instance.log;

//////////

instance.log = function methodThatsNoLongerBound() { console.log(this); }
// this should error because it is no longer bound
const myLog2 = instance.log;

//////////

instance.log = instance.log.bind(instance);
// this should not error, because the method is bound
const myLog3 = instance.log;

//////////

class MyClass2 {
    constructor() {
        // will permanently bind and should not be an error (but currently is)
        this.log = this.log.bind(this);

        if (process.env.DEV) {
            this.doSomethingStupid();
        }
    }

    public log(): void {
        console.log(this);
    }

    private doSomethingStupid() {
        this.log = function methodThatsNoLongerBound() { console.log(this); };
    }
}

const instance2 = new MyClass2();

// this should error, because the method is potentially not bound
const myLog4 = instance.log;
Was this page helpful?
0 / 5 - 0 ratings