Skip to content

Commit c4b13fd

Browse files
committed
Have getAssignmentReducedType use a new relation that checks for weak
types. Fixes microsoft#26405. Also add a debug assert to catch any future cases like microsoft#26130 in which the target type of a valid assignment is narrowed to something to which the source type is no longer assignable.
1 parent e8b72aa commit c4b13fd

File tree

5 files changed

+70
-10
lines changed

5 files changed

+70
-10
lines changed

src/compiler/checker.ts

+14-10
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,8 @@ namespace ts {
609609
const assignableRelation = createMap<RelationComparisonResult>();
610610
const definitelyAssignableRelation = createMap<RelationComparisonResult>();
611611
const comparableRelation = createMap<RelationComparisonResult>();
612+
// Same as comparableRelation except we do check for weak types.
613+
const narrowableRelation = createMap<RelationComparisonResult>();
612614
const identityRelation = createMap<RelationComparisonResult>();
613615
const enumRelation = createMap<RelationComparisonResult>();
614616

@@ -10906,7 +10908,7 @@ namespace ts {
1090610908
if (s & TypeFlags.Null && (!strictNullChecks || t & TypeFlags.Null)) return true;
1090710909
if (s & TypeFlags.Object && t & TypeFlags.NonPrimitive) return true;
1090810910
if (s & TypeFlags.UniqueESSymbol || t & TypeFlags.UniqueESSymbol) return false;
10909-
if (relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation) {
10911+
if (relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation || relation === narrowableRelation) {
1091010912
if (s & TypeFlags.Any) return true;
1091110913
// Type number or any numeric literal type is assignable to any numeric enum type or any
1091210914
// numeric enum literal type. This rule exists for backwards compatibility reasons because
@@ -10925,7 +10927,7 @@ namespace ts {
1092510927
target = (<LiteralType>target).regularType;
1092610928
}
1092710929
if (source === target ||
10928-
relation === comparableRelation && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
10930+
(relation === comparableRelation || relation === narrowableRelation) && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
1092910931
relation !== identityRelation && isSimpleTypeRelatedTo(source, target, relation)) {
1093010932
return true;
1093110933
}
@@ -11025,7 +11027,7 @@ namespace ts {
1102511027
}
1102611028

1102711029
if (!message) {
11028-
if (relation === comparableRelation) {
11030+
if (relation === comparableRelation || relation === narrowableRelation) {
1102911031
message = Diagnostics.Type_0_is_not_comparable_to_type_1;
1103011032
}
1103111033
else if (sourceType === targetType) {
@@ -11120,7 +11122,7 @@ namespace ts {
1112011122
return isIdenticalTo(source, target);
1112111123
}
1112211124

11123-
if (relation === comparableRelation && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
11125+
if ((relation === comparableRelation || relation === narrowableRelation) && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
1112411126
isSimpleTypeRelatedTo(source, target, relation, reportErrors ? reportError : undefined)) return Ternary.True;
1112511127

1112611128
if (isObjectLiteralType(source) && source.flags & TypeFlags.FreshLiteral) {
@@ -11171,7 +11173,7 @@ namespace ts {
1117111173
// we need to deconstruct unions before intersections (because unions are always at the top),
1117211174
// and we need to handle "each" relations before "some" relations for the same kind of type.
1117311175
if (source.flags & TypeFlags.Union) {
11174-
result = relation === comparableRelation ?
11176+
result = (relation === comparableRelation || relation === narrowableRelation) ?
1117511177
someTypeRelatedToType(source as UnionType, target, reportErrors && !(source.flags & TypeFlags.Primitive)) :
1117611178
eachTypeRelatedToType(source as UnionType, target, reportErrors && !(source.flags & TypeFlags.Primitive));
1117711179
}
@@ -11295,7 +11297,7 @@ namespace ts {
1129511297
}
1129611298
if (maybeTypeOfKind(target, TypeFlags.Object) && !(getObjectFlags(target) & ObjectFlags.ObjectLiteralPatternWithComputedProperties)) {
1129711299
const isComparingJsxAttributes = !!(getObjectFlags(source) & ObjectFlags.JsxAttributes);
11298-
if ((relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation) &&
11300+
if ((relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation || relation === narrowableRelation) &&
1129911301
(isTypeSubsetOf(globalObjectType, target) || (!isComparingJsxAttributes && isEmptyObjectType(target)))) {
1130011302
return false;
1130111303
}
@@ -11813,7 +11815,7 @@ namespace ts {
1181311815
// related to Y, where X' is an instantiation of X in which P is replaced with Q. Notice
1181411816
// that S and T are contra-variant whereas X and Y are co-variant.
1181511817
function mappedTypeRelatedTo(source: MappedType, target: MappedType, reportErrors: boolean): Ternary {
11816-
const modifiersRelated = relation === comparableRelation || (relation === identityRelation ? getMappedTypeModifiers(source) === getMappedTypeModifiers(target) :
11818+
const modifiersRelated = relation === comparableRelation || relation === narrowableRelation || (relation === identityRelation ? getMappedTypeModifiers(source) === getMappedTypeModifiers(target) :
1181711819
getCombinedMappedTypeOptionality(source) <= getCombinedMappedTypeOptionality(target));
1181811820
if (modifiersRelated) {
1181911821
let result: Ternary;
@@ -11935,7 +11937,7 @@ namespace ts {
1193511937
}
1193611938
result &= related;
1193711939
// When checking for comparability, be more lenient with optional properties.
11938-
if (relation !== comparableRelation && sourceProp.flags & SymbolFlags.Optional && !(targetProp.flags & SymbolFlags.Optional)) {
11940+
if (relation !== comparableRelation && relation !== narrowableRelation && sourceProp.flags & SymbolFlags.Optional && !(targetProp.flags & SymbolFlags.Optional)) {
1193911941
// TypeScript 1.0 spec (April 2014): 3.8.3
1194011942
// S is a subtype of a type T, and T is a supertype of S if ...
1194111943
// S' and T are object types and, for each member M in T..
@@ -12055,7 +12057,7 @@ namespace ts {
1205512057
// in the context of the target signature before checking the relationship. Ideally we'd do
1205612058
// this regardless of the number of signatures, but the potential costs are prohibitive due
1205712059
// to the quadratic nature of the logic below.
12058-
const eraseGenerics = relation === comparableRelation || !!compilerOptions.noStrictGenericChecks;
12060+
const eraseGenerics = relation === comparableRelation || relation === narrowableRelation || !!compilerOptions.noStrictGenericChecks;
1205912061
result = signatureRelatedTo(sourceSignatures[0], targetSignatures[0], eraseGenerics, reportErrors);
1206012062
}
1206112063
else {
@@ -13878,7 +13880,9 @@ namespace ts {
1387813880
if (assignedType.flags & TypeFlags.Never) {
1387913881
return assignedType;
1388013882
}
13881-
const reducedType = filterType(declaredType, t => isTypeComparableTo(assignedType, t));
13883+
const reducedType = filterType(declaredType, t => isTypeRelatedTo(assignedType, t, narrowableRelation));
13884+
// If the assignment was originally valid, it should still be valid with the reduced target type.
13885+
Debug.assert(!isTypeAssignableTo(assignedType, declaredType) || isTypeAssignableTo(assignedType, reducedType));
1388213886
if (!(reducedType.flags & TypeFlags.Never)) {
1388313887
return reducedType;
1388413888
}

tests/baselines/reference/assignmentTypeNarrowing.js

+8
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ let a: string[];
2727
for (x of a) {
2828
x; // string
2929
}
30+
31+
// Repro from #26405
32+
33+
type AOrArrA<T> = T | T[];
34+
const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
35+
arr.push({ x: "ok" });
3036

3137

3238
//// [assignmentTypeNarrowing.js]
@@ -51,3 +57,5 @@ for (var _i = 0, a_1 = a; _i < a_1.length; _i++) {
5157
x = a_1[_i];
5258
x; // string
5359
}
60+
var arr = [{ x: "ok" }]; // weak type
61+
arr.push({ x: "ok" });

tests/baselines/reference/assignmentTypeNarrowing.symbols

+20
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,23 @@ for (x of a) {
6262
>x : Symbol(x, Decl(assignmentTypeNarrowing.ts, 0, 3))
6363
}
6464

65+
// Repro from #26405
66+
67+
type AOrArrA<T> = T | T[];
68+
>AOrArrA : Symbol(AOrArrA, Decl(assignmentTypeNarrowing.ts, 27, 1))
69+
>T : Symbol(T, Decl(assignmentTypeNarrowing.ts, 31, 13))
70+
>T : Symbol(T, Decl(assignmentTypeNarrowing.ts, 31, 13))
71+
>T : Symbol(T, Decl(assignmentTypeNarrowing.ts, 31, 13))
72+
73+
const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
74+
>arr : Symbol(arr, Decl(assignmentTypeNarrowing.ts, 32, 5))
75+
>AOrArrA : Symbol(AOrArrA, Decl(assignmentTypeNarrowing.ts, 27, 1))
76+
>x : Symbol(x, Decl(assignmentTypeNarrowing.ts, 32, 20))
77+
>x : Symbol(x, Decl(assignmentTypeNarrowing.ts, 32, 35))
78+
79+
arr.push({ x: "ok" });
80+
>arr.push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --))
81+
>arr : Symbol(arr, Decl(assignmentTypeNarrowing.ts, 32, 5))
82+
>push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --))
83+
>x : Symbol(x, Decl(assignmentTypeNarrowing.ts, 33, 10))
84+

tests/baselines/reference/assignmentTypeNarrowing.types

+22
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,25 @@ for (x of a) {
9696
>x : string
9797
}
9898

99+
// Repro from #26405
100+
101+
type AOrArrA<T> = T | T[];
102+
>AOrArrA : AOrArrA<T>
103+
104+
const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
105+
>arr : AOrArrA<{ x?: "ok"; }>
106+
>x : "ok"
107+
>[{ x: "ok" }] : { x: "ok"; }[]
108+
>{ x: "ok" } : { x: "ok"; }
109+
>x : "ok"
110+
>"ok" : "ok"
111+
112+
arr.push({ x: "ok" });
113+
>arr.push({ x: "ok" }) : number
114+
>arr.push : (...items: { x?: "ok"; }[]) => number
115+
>arr : { x?: "ok"; }[]
116+
>push : (...items: { x?: "ok"; }[]) => number
117+
>{ x: "ok" } : { x: "ok"; }
118+
>x : "ok"
119+
>"ok" : "ok"
120+

tests/cases/conformance/expressions/assignmentOperator/assignmentTypeNarrowing.ts

+6
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,9 @@ let a: string[];
2626
for (x of a) {
2727
x; // string
2828
}
29+
30+
// Repro from #26405
31+
32+
type AOrArrA<T> = T | T[];
33+
const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
34+
arr.push({ x: "ok" });

0 commit comments

Comments
 (0)