Forbid this as constructor parameter type

[sandersn]

Fixes #5449. Thanks @ahejlsberg for pointing out how to fix this. If you have a better way to find out whether something is a constructor parameter, please do suggest it.

[ahejlsberg]

This isn't going to work. The right way to do it is to walk up the parent chain from node and see if you hit the body of the constructor before you get to the constructor itself.

[ahejlsberg]

Also, the some method on Array is an ES5 thing. We restrict ourselves to ES3 so we can run anywhere. In this particular case I'd use the forEach helper function. But, as I said above, there's a better way to do the test.

[sandersn]

Good to know about some. I switched to walking up the parent chain.

[DanielRosenwasser]

Could we just consider disallowing this to be used as a parameter type? It's potential for users to shoot themselves in the foot anyway:

class A {
    private a: number;

    getA() {
        return this.a;
    }

    foo(other: this) {
        return other.getA() === this.getA();
    }
}

class B extends A {
    private b: string;

    getB() {
        return this.b;
    }

    foo(other: this) {
        return super.foo(other) && other.getB() === this.getB();
    }
}

let b = new B();

let a1 = new A();
let a2: A = b;

// Valid & runs fine
a1.foo(a2)
a1.foo(b);

// Valid & crashes
a2.foo(a1);

[ahejlsberg]

Instead of the isConstructorParameter function I would just modify the existing test:

if (!(container.flags & NodeFlags.Static) &&
    (container.kind !== SyntaxKind.Constructor || isNodeDescendentOf(node, (<ConstructorDeclaration>container).body))) {
    ...
}

The isNodeDescendentOf is currently a local helper in emitter.ts, so you need to move it to utilities.ts so it can be shared.

[ahejlsberg]

Maybe use this in a type annotation of a local variable to test that this can be referenced in the body of a constructor.

[ahejlsberg]

👍 Once you address my comment about the test.