Skip to content

Commit e204acf

Browse files
weswighamweswigham
authored
Ensure subtype relation ordering for readonly properties (microsoft#47069)
* Ensure subtype relation ordering for readonly properties * Probably fix post-LKG assignability error
1 parent 033f9e0 commit e204acf

7 files changed

+705
-1
lines changed

‎src/compiler/checker.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19732,6 +19732,18 @@ namespace ts {
1973219732
}
1973319733
return Ternary.False;
1973419734
}
19735+
19736+
// Ensure {readonly a: whatever} is not a subtype of {a: whatever},
19737+
// while {a: whatever} is a subtype of {readonly a: whatever}.
19738+
// This ensures the subtype relationship is ordered, and preventing declaration order
19739+
// from deciding which type "wins" in union subtype reduction.
19740+
// They're still assignable to one another, since `readonly` doesn't affect assignability.
19741+
if (
19742+
(relation === subtypeRelation || relation === strictSubtypeRelation) &&
19743+
!!(sourcePropFlags & ModifierFlags.Readonly) && !(targetPropFlags & ModifierFlags.Readonly)
19744+
) {
19745+
return Ternary.False;
19746+
}
1973519747
// If the target comes from a partial union prop, allow `undefined` in the target type
1973619748
const related = isPropertySymbolTypeRelated(sourceProp, targetProp, getTypeOfSourceProperty, reportErrors, intersectionState);
1973719749
if (!related) {

‎src/services/textChanges.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ namespace ts.textChanges {
303303
export class ChangeTracker {
304304
private readonly changes: Change[] = [];
305305
private readonly newFiles: { readonly oldFile: SourceFile | undefined, readonly fileName: string, readonly statements: readonly (Statement | SyntaxKind.NewLineTrivia)[] }[] = [];
306-
private readonly classesWithNodesInsertedAtStart = new Map<number, { readonly node: ClassDeclaration | InterfaceDeclaration | ObjectLiteralExpression, readonly sourceFile: SourceFile }>(); // Set<ClassDeclaration> implemented as Map<node id, ClassDeclaration>
306+
private readonly classesWithNodesInsertedAtStart = new Map<number, { readonly node: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression, readonly sourceFile: SourceFile }>(); // Set<ClassDeclaration> implemented as Map<node id, ClassDeclaration>
307307
private readonly deletedNodes: { readonly sourceFile: SourceFile, readonly node: Node | NodeArray<TypeParameterDeclaration> }[] = [];
308308

309309
public static fromContext(context: TextChangesContext): ChangeTracker {

‎tests/baselines/reference/readonlyPropertySubtypeRelationDirected.errors.txt

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
tests/cases/compiler/four.ts(11,11): error TS2540: Cannot assign to 'a' because it is a read-only property.
2+
tests/cases/compiler/four.ts(15,11): error TS2540: Cannot assign to 'a' because it is a read-only property.
3+
tests/cases/compiler/one.ts(11,11): error TS2540: Cannot assign to 'a' because it is a read-only property.
4+
tests/cases/compiler/one.ts(15,11): error TS2540: Cannot assign to 'a' because it is a read-only property.
5+
tests/cases/compiler/three.ts(11,11): error TS2540: Cannot assign to 'a' because it is a read-only property.
6+
tests/cases/compiler/three.ts(15,11): error TS2540: Cannot assign to 'a' because it is a read-only property.
7+
tests/cases/compiler/two.ts(11,11): error TS2540: Cannot assign to 'a' because it is a read-only property.
8+
tests/cases/compiler/two.ts(15,11): error TS2540: Cannot assign to 'a' because it is a read-only property.
9+
10+
11+
==== tests/cases/compiler/one.ts (2 errors) ====
12+
export {};
13+
// When the non-readonly type is declared first, the unioned type of `three` in `doSomething` is never treated as readonly
14+
const two: { a: string } = { a: 'two' };
15+
const one: { readonly a: string } = { a: 'one' };
16+
17+
function doSomething(condition: boolean) {
18+
// when `one` comes first in the conditional check, the return type of `doSomething` is inferred as `a` is readonly, but `a` is
19+
// only treated as readonly (i.e. it will produce a diagnostic if you try to assign to it) based on the order of declarations of `one` and `two` above
20+
const three = (condition) ? one : two;
21+
22+
three.a = 'foo';
23+
~
24+
!!! error TS2540: Cannot assign to 'a' because it is a read-only property.
25+
26+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
27+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
28+
three.a = 'foo2';
29+
~
30+
!!! error TS2540: Cannot assign to 'a' because it is a read-only property.
31+
32+
return three;
33+
}
34+
==== tests/cases/compiler/two.ts (2 errors) ====
35+
export {};
36+
// When the non-readonly type is declared first, the unioned type of `three` in `doSomething` is never treated as readonly
37+
const two: { a: string } = { a: 'two' };
38+
const one: { readonly a: string } = { a: 'one' };
39+
40+
function doSomething(condition: boolean) {
41+
// when `two` comes first in the conditional check, the return type of `doSomething` is inferred as not readonly but produces the same diagnostics as above
42+
// based on the declaration order of `one` and `two`
43+
const three = (condition) ? two : one;
44+
45+
three.a = 'foo';
46+
~
47+
!!! error TS2540: Cannot assign to 'a' because it is a read-only property.
48+
49+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
50+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
51+
three.a = 'foo2';
52+
~
53+
!!! error TS2540: Cannot assign to 'a' because it is a read-only property.
54+
55+
return three;
56+
}
57+
58+
==== tests/cases/compiler/three.ts (2 errors) ====
59+
export {};
60+
// When the readonly type is declared first, the unioned type of `three` in `doSomething` is always treated as readonly by the compiler
61+
const one: { readonly a: string } = { a: 'one' };
62+
const two: { a: string } = { a: 'two' };
63+
64+
function doSomething(condition: boolean) {
65+
// when `one` comes first in the conditional check, the return type of `doSomething` is inferred as `a` is readonly, but `a` is
66+
// only treated as readonly (i.e. it will produce a diagnostic if you try to assign to it) based on the order of declarations of `one` and `two` above
67+
const three = (condition) ? one : two;
68+
69+
three.a = 'foo';
70+
~
71+
!!! error TS2540: Cannot assign to 'a' because it is a read-only property.
72+
73+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
74+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
75+
three.a = 'foo2';
76+
~
77+
!!! error TS2540: Cannot assign to 'a' because it is a read-only property.
78+
79+
return three;
80+
}
81+
82+
==== tests/cases/compiler/four.ts (2 errors) ====
83+
export {};
84+
// When the readonly type is declared first, the unioned type of `three` in `doSomething` is always treated as readonly by the compiler
85+
const one: { readonly a: string } = { a: 'one' };
86+
const two: { a: string } = { a: 'two' };
87+
88+
function doSomething(condition: boolean) {
89+
// when `two` comes first in the conditional check, the return type of `doSomething` is inferred as not readonly but produces the same diagnostics as above
90+
// based on the declaration order of `one` and `two`
91+
const three = (condition) ? two : one;
92+
93+
three.a = 'foo';
94+
~
95+
!!! error TS2540: Cannot assign to 'a' because it is a read-only property.
96+
97+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
98+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
99+
three.a = 'foo2';
100+
~
101+
!!! error TS2540: Cannot assign to 'a' because it is a read-only property.
102+
103+
return three;
104+
}

‎tests/baselines/reference/readonlyPropertySubtypeRelationDirected.js

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
//// [tests/cases/compiler/readonlyPropertySubtypeRelationDirected.ts] ////
2+
3+
//// [one.ts]
4+
export {};
5+
// When the non-readonly type is declared first, the unioned type of `three` in `doSomething` is never treated as readonly
6+
const two: { a: string } = { a: 'two' };
7+
const one: { readonly a: string } = { a: 'one' };
8+
9+
function doSomething(condition: boolean) {
10+
// when `one` comes first in the conditional check, the return type of `doSomething` is inferred as `a` is readonly, but `a` is
11+
// only treated as readonly (i.e. it will produce a diagnostic if you try to assign to it) based on the order of declarations of `one` and `two` above
12+
const three = (condition) ? one : two;
13+
14+
three.a = 'foo';
15+
16+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
17+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
18+
three.a = 'foo2';
19+
20+
return three;
21+
}
22+
//// [two.ts]
23+
export {};
24+
// When the non-readonly type is declared first, the unioned type of `three` in `doSomething` is never treated as readonly
25+
const two: { a: string } = { a: 'two' };
26+
const one: { readonly a: string } = { a: 'one' };
27+
28+
function doSomething(condition: boolean) {
29+
// when `two` comes first in the conditional check, the return type of `doSomething` is inferred as not readonly but produces the same diagnostics as above
30+
// based on the declaration order of `one` and `two`
31+
const three = (condition) ? two : one;
32+
33+
three.a = 'foo';
34+
35+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
36+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
37+
three.a = 'foo2';
38+
39+
return three;
40+
}
41+
42+
//// [three.ts]
43+
export {};
44+
// When the readonly type is declared first, the unioned type of `three` in `doSomething` is always treated as readonly by the compiler
45+
const one: { readonly a: string } = { a: 'one' };
46+
const two: { a: string } = { a: 'two' };
47+
48+
function doSomething(condition: boolean) {
49+
// when `one` comes first in the conditional check, the return type of `doSomething` is inferred as `a` is readonly, but `a` is
50+
// only treated as readonly (i.e. it will produce a diagnostic if you try to assign to it) based on the order of declarations of `one` and `two` above
51+
const three = (condition) ? one : two;
52+
53+
three.a = 'foo';
54+
55+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
56+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
57+
three.a = 'foo2';
58+
59+
return three;
60+
}
61+
62+
//// [four.ts]
63+
export {};
64+
// When the readonly type is declared first, the unioned type of `three` in `doSomething` is always treated as readonly by the compiler
65+
const one: { readonly a: string } = { a: 'one' };
66+
const two: { a: string } = { a: 'two' };
67+
68+
function doSomething(condition: boolean) {
69+
// when `two` comes first in the conditional check, the return type of `doSomething` is inferred as not readonly but produces the same diagnostics as above
70+
// based on the declaration order of `one` and `two`
71+
const three = (condition) ? two : one;
72+
73+
three.a = 'foo';
74+
75+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
76+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
77+
three.a = 'foo2';
78+
79+
return three;
80+
}
81+
82+
//// [one.js]
83+
"use strict";
84+
exports.__esModule = true;
85+
// When the non-readonly type is declared first, the unioned type of `three` in `doSomething` is never treated as readonly
86+
var two = { a: 'two' };
87+
var one = { a: 'one' };
88+
function doSomething(condition) {
89+
// when `one` comes first in the conditional check, the return type of `doSomething` is inferred as `a` is readonly, but `a` is
90+
// only treated as readonly (i.e. it will produce a diagnostic if you try to assign to it) based on the order of declarations of `one` and `two` above
91+
var three = (condition) ? one : two;
92+
three.a = 'foo';
93+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
94+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
95+
three.a = 'foo2';
96+
return three;
97+
}
98+
//// [two.js]
99+
"use strict";
100+
exports.__esModule = true;
101+
// When the non-readonly type is declared first, the unioned type of `three` in `doSomething` is never treated as readonly
102+
var two = { a: 'two' };
103+
var one = { a: 'one' };
104+
function doSomething(condition) {
105+
// when `two` comes first in the conditional check, the return type of `doSomething` is inferred as not readonly but produces the same diagnostics as above
106+
// based on the declaration order of `one` and `two`
107+
var three = (condition) ? two : one;
108+
three.a = 'foo';
109+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
110+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
111+
three.a = 'foo2';
112+
return three;
113+
}
114+
//// [three.js]
115+
"use strict";
116+
exports.__esModule = true;
117+
// When the readonly type is declared first, the unioned type of `three` in `doSomething` is always treated as readonly by the compiler
118+
var one = { a: 'one' };
119+
var two = { a: 'two' };
120+
function doSomething(condition) {
121+
// when `one` comes first in the conditional check, the return type of `doSomething` is inferred as `a` is readonly, but `a` is
122+
// only treated as readonly (i.e. it will produce a diagnostic if you try to assign to it) based on the order of declarations of `one` and `two` above
123+
var three = (condition) ? one : two;
124+
three.a = 'foo';
125+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
126+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
127+
three.a = 'foo2';
128+
return three;
129+
}
130+
//// [four.js]
131+
"use strict";
132+
exports.__esModule = true;
133+
// When the readonly type is declared first, the unioned type of `three` in `doSomething` is always treated as readonly by the compiler
134+
var one = { a: 'one' };
135+
var two = { a: 'two' };
136+
function doSomething(condition) {
137+
// when `two` comes first in the conditional check, the return type of `doSomething` is inferred as not readonly but produces the same diagnostics as above
138+
// based on the declaration order of `one` and `two`
139+
var three = (condition) ? two : one;
140+
three.a = 'foo';
141+
// the inferred (displayed?) type of `a` also depends on the order of the condition above. When `one` comes first, the displayed type is `any`
142+
// when `two` comes first, the displayed type is `string`, but the diagnostic will always correctly find that it's string
143+
three.a = 'foo2';
144+
return three;
145+
}

0 commit comments

Comments
 (0)