Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
gh-andre opened this issue Jul 21, 2024 · 8 comments
Labels
Duplicate An existing issue was already created

Comments

@gh-andre
Copy link

πŸ”Ž Search Terms

type shape equivalency getters setters

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about type shape equivalency

⏯ 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
Copy link

See #58296

@gh-andre
Copy link
Author

@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
Copy link
Contributor

jcalz commented Jul 21, 2024

#59331 seems similar

@gh-andre
Copy link
Author

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

@MartinJohns
Copy link
Contributor

MartinJohns commented Jul 22, 2024

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
Copy link
Author

@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
Copy link
Contributor

MartinJohns commented Jul 22, 2024

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).

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jul 22, 2024
@typescript-bot
Copy link
Collaborator

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

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants