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

Fixed union/intersection write types #56895

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11979,7 +11979,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (!links.writeType && links.deferralWriteConstituents) {
Debug.assertIsDefined(links.deferralParent);
Debug.assertIsDefined(links.deferralConstituents);
links.writeType = links.deferralParent.flags & TypeFlags.Union ? getUnionType(links.deferralWriteConstituents) : getIntersectionType(links.deferralWriteConstituents);
links.writeType = links.deferralParent.flags & TypeFlags.Union ? getIntersectionType(links.deferralWriteConstituents) : getUnionType(links.deferralWriteConstituents);
}
return links.writeType;
}
Expand Down Expand Up @@ -14674,7 +14674,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
else {
result.links.type = isUnion ? getUnionType(propTypes) : getIntersectionType(propTypes);
if (writeTypes) {
result.links.writeType = isUnion ? getUnionType(writeTypes) : getIntersectionType(writeTypes);
result.links.writeType = isUnion ? getIntersectionType(writeTypes) : getUnionType(writeTypes);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after the original change has been merged it has been mentioned in the comments that likely this should work in the way I have adjusted it here but I don't see any open issue that would be the result of this discussion there

Copy link

@craigphicks craigphicks Dec 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Currently the getters and setter are displayed as plain readonly property (for getter) and plain property (for setter). See setter inferred from union has incorrect variance #56894 and Flow is improperly narrowing getter values #56899 which show the display. I think the displayed types should show getter and setter declarations so the user can understand the difference in behavior.
  2. What about the case where if (propTypes.length > 2){ ... } before this else clause? Isn't that branch handled later in different code which also has to be updated?
  3. A question: Apparently the writeTypes exists here for setters, because test results changed. But the tests results didn't change for plain types, so writeTypes does not exist for them, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. if I understand you correctly - what is displayed depends on the location. In write locations, the write type is displayed and in read locations, you get the read type. To change this the TS team would have to receive a well-motivated feature request with many examples of how it would differ from the current behavior.
  2. Yes - you are right. I'll take a look at this.
  3. What do you consider a plain type here? Could you rephrase this question?

}
}
return result;
Expand Down
64 changes: 64 additions & 0 deletions tests/baselines/reference/divergentAccessorsTypes3.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
divergentAccessorsTypes3.ts(33,1): error TS2322: Type 'number' is not assignable to type 'string'.
divergentAccessorsTypes3.ts(36,1): error TS2322: Type 'number' is not assignable to type 'never'.
divergentAccessorsTypes3.ts(37,1): error TS2322: Type 'string' is not assignable to type 'never'.
divergentAccessorsTypes3.ts(38,1): error TS2322: Type 'boolean' is not assignable to type 'never'.
divergentAccessorsTypes3.ts(40,1): error TS2322: Type 'number' is not assignable to type 'string'.
divergentAccessorsTypes3.ts(42,1): error TS2322: Type 'boolean' is not assignable to type 'string'.


==== divergentAccessorsTypes3.ts (6 errors) ====
class One {
get prop1(): string { return ""; }
set prop1(s: string | number) { }

get prop2(): string { return ""; }
set prop2(s: string | number) { }

prop3: number;

get prop4(): string { return ""; }
set prop4(s: string | number) { }
}

class Two {
get prop1(): string { return ""; }
set prop1(s: string | number) { }

get prop2(): string { return ""; }
set prop2(s: string) { }

get prop3(): string { return ""; }
set prop3(s: string | boolean) { }

get prop4(): string { return ""; }
set prop4(s: string | boolean) { }
}

declare const u1: One|Two;

u1.prop1 = 42;
u1.prop1 = "hello";

u1.prop2 = 42;
~~~~~~~~
!!! error TS2322: Type 'number' is not assignable to type 'string'.
u1.prop2 = "hello";

u1.prop3 = 42;
~~~~~~~~
!!! error TS2322: Type 'number' is not assignable to type 'never'.
u1.prop3 = "hello";
~~~~~~~~
!!! error TS2322: Type 'string' is not assignable to type 'never'.
u1.prop3 = true;
~~~~~~~~
!!! error TS2322: Type 'boolean' is not assignable to type 'never'.

u1.prop4 = 42;
~~~~~~~~
!!! error TS2322: Type 'number' is not assignable to type 'string'.
u1.prop4 = "hello";
u1.prop4 = true;
~~~~~~~~
!!! error TS2322: Type 'boolean' is not assignable to type 'string'.

32 changes: 16 additions & 16 deletions tests/baselines/reference/divergentAccessorsTypes3.types
Original file line number Diff line number Diff line change
Expand Up @@ -87,57 +87,57 @@ u1.prop1 = "hello";

u1.prop2 = 42;
>u1.prop2 = 42 : 42
>u1.prop2 : string | number
>u1.prop2 : string
>u1 : One | Two
>prop2 : string | number
>prop2 : string
>42 : 42

