Skip to content

Commit

Permalink
Go back to the old narrowing algorithm (pre #26143) and avoid #26130 by
Browse files Browse the repository at this point in the history
skipping narrowing if the old algorithm produces a type to which the
assigned type is not assignable.

This also means we'll no longer narrow for erroneous assignments where
the assigned type is not assignable to the declared type.  This is the
reason for the numericLiteralTypes3 baseline change.

Fixes #26405.
  • Loading branch information
mattmccutchen committed Aug 18, 2018
1 parent 926bdee commit cc1c2ab
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 6 deletions.
20 changes: 18 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13900,6 +13900,18 @@ namespace ts {
return flow.id;
}

function typeMaybeAssignableTo(source: Type, target: Type) {
if (!(source.flags & TypeFlags.Union)) {
return isTypeAssignableTo(source, target);
}
for (const t of (<UnionType>source).types) {
if (isTypeAssignableTo(t, target)) {
return true;
}
}
return false;
}

// Remove those constituent types of declaredType to which no constituent type of assignedType is assignable.
// For example, when a variable of type number | string | boolean is assigned a value of type number | boolean,
// we remove type string.
Expand All @@ -13908,8 +13920,12 @@ namespace ts {
if (assignedType.flags & TypeFlags.Never) {
return assignedType;
}
const reducedType = filterType(declaredType, t => isTypeComparableTo(assignedType, t));
if (!(reducedType.flags & TypeFlags.Never)) {
const reducedType = filterType(declaredType, t => typeMaybeAssignableTo(assignedType, t));
// Our crude heuristic produces an invalid result in some cases: see GH#26130.
// For now, when that happens, we give up and don't narrow at all. (This also
// means we'll never narrow for erroneous assignments where the assigned type
// is not assignable to the declared type.)
if (isTypeAssignableTo(assignedType, reducedType)) {
return reducedType;
}
}
Expand Down
8 changes: 8 additions & 0 deletions tests/baselines/reference/assignmentTypeNarrowing.js
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
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
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"

4 changes: 2 additions & 2 deletions tests/baselines/reference/enumAssignmentCompat3.types
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,9 @@ abc = merged; // missing 'd'
>merged : Merged.E

merged = abc; // ok
>merged = abc : First.E.a | First.E.b
>merged = abc : First.E
>merged : Merged.E
>abc : First.E.a | First.E.b
>abc : First.E

abc = merged2; // ok
>abc = merged2 : Merged2.E
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/numericLiteralTypes3.types
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ function f4(a: A, b: B, c: C, d: D) {
>c : C

d = d;
>d = d : 1 | 2
>d = d : D
>d : D
>d : D
>d : 1 | 2
}

function f5(a: A, b: B, c: C, d: D) {
Expand Down
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 cc1c2ab

Please sign in to comment.