Class declaration not checked for compatibility with Object.prototype

[samwgoldman]

TypeScript Version: Whatever is running on the TypeScript playground

Code

class C {
  toString(): number {
    return 0;
  }
}

function f(o: {}): string {
  return o.toString();
}

f(new C);

Expected behavior:

The declaration of C should be an error, because toString is not compatible with the implementation in Object.prototype.

Actual behavior:

There is no error. Calling f with an instance of C results in a number at runtime where we expect a string.

[RyanCavanaugh]

I agree it's unsound, but it'd also be strange/annoying to effectively say (by the same argument) that e.g. const x = { toString: true }; is an illegal object literal that you simply cannot write. Sometimes you do want to do that and aren't going to dangerously alias by a supertype.

[samwgoldman]

Hmm, yeah. I agree it's definitely sacrificing convenience for soundness. Not sure what the calculus is for making those tradeoffs for your team, so I'll trust the experts!

[RyanCavanaugh]

I was vaguely surprised that this wasn't an error in Flow (it is in TS):

const j = { toString: true };
var m: Object = j;
m.toString();

Does Object work differently?

[samwgoldman]

You're right to be surprised; It should be an error. I'm leaning toward making the literal itself an error (the strange/annoying solution). Object is different than in TS, I believe. It's a weird flavor of any that I don't particularly care for :/

[RyanCavanaugh]

Just playing with it more... interesting behavior! It's hard to reckon what the right behavior is.

const j = { toString: true };
var a = j; // OK
var b = { toString: j.toString }; // OK
var c: {} = j; // OK
c.toString(); // OK

var x = { ...j }; // Error (?!)

I'm leaning toward making the literal itself an error

Not sure if Flow allows people to add things to the global type of all Objects (e.g. cute global-prototype-mutating libraries) but this might cause some weird interactions.

[samwgoldman]

Yeah, we're inconsistent about what we check against Object.prototype (for now). If you take the stance that all object should be subtypes of the Object.prototype, things become concrete if inflexible.

[vkurchatkin]

It's a bit weird though that

class Foo extends Object {
  toString() {
    return 1;
  }
}

is an error and

class Foo {
  toString() {
    return 1;
  }
}

is not, while semantically this is exactly the same thing.

[DanielRosenwasser]

It is to some extent, but I think our view up until this point has been that extends and implements enforce some guarantees about what declared type's shape will be (as opposed to intersections and unions which act a little more blindly - not always a bad thing though!).

[RyanCavanaugh]

Just thinking... if soundness is the goal, the only plausible fix is to disallow calls to things from Object.protoype. It's not going to be possible to detect this case:

var x = "to" + "String";
var j = { [x]: 20 };
j.toString(); // 😱 

[samwgoldman]

For sure, and I don't think anyone is suggesting that soundness is binary 100% or 0% (when it comes to static analysis of JS, that is). It's all about trade-offs, and I was just hoping to clarify what kind of trade-off was being made here, and hopefully to get a sense of what the calculus is for deciding those trade-offs.

[mhegazy]

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.