treating setters as properties rather than functions can be unsound

[craigphicks]

🔎 Search Terms

setter union variance

is:issue in-title: setter (nothing relevant found)

is:issue in-title: setter union variance (nothing relevant found)

🕗 Version & Regression Information

confirmed 4.3.5 ~ 5.3.2

⏯ Playground Link

https://www.typescriptlang.org/play?ssl=16&ssc=78&pln=15&pc=50#code/JYOwLgpgTgZghgYwgAgMoTJKAeAKgPmQG8AoZc5AZw2QAcoB7WgCgA8AuXASgG5kB6fsigBXcMAC2KaIyjJgMZK2QgGYeSGQATBhLihkuAHQkAviQQAbOJUrIAwgEYATPIm1LEKeDvpM0bCI4dkcAH2dTfFIKKhp6JjZ2IJDTUOSIrmiYigVkZgBCAG1HABpnAF0jUCsRLQhKNiM4Li5kMAALRgB3JR4yCnNzKxs7e2cAZjcPLwgfNAwsQODx8Mis8mp1eJYOZPHU9NNM-uz5RQLC5xLxyurLWvrG5taO7t6T5EGSGDEEMGAGJofs5mCJ2A4XMhQg4Jq11gIhO1oCgAETbFHyOxgKAQOCQLTISzAADWKA8+k022gYAAnh9BGd5Oousi2ji8RACUTScg4HRrAYfiA-gCQCViMFHKZxXtTMgugwRJYCQAjaRQWSUD4iIzbPgMyjtBhdOxhZyhcbi0TiKRUI0m5BiOowUCc8gMgCqADlUAB5b0AEW1usYtGQAF4JSl9UJaAxbMAVZ42p1jQjkN6-YHg9sI1GIn0Yjrc5HZTG6PHKInk68056ff6vUHzEKRYDkD9xqDwU5nHCc6G84dCzlzsXQ01w1O+-CYoIPl9gcwQBAer3mC0+gzyTSAOaMJ3IRVgWgiMAkBmFACiACUb+VwSir6wIAgz26AFJwABucFQCCgYBaHUAAxfRPC0dgMQvIRrzvB9kEmEhvhELsVzXFwN14GD+TgPcDxAAlj1Pc9L1ve9H2fV93wJL9f3-QDgOQMDgAgqDkBwuCKMdEMmExR1CIgF0VwJEggA

💻 Code

interface Setter<T> {
    set prop(x:T); // runtime error if x not in domain T.
}
class C12 implements Setter<{a:1|2}>{
    set prop(x:{a:1}|{a:2}){
        if (![1,2].includes(x.a)) throw x;
    }
}
class C23 implements Setter<{a:3|2}>{
    set prop(x:{a:3}|{a:2}){
        if (![2,3].includes(x.a)) throw x;
    }
}
function fu2(u: C12 | C23) {
    // here "prop" is treated like plain property
    // if it were treated like a plain function, {a:1}, {a:3} would be errors
    u.prop; // shows 1|2|3  // UNSOUND
    u.prop = {a:1}; // possible throw // UNSOUND
    u.prop = {a:2};
    u.prop = {a:3}; // possible throw // UNSOUND
}
function fu3(u: C12) {
    u.prop = {a:2};
    if (u.prop.a===2){
        //
    }
}
fu2(new C12());
// playground output
// [ERR]: "Executed JavaScript Failed:" 
// [ERR]: 3 

fu3(new C12());
// playground output
// [ERR]: "Executed JavaScript Failed:" 
// [ERR]: u.prop is undefined 

🙁 Actual behavior

• TypeScript doesn't detect obvious runtime errors that it would detect if setters were treated as functions.
• TypeScript doesn't treat a missing getter as returning undefined.

🙂 Expected behavior

• setters should be treated as functions
• call to missing getters should return undefined

Additional information about the issue

This supersedes issue #56894.
@Anadarist 's pull #56895 claims to fix #56894, it would be great if it fixed this issue also.

[Andarist]

I think those are 2 separate issues. My PR fixes fu2 but TypeScript doesn't support write-only properties today and thus fu3 stays unchanged.

[fatcerberus]

Issue for writeonly properties: #21759

[craigphicks]

I thought writeonly suggestion #21795 was interesting. However, I guess I am more of a JS-literalist at heart, and writeonly is not a JS term, but set [name]() is.

I think it would be best to have TypeScript represent prototype set() and/or get() literally - and to honor that behavior as literally as possible. (Obviously TypeScript accepts the set/get notation, but now it converts that a plain type (or is it a plain type with caveats?)).

The best thing about the JS-literalist approach is that JS provides a solid reference for correct behavior.