u1.prop2 = "hello";
>u1.prop2 = "hello" : "hello"
>u1.prop2 : string | number
>u1.prop2 : string
>u1 : One | Two
>prop2 : string | number
>prop2 : string
>"hello" : "hello"

u1.prop3 = 42;
>u1.prop3 = 42 : 42
>u1.prop3 : string | number | boolean
>u1.prop3 : never
>u1 : One | Two
>prop3 : string | number | boolean
>prop3 : never
>42 : 42

u1.prop3 = "hello";
>u1.prop3 = "hello" : "hello"
>u1.prop3 : string | number | boolean
>u1.prop3 : never
>u1 : One | Two
>prop3 : string | number | boolean
>prop3 : never
>"hello" : "hello"

u1.prop3 = true;
>u1.prop3 = true : true
>u1.prop3 : string | number | boolean
>u1.prop3 : never
>u1 : One | Two
>prop3 : string | number | boolean
>prop3 : never
>true : true

u1.prop4 = 42;
>u1.prop4 = 42 : 42
>u1.prop4 : string | number | boolean
>u1.prop4 : string
>u1 : One | Two
>prop4 : string | number | boolean
>prop4 : string
>42 : 42

u1.prop4 = "hello";
>u1.prop4 = "hello" : "hello"
>u1.prop4 : string | number | boolean
>u1.prop4 : string
>u1 : One | Two
>prop4 : string | number | boolean
>prop4 : string
>"hello" : "hello"

u1.prop4 = true;
>u1.prop4 = true : true
>u1.prop4 : string | number | boolean
>u1.prop4 : string
>u1 : One | Two
>prop4 : string | number | boolean
>prop4 : string
>true : true

36 changes: 0 additions & 36 deletions tests/baselines/reference/divergentAccessorsTypes4.errors.txt

This file was deleted.

12 changes: 6 additions & 6 deletions tests/baselines/reference/divergentAccessorsTypes4.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ declare const i: One & Two;

// "hello"
i.prop1;
// number | "hello"
// string | number
i.prop1 = 42;
i.prop1 = "hello";

// never
i.prop2;
// 42
// string | number
i.prop2 = 42;
i.prop2 = "hello"; // error
i.prop2 = "hello";


//// [divergentAccessorsTypes4.js]
Expand Down Expand Up @@ -63,11 +63,11 @@ var Two = /** @class */ (function () {
}());
// "hello"
i.prop1;
// number | "hello"
// string | number
i.prop1 = 42;
i.prop1 = "hello";
// never
i.prop2;
// 42
// string | number
i.prop2 = 42;
i.prop2 = "hello"; // error
i.prop2 = "hello";
6 changes: 3 additions & 3 deletions tests/baselines/reference/divergentAccessorsTypes4.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ i.prop1;
>i : Symbol(i, Decl(divergentAccessorsTypes4.ts, 16, 13))
>prop1 : Symbol(prop1, Decl(divergentAccessorsTypes4.ts, 0, 11), Decl(divergentAccessorsTypes4.ts, 1, 36), Decl(divergentAccessorsTypes4.ts, 7, 11), Decl(divergentAccessorsTypes4.ts, 8, 42))

// number | "hello"
// string | number
i.prop1 = 42;
>i.prop1 : Symbol(prop1, Decl(divergentAccessorsTypes4.ts, 0, 11), Decl(divergentAccessorsTypes4.ts, 1, 36), Decl(divergentAccessorsTypes4.ts, 7, 11), Decl(divergentAccessorsTypes4.ts, 8, 42))
>i : Symbol(i, Decl(divergentAccessorsTypes4.ts, 16, 13))
Expand All @@ -62,13 +62,13 @@ i.prop2;
>i : Symbol(i, Decl(divergentAccessorsTypes4.ts, 16, 13))
>prop2 : Symbol(prop2, Decl(divergentAccessorsTypes4.ts, 2, 35), Decl(divergentAccessorsTypes4.ts, 9, 36), Decl(divergentAccessorsTypes4.ts, 11, 36))

// 42
// string | number
i.prop2 = 42;
>i.prop2 : Symbol(prop2, Decl(divergentAccessorsTypes4.ts, 2, 35), Decl(divergentAccessorsTypes4.ts, 9, 36), Decl(divergentAccessorsTypes4.ts, 11, 36))
>i : Symbol(i, Decl(divergentAccessorsTypes4.ts, 16, 13))
>prop2 : Symbol(prop2, Decl(divergentAccessorsTypes4.ts, 2, 35), Decl(divergentAccessorsTypes4.ts, 9, 36), Decl(divergentAccessorsTypes4.ts, 11, 36))

i.prop2 = "hello"; // error
i.prop2 = "hello";
>i.prop2 : Symbol(prop2, Decl(divergentAccessorsTypes4.ts, 2, 35), Decl(divergentAccessorsTypes4.ts, 9, 36), Decl(divergentAccessorsTypes4.ts, 11, 36))
>i : Symbol(i, Decl(divergentAccessorsTypes4.ts, 16, 13))
>prop2 : Symbol(prop2, Decl(divergentAccessorsTypes4.ts, 2, 35), Decl(divergentAccessorsTypes4.ts, 9, 36), Decl(divergentAccessorsTypes4.ts, 11, 36))
Expand Down
22 changes: 11 additions & 11 deletions tests/baselines/reference/divergentAccessorsTypes4.types
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ i.prop1;
>i : One & Two
>prop1 : "hello"

