Using private or protected make type checking nominal instead of structural

[blakeembrey]

From typings/typings#380. It's definitely unexpected and I can't understand why, but I see in the specification it's intentional. For example:

export class Test {
  private method () {}
}

export class Test2 {
  private method () {}
}

export function compare (t: Test) {}

compare(new Test2())

Expected behavior:

No error, they are structurally the same. Private and protected methods can not be used outside of the class, so it shouldn't affect assignability.

Actual behavior:

index.ts(11,9): error TS2345: Argument of type 'Test2' is not assignable to parameter of type 'Test'.
  Types have separate declarations of a private property 'method'.

Edit: From the issue in typings, I also linked to #6496 and #6365 which appear to describe the issue but only dive into workarounds instead of validating why this is happening.

[RyanCavanaugh]

Consider something like this

class Foo {
    private versionNumber = 10;

    public compareTo(other: Foo) {
        return this.getVersion() === other.getVersion();
    }

    private getVersion() {
        return this.versionNumber;
    }
}

class Bar {
    private myName = 'default;'
    public compareTo(other: Bar) {
        return this.myName === other.myName;
    }

    private getVersion() {
        /* ... DANGEROUS CODE HERE ...*/
        return -1;
    }
}

By inspection, Bar#getVersion is never invoked -- it's a private method and there are no calls to it from the originating class. That inspection ought to be sufficient for code that doesn't actively try to work around the typechecker.

But this code does invoke Bar#getVersion:

let f = new Foo();
let b = new Bar();
f.compareTo(b);

[blakeembrey]

@RyanCavanaugh I can't see the issue. It sounds like you're saying to use private as a hack here to hide away potentially dangerous code. What's the difference here to the methods being public (the case where this would work)? The exact same thing is possible, but using private means the type checker should rely on implementation details?

Edit: For example, switch to public and the same situation can occur, why is private or protected special here? I'm sure a lot of implementations do comparisons of public properties too, not just private. If you want something to be a nominal type check, I wouldn't have expected it to be "add private to a property".

class Foo {
    public versionNumber = 10;

    public compareTo(other: Foo) {
        return this.getVersion() === other.getVersion();
    }

    public getVersion() {
        return this.versionNumber;
    }
}

class Bar {
    public versionNumber = -1;
    public myName = 'default;'
    public compareTo(other: Bar) {
        return this.myName === other.myName;
    }

    public getVersion() {
        /* ... DANGEROUS CODE HERE ...*/
        return -1;
    }
}

let f = new Foo();
let b = new Bar();
f.compareTo(b);

[RyanCavanaugh]

I don't understand what's a hack about the idea of method visibility.

A private method isn't callable by someone who's not you.

This should include the case where the person who isn't you happens to be structurally compatible with you, no?

[blakeembrey]

The hack isn't method visibility, that's fine. The part that sounds hacky is using private to switch to a nominal type system instead of being structural.

This should include the case where the person who isn't you happens to be structurally compatible with you, no?

Yes, absolutely. I overlooked this.getVersion() === other.getVersion(). That should be disallowed based on the fact that other.getVersion is private and can not be used outside of it's instance, not based on the fact that it's nominal.

Edit: is -> isn't.

[RyanCavanaugh]

Allowing a class to invoke its own private members on another instance of that same class was an intentional design decision. This a very common thing in OOP languages and we took precedent from that. It'd be pretty cumbersome if we disallowed it (I suspect most people would just cast to any if it were disallowed).

[blakeembrey]

Thanks, I realised working thinking about your example. It still feels like a bit of a wart on the type system that will be surprising to people. Proper detection of the use-case would require reverse flow analysis, I suppose? Not sure what you'd call it, you could check the compatibility of other.getVersion by checking the called instance to determine private and public availability and error at the call signature when an unavailable private method is called. But I can see the current implementation is much cheaper.

[blakeembrey]

Ok, I guess the solution here is to make a common interface. Thanks.

Edit: I'm still not convinced though 😄 There's plenty of times when two different instances may be created, even accidentally such as in the linked use-cases or across execution frames. It feels odd to have an artificial restriction here when JavaScript itself has a dozen ways you can end up with separate instances at runtime too (where the structural types say 👍, but instanceof would fail).