• The types of the setter and getter are uncoupled
• Either setter or getter can be omitted, the behavior in those corner cases is as simple to know as opening nodejs.
• They are functions (which implicitly handles writeonly when there is only a setter).

Less need for documentation, less confusion for ts-users and for ts-developers.

The counter argument to that is that 95% of all implementations are just using get() and set() as an alias for a plain type. However in those cases the user has to option to masquerade the get()/set() implementation with a plain type -

interface A { a: number}
class C implements A { #a = 0; set a(x:number){}; get a():number{ return this.#a; } }
export function createA(): A { return new C(); }

Just my two cents. This opinion is not part of the bug issue.

[fatcerberus]

Either setter or getter can be omitted, the behavior in those corner cases is as simple to know as opening nodejs.

There's one surprising aspect to this: A class with only a setter will provide an automatic getter that returns undefined, even if it inherits from a type with an extant getter for the same property. This has actually caught people off-guard in the past and prompted at least one feature request to change the behavior (which TS won't do because it would deviate from JS spec). So, if you intend the type system to accurately model runtime behavior, the getter and setter for a given property can't be completely uncoupled.

This is actually a source of unsoundness today:
TS Playground

[craigphicks]

Am I correct that you are saying this about A class with only a setter which extends a class with a getter - ?

1. TS behavior is following the JS specs
2. At least Nodejs behavior - (defaults to undefined) - does not follow the JS specs.

If that is true, then of course it is a mess and the Ecma group needs to get its act together and fix that, or least specify that that aspect is left to the implementer.

However, looking at ECMA specs 15.4.2 Static Semantics: HasDirectSuper and 10.1.8.1 OrdinaryGet, I think it might actually specifiy returning undefined as the exactly correct behavior.

From the former 15.4.2 Static Semantics: HasDirectSuper specification:

The syntax-directed operation HasDirectSuper takes no arguments and returns a Boolean. It is defined piecewise over the following productions:
MethodDefinition : ClassElementName ( UniqueFormalParameters ) { FunctionBody }

1. If UniqueFormalParameters Contains SuperCall is true, return true.
2. Return FunctionBody Contains SuperCall.

MethodDefinition : get ClassElementName ( ) { FunctionBody }

1. Return FunctionBody Contains SuperCall.

MethodDefinition : set ClassElementName ( PropertySetParameterList ) { FunctionBody }

1. If PropertySetParameterList Contains SuperCall is true, return true.
2. Return FunctionBody Contains SuperCall.

..... [more Method kinds]

Note: get is the only method kind listed which does not include "If UniqueFormalParameters Contains SuperCall is true, return true".

From the latter 10.1.8.1 OrdinaryGet specification:

The abstract operation OrdinaryGet takes arguments O (an Object), P (a property key), and Receiver (an ECMAScript language value) and returns either a normal completion containing an ECMAScript language value or a throw completion. It performs the following steps when called:

1. Let desc be ? O.[[GetOwnProperty]](P).
2. If desc is undefined, then
   a. Let parent be ? O.[[GetPrototypeOf]]().
   b. If parent is null, return undefined.
   c. Return ? parent.[[Get]](P, Receiver).
3. If IsDataDescriptor(desc) is true, return desc.[[Value]].
4. Assert: IsAccessorDescriptor(desc) is true.
5. Let getter be desc.[[Get]].
6. If getter is undefined, return undefined.
7. Return ? Call(getter, Receiver).

I think desc in line 1 is always defined when either a setter or a getter is defined. So when a setter is defined and a getter isn't, then line 6 is always taken.

I have little experience interpreting the ECMA specs, so you should take my analysis with a pinch of salt.

[fatcerberus]

However, looking at ECMA specs ... I think it might actually specifiy [sic] returning undefined as the exactly correct behavior.

Yes, it is correct ECMAScript behavior that a class with only a setter returns undefined when you access the property, regardless of inheritance. That's why I said

which TS won't do [changing this behavior] because it would deviate from JS spec

The only reason I'm pointing this out is because this behavior means that things aren't quite as simple as treating getters and setters as independent methods in the type system.

[RyanCavanaugh]

I think it would be best to have TypeScript represent prototype set() and/or get() literally - and to honor that behavior as literally as possible.

This issue was foretold: People asked us for divergent get/set types, we pushed back and said "No that will be unsound in many ways that are impractical to seal up", people said "No that's okay, because it's no less unsound than the existing hole around property aliasing", and we said "Yeah good point", and now we're here.

Properties in TypeScript have never been sound on writes, writeonly properties don't exist, and generally writeonly properties in JS are rare enough to not really merit further consideration.

[typescript-bot]

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