// number | "hello"
// string | number
i.prop1 = 42;
>i.prop1 = 42 : 42
>i.prop1 : number | "hello"
>i.prop1 : string | number
>i : One & Two
>prop1 : number | "hello"
>prop1 : string | number
>42 : 42

i.prop1 = "hello";
>i.prop1 = "hello" : "hello"
>i.prop1 : number | "hello"
>i.prop1 : string | number
>i : One & Two
>prop1 : number | "hello"
>prop1 : string | number
>"hello" : "hello"

// never
Expand All @@ -67,18 +67,18 @@ i.prop2;
>i : One & Two
>prop2 : never

// 42
// string | number
i.prop2 = 42;
>i.prop2 = 42 : 42
>i.prop2 : 42
>i.prop2 : string | number
>i : One & Two
>prop2 : 42
>prop2 : string | number
>42 : 42

i.prop2 = "hello"; // error
i.prop2 = "hello";
>i.prop2 = "hello" : "hello"
>i.prop2 : 42
>i.prop2 : string | number
>i : One & Two
>prop2 : 42
>prop2 : string | number
>"hello" : "hello"

46 changes: 0 additions & 46 deletions tests/baselines/reference/divergentAccessorsTypes5.errors.txt

This file was deleted.

8 changes: 4 additions & 4 deletions tests/baselines/reference/divergentAccessorsTypes5.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ class Three {
declare const i: One & Two & Three;

// "hello"
i.prop1 = 42; // error
i.prop1 = 42;
i.prop1 = "hello";

// 42
i.prop2 = 42;
i.prop2 = "hello"; // error
i.prop2 = "hello";


//// [divergentAccessorsTypes5.js]
Expand Down Expand Up @@ -88,8 +88,8 @@ var Three = /** @class */ (function () {
return Three;
}());
// "hello"
i.prop1 = 42; // error
i.prop1 = 42;
i.prop1 = "hello";
// 42
i.prop2 = 42;
i.prop2 = "hello"; // error
i.prop2 = "hello";
4 changes: 2 additions & 2 deletions tests/baselines/reference/divergentAccessorsTypes5.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ declare const i: One & Two & Three;
>Three : Symbol(Three, Decl(divergentAccessorsTypes5.ts, 17, 1))

// "hello"
i.prop1 = 42; // error
i.prop1 = 42;
>i.prop1 : Symbol(prop1, Decl(divergentAccessorsTypes5.ts, 3, 11), Decl(divergentAccessorsTypes5.ts, 4, 36), Decl(divergentAccessorsTypes5.ts, 10, 11), Decl(divergentAccessorsTypes5.ts, 11, 42), Decl(divergentAccessorsTypes5.ts, 19, 13) ... and 1 more)
>i : Symbol(i, Decl(divergentAccessorsTypes5.ts, 27, 13))
>prop1 : Symbol(prop1, Decl(divergentAccessorsTypes5.ts, 3, 11), Decl(divergentAccessorsTypes5.ts, 4, 36), Decl(divergentAccessorsTypes5.ts, 10, 11), Decl(divergentAccessorsTypes5.ts, 11, 42), Decl(divergentAccessorsTypes5.ts, 19, 13) ... and 1 more)
Expand All @@ -78,7 +78,7 @@ i.prop2 = 42;
>i : Symbol(i, Decl(divergentAccessorsTypes5.ts, 27, 13))
>prop2 : Symbol(prop2, Decl(divergentAccessorsTypes5.ts, 5, 35), Decl(divergentAccessorsTypes5.ts, 12, 36), Decl(divergentAccessorsTypes5.ts, 14, 36), Decl(divergentAccessorsTypes5.ts, 21, 37), Decl(divergentAccessorsTypes5.ts, 23, 36))

i.prop2 = "hello"; // error
i.prop2 = "hello";
>i.prop2 : Symbol(prop2, Decl(divergentAccessorsTypes5.ts, 5, 35), Decl(divergentAccessorsTypes5.ts, 12, 36), Decl(divergentAccessorsTypes5.ts, 14, 36), Decl(divergentAccessorsTypes5.ts, 21, 37), Decl(divergentAccessorsTypes5.ts, 23, 36))
>i : Symbol(i, Decl(divergentAccessorsTypes5.ts, 27, 13))
>prop2 : Symbol(prop2, Decl(divergentAccessorsTypes5.ts, 5, 35), Decl(divergentAccessorsTypes5.ts, 12, 36), Decl(divergentAccessorsTypes5.ts, 14, 36), Decl(divergentAccessorsTypes5.ts, 21, 37), Decl(divergentAccessorsTypes5.ts, 23, 36))
Expand Down
Loading
Loading