Repository navigation
treating setters as properties rather than functions can be unsoundΒ #56922
Description
π 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
π» 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.
Activity
Andarist commentedon Jan 2, 2024
I think those are 2 separate issues. My PR fixes
fu2but TypeScript doesn't support write-only properties today and thusfu3stays unchanged.fatcerberus commentedon Jan 2, 2024
Issue for
writeonlyproperties: #21759craigphicks commentedon Jan 2, 2024
I thought
writeonlysuggestion #21795 was interesting. However, I guess I am more of a JS-literalist at heart, andwriteonlyis not a JS term, butset [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.
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 -
Just my two cents. This opinion is not part of the bug issue.
fatcerberus commentedon Jan 2, 2024
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 commentedon Jan 2, 2024
Am I correct that you are saying this about A class with only a setter which extends a class with a getter - ?
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
undefinedas the exactly correct behavior.From the former 15.4.2 Static Semantics: HasDirectSuper specification:
Note:
getis 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:
I think
descin 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 commentedon Jan 2, 2024
Yes, it is correct ECMAScript behavior that a class with only a setter returns
undefinedwhen you access the property, regardless of inheritance. That's why I saidThe 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 commentedon Jan 2, 2024
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 commentedon Jan 7, 2024
This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.