Skip to content

Commit

Permalink
Have getAssignmentReducedType use a new relation that checks for weak
Browse files Browse the repository at this point in the history 
types.

Fixes #26405.

Also add a debug assert to catch any future cases like #26130 in which
the target type of a valid assignment is narrowed to something to which
the source type is no longer assignable.
  • Loading branch information
@mattmccutchen
mattmccutchen committed Aug 13, 2018
1 parent e8b72aa commit c4b13fd
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 10 deletions.
24 changes: 14 additions & 10 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,8 @@ namespace ts {
const assignableRelation = createMap<RelationComparisonResult>();
const definitelyAssignableRelation = createMap<RelationComparisonResult>();
const comparableRelation = createMap<RelationComparisonResult>();
// Same as comparableRelation except we do check for weak types.
const narrowableRelation = createMap<RelationComparisonResult>();
const identityRelation = createMap<RelationComparisonResult>();
const enumRelation = createMap<RelationComparisonResult>();

Expand Down Expand Up @@ -10906,7 +10908,7 @@ namespace ts {
if (s & TypeFlags.Null && (!strictNullChecks || t & TypeFlags.Null)) return true;
if (s & TypeFlags.Object && t & TypeFlags.NonPrimitive) return true;
if (s & TypeFlags.UniqueESSymbol || t & TypeFlags.UniqueESSymbol) return false;
if (relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation) {
if (relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation || relation === narrowableRelation) {
if (s & TypeFlags.Any) return true;
// Type number or any numeric literal type is assignable to any numeric enum type or any
// numeric enum literal type. This rule exists for backwards compatibility reasons because
Expand All @@ -10925,7 +10927,7 @@ namespace ts {
target = (<LiteralType>target).regularType;
}
if (source === target ||
relation === comparableRelation && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
(relation === comparableRelation || relation === narrowableRelation) && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
relation !== identityRelation && isSimpleTypeRelatedTo(source, target, relation)) {
return true;
}
Expand Down Expand Up @@ -11025,7 +11027,7 @@ namespace ts {
}

if (!message) {
if (relation === comparableRelation) {
if (relation === comparableRelation || relation === narrowableRelation) {
message = Diagnostics.Type_0_is_not_comparable_to_type_1;
}
else if (sourceType === targetType) {
Expand Down Expand Up @@ -11120,7 +11122,7 @@ namespace ts {
return isIdenticalTo(source, target);
}

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

if (isObjectLiteralType(source) && source.flags & TypeFlags.FreshLiteral) {
Expand Down Expand Up @@ -11171,7 +11173,7 @@ namespace ts {
// we need to deconstruct unions before intersections (because unions are always at the top),
// and we need to handle "each" relations before "some" relations for the same kind of type.
if (source.flags & TypeFlags.Union) {
result = relation === comparableRelation ?
result = (relation === comparableRelation || relation === narrowableRelation) ?
someTypeRelatedToType(source as UnionType, target, reportErrors && !(source.flags & TypeFlags.Primitive)) :
eachTypeRelatedToType(source as UnionType, target, reportErrors && !(source.flags & TypeFlags.Primitive));
}
Expand Down Expand Up @@ -11295,7 +11297,7 @@ namespace ts {
}
if (maybeTypeOfKind(target, TypeFlags.Object) && !(getObjectFlags(target) & ObjectFlags.ObjectLiteralPatternWithComputedProperties)) {
const isComparingJsxAttributes = !!(getObjectFlags(source) & ObjectFlags.JsxAttributes);
if ((relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation) &&
if ((relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation || relation === narrowableRelation) &&
(isTypeSubsetOf(globalObjectType, target) || (!isComparingJsxAttributes && isEmptyObjectType(target)))) {
return false;
}
Expand Down Expand Up @@ -11813,7 +11815,7 @@ namespace ts {
// related to Y, where X' is an instantiation of X in which P is replaced with Q. Notice
// that S and T are contra-variant whereas X and Y are co-variant.
function mappedTypeRelatedTo(source: MappedType, target: MappedType, reportErrors: boolean): Ternary {
const modifiersRelated = relation === comparableRelation || (relation === identityRelation ? getMappedTypeModifiers(source) === getMappedTypeModifiers(target) :
const modifiersRelated = relation === comparableRelation || relation === narrowableRelation || (relation === identityRelation ? getMappedTypeModifiers(source) === getMappedTypeModifiers(target) :
getCombinedMappedTypeOptionality(source) <= getCombinedMappedTypeOptionality(target));
if (modifiersRelated) {
let result: Ternary;
Expand Down Expand Up @@ -11935,7 +11937,7 @@ namespace ts {
}
result &= related;
// When checking for comparability, be more lenient with optional properties.
if (relation !== comparableRelation && sourceProp.flags & SymbolFlags.Optional && !(targetProp.flags & SymbolFlags.Optional)) {
if (relation !== comparableRelation && relation !== narrowableRelation && sourceProp.flags & SymbolFlags.Optional && !(targetProp.flags & SymbolFlags.Optional)) {
// TypeScript 1.0 spec (April 2014): 3.8.3
// S is a subtype of a type T, and T is a supertype of S if ...
// S' and T are object types and, for each member M in T..
Expand Down Expand Up @@ -12055,7 +12057,7 @@ namespace ts {
// in the context of the target signature before checking the relationship. Ideally we'd do
// this regardless of the number of signatures, but the potential costs are prohibitive due
// to the quadratic nature of the logic below.
const eraseGenerics = relation === comparableRelation || !!compilerOptions.noStrictGenericChecks;
const eraseGenerics = relation === comparableRelation || relation === narrowableRelation || !!compilerOptions.noStrictGenericChecks;
result = signatureRelatedTo(sourceSignatures[0], targetSignatures[0], eraseGenerics, reportErrors);
}
else {
Expand Down Expand Up @@ -13878,7 +13880,9 @@ namespace ts {
if (assignedType.flags & TypeFlags.Never) {
return assignedType;
}
const reducedType = filterType(declaredType, t => isTypeComparableTo(assignedType, t));
const reducedType = filterType(declaredType, t => isTypeRelatedTo(assignedType, t, narrowableRelation));
// If the assignment was originally valid, it should still be valid with the reduced target type.
Debug.assert(!isTypeAssignableTo(assignedType, declaredType) || isTypeAssignableTo(assignedType, reducedType));
if (!(reducedType.flags & TypeFlags.Never)) {
return reducedType;
}
Expand Down
8 changes: 8 additions & 0 deletions tests/baselines/reference/assignmentTypeNarrowing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ let a: string[];
for (x of a) {
x; // string
}

// Repro from #26405

type AOrArrA<T> = T | T[];
const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
arr.push({ x: "ok" });


//// [assignmentTypeNarrowing.js]
Expand All @@ -51,3 +57,5 @@ for (var _i = 0, a_1 = a; _i < a_1.length; _i++) {
x = a_1[_i];
x; // string
}
var arr = [{ x: "ok" }]; // weak type
arr.push({ x: "ok" });
20 changes: 20 additions & 0 deletions tests/baselines/reference/assignmentTypeNarrowing.symbols
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,23 @@ for (x of a) {
>x : Symbol(x, Decl(assignmentTypeNarrowing.ts, 0, 3))
}

// Repro from #26405

type AOrArrA<T> = T | T[];
>AOrArrA : Symbol(AOrArrA, Decl(assignmentTypeNarrowing.ts, 27, 1))
>T : Symbol(T, Decl(assignmentTypeNarrowing.ts, 31, 13))
>T : Symbol(T, Decl(assignmentTypeNarrowing.ts, 31, 13))
>T : Symbol(T, Decl(assignmentTypeNarrowing.ts, 31, 13))

const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
>arr : Symbol(arr, Decl(assignmentTypeNarrowing.ts, 32, 5))
>AOrArrA : Symbol(AOrArrA, Decl(assignmentTypeNarrowing.ts, 27, 1))
>x : Symbol(x, Decl(assignmentTypeNarrowing.ts, 32, 20))
>x : Symbol(x, Decl(assignmentTypeNarrowing.ts, 32, 35))

arr.push({ x: "ok" });
>arr.push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --))
>arr : Symbol(arr, Decl(assignmentTypeNarrowing.ts, 32, 5))
>push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --))
>x : Symbol(x, Decl(assignmentTypeNarrowing.ts, 33, 10))

22 changes: 22 additions & 0 deletions tests/baselines/reference/assignmentTypeNarrowing.types
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,25 @@ for (x of a) {
>x : string
}

// Repro from #26405

type AOrArrA<T> = T | T[];
>AOrArrA : AOrArrA<T>

const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
>arr : AOrArrA<{ x?: "ok"; }>
>x : "ok"
>[{ x: "ok" }] : { x: "ok"; }[]
>{ x: "ok" } : { x: "ok"; }
>x : "ok"
>"ok" : "ok"

arr.push({ x: "ok" });
>arr.push({ x: "ok" }) : number
>arr.push : (...items: { x?: "ok"; }[]) => number
>arr : { x?: "ok"; }[]
>push : (...items: { x?: "ok"; }[]) => number
>{ x: "ok" } : { x: "ok"; }
>x : "ok"
>"ok" : "ok"

6 changes: 6 additions & 0 deletions tests/cases/conformance/expressions/assignmentOperator/assignmentTypeNarrowing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,9 @@ let a: string[];
for (x of a) {
x; // string
}

// Repro from #26405

type AOrArrA<T> = T | T[];
const arr: AOrArrA<{x?: "ok"}> = [{ x: "ok" }]; // weak type
arr.push({ x: "ok" });

0 comments on commit c4b13fd

Please sign in to comment.