microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.94k stars 12.48k forks source link

Shape-based type equivalence should evaluate getters and setters, not just getters #59382

Closed gh-andre closed 3 months ago

gh-andre commented 3 months ago

🔎 Search Terms

type shape equivalency getters setters

🕗 Version & Regression Information

⏯ Playground Link

https://www.typescriptlang.org/play/?#code/MYGwhgzhAEAi0G8BQ1XTALmgOwK4FsAjAUwCcBuFNQrCAF1IEtsBzStaYAe23tN2B0upABQBKKqmQcOdABaMIAOjDQAvNACMAJgDM7GanmKlhddABEAQQBCAYQsHUAXySukoSDACiiSdAAHJgA3MDpiaAB9ABMsWCdOHj4BIVFYuAkOaUNoY2UY82iE1w4WYjp0cURSctxSbFyFfOiVchK0MorCKoQaujqGvKUY0za3JCQAM1xsQUYeaEnsEXTYMWgsYK5GaKRs1BbVDV0T8lQAenPofmw6RnwIybBGEDqIkmAwXAgI+TJiADkMGwXGgPzo4VI4yQIHK0GIWF8GmwxAA7tBvCIUejYOIxJQpstiPiLlcIHIuLgQNFrsQAsIKr4QRVuPgAmFGIRYdBUYx5HBoFjQeDIRAJEA

💻 Code

class D {
    a: number;
    b: string;
    constructor()
    {
        this.a = 123;
        this.b = "ABC";
    }
}

class E {
    private _d: D;
    constructor(d: D)
    {
        this._d = d;
    }
    get a() {return this._d.a;}
    get b() {return this._d.b;}
}

function fn(d: D) : void
{
    d.a = 333;  // runtime failure because there's no setter
}

let e: E = new E(new D());

fn(e);  // should report E not compatible with D (no setters)

🙁 Actual behavior

No errors are reported when passing and instance of E in a call where D is expected because E has getters for all properties of D, even though only getters exist and there are no setters. As a result of this, a runtime error will be reported if an attempt to assign any of those properties is made.

🙂 Expected behavior

The type checker should consider type shapes equivalent only if they have same get/set behavior. In the example above, if a and b in D were readonly, OR if E had get/set for both properties, then the example would contain no errors. Both of these conditions should be checked when evaluating type shape equivalency.

Additional information about the issue

TypeScript 5.5.3

whzx5byb commented 3 months ago

See https://github.com/microsoft/TypeScript/pull/58296

gh-andre commented 3 months ago

@whzx5byb Not the same. The type checker should catch incompatible shapes without having to enforce readonly, which in this case is undesirable anyway - I have a document D in the entity E as a private data member, but when used on its own, D can be set as needed.

jcalz commented 3 months ago

59331 seems similar

gh-andre commented 3 months ago

@jcalz That one does look similar, perhaps if only interfaces are considered, without inheritance.

MartinJohns commented 3 months ago

Also #49046. The issue linked by whzx5byb is what would aim to solve this.

The getter-only version is essentially a "readonly" property, but readonly properties don't make an incompatible type. Types with readonly properties are implicitly assignable to types with mutable properties.

Worth mentioning that TypeScript does not differentiate between properties and setter/getter.

gh-andre commented 3 months ago

@MartinJohns

Types with readonly properties are implicitly assignable to types with mutable properties.

Isn't this exactly what the static type checker is supposed to catch, so it doesn't blow up at runtime?

The getter-only version is essentially a "readonly" property

If there was a publicly available TypeScript spec where these two would be designated as interchangeable constructs, then yes. Otherwise, I'm just trying to highlight that there are use cases with failing shape equivalency without an explicit readonly being used.

MartinJohns commented 3 months ago

Isn't this exactly what the static type checker is supposed to catch, so it doesn't blow up at runtime?

TypeScript is not designed to have a fully sound type system. It has a lot of gotchas that can blow up at runtime or provide unexpected results.

TypeScript Design Goals Non-Goal 3:

Apply a sound or "provably correct" type system. Instead, strike a balance between correctness and productivity.


If there was a publicly available TypeScript spec

TypeScript does not have a spec. Not even a non-public one. The website, the wiki and the issues act as documentation. The last one sometimes takes a bit of time to dig through. There used to be a (heavily outdated) spec, but maintaining it required too much work for barely any benefit.


A more accurate duplicate is #27538, which was marked as a duplicate of #8496 (which is slightly different, but the underlaying problem is the same).

typescript-bot commented 3 months ago

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.