Skip to content

Commit

Permalink
Fix #24991: Weaken narrowing for == (#29840)
Browse files Browse the repository at this point in the history
Spelling
  • Loading branch information
jack-williams authored and RyanCavanaugh committed Apr 26, 2019
1 parent 95413f0 commit 0949ad1
Show file tree
Hide file tree
Showing 6 changed files with 506 additions and 1 deletion.
31 changes: 30 additions & 1 deletion src/compiler/checker.ts
Expand Up @@ -14227,6 +14227,32 @@ namespace ts {
return strictNullChecks ? getGlobalNonNullableTypeInstantiation(type) : type;
}


/**
* Is source potentially coercible to target type under `==`.
* Assumes that `source` is a constituent of a union, hence
* the boolean literal flag on the LHS, but not on the RHS.
*
* This does not fully replicate the semantics of `==`. The
* intention is to catch cases that are clearly not right.
*
* Comparing (string | number) to number should not remove the
* string element.
*
* Comparing (string | number) to 1 will remove the string
* element, though this is not sound. This is a pragmatic
* choice.
*
* @see narrowTypeByEquality
*
* @param source
* @param target
*/
function isCoercibleUnderDoubleEquals(source: Type, target: Type): boolean {
return ((source.flags & (TypeFlags.Number | TypeFlags.String | TypeFlags.BooleanLiteral)) !== 0)
&& ((target.flags & (TypeFlags.Number | TypeFlags.String | TypeFlags.Boolean)) !== 0);
}

/**
* Return true if type was inferred from an object literal, written as an object type literal, or is the shape of a module
* with no call or construct signatures.
Expand Down Expand Up @@ -16570,7 +16596,10 @@ namespace ts {
return type;
}
if (assumeTrue) {
const narrowedType = filterType(type, t => areTypesComparable(t, valueType));
const filterFn: (t: Type) => boolean = operator === SyntaxKind.EqualsEqualsToken ?
(t => areTypesComparable(t, valueType) || isCoercibleUnderDoubleEquals(t, valueType)) :
t => areTypesComparable(t, valueType);
const narrowedType = filterType(type, filterFn);
return narrowedType.flags & TypeFlags.Never ? type : replacePrimitivesWithLiterals(narrowedType, valueType);
}
if (isUnitType(valueType)) {
Expand Down
71 changes: 71 additions & 0 deletions tests/baselines/reference/narrowByEquality.errors.txt
@@ -0,0 +1,71 @@
tests/cases/compiler/narrowByEquality.ts(53,15): error TS2322: Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/compiler/narrowByEquality.ts(54,9): error TS2322: Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.


==== tests/cases/compiler/narrowByEquality.ts (2 errors) ====
declare let x: number | string | boolean
declare let n: number;
declare let s: string;
declare let b: boolean;

if (x == n) {
x;
}

if (x == s) {
x;
}

if (x == b) {
x;
}

if (x == 1) {
x;
}

if (x == "") {
x;
}

if (x == "foo") {
x;
}

if (x == true) {
x;
}

if (x == false) {
x;
}

declare let xAndObj: number | string | boolean | object

if (xAndObj == {}) {
xAndObj;
}

if (x == xAndObj) {
x;
xAndObj;
}

// Repro from #24991

function test(level: number | string):number {
if (level == +level) {
const q2: number = level; // error
~~
!!! error TS2322: Type 'string | number' is not assignable to type 'number'.
!!! error TS2322: Type 'string' is not assignable to type 'number'.
return level;
~~~~~~~~~~~~~
!!! error TS2322: Type 'string | number' is not assignable to type 'number'.
!!! error TS2322: Type 'string' is not assignable to type 'number'.
}
return 0;
}

101 changes: 101 additions & 0 deletions tests/baselines/reference/narrowByEquality.js
@@ -0,0 +1,101 @@
//// [narrowByEquality.ts]
declare let x: number | string | boolean
declare let n: number;
declare let s: string;
declare let b: boolean;

if (x == n) {
x;
}

if (x == s) {
x;
}

if (x == b) {
x;
}

if (x == 1) {
x;
}

if (x == "") {
x;
}

if (x == "foo") {
x;
}

if (x == true) {
x;
}

if (x == false) {
x;
}

declare let xAndObj: number | string | boolean | object

if (xAndObj == {}) {
xAndObj;
}

if (x == xAndObj) {
x;
xAndObj;
}

// Repro from #24991

function test(level: number | string):number {
if (level == +level) {
const q2: number = level; // error
return level;
}
return 0;
}


//// [narrowByEquality.js]
"use strict";
if (x == n) {
x;
}
if (x == s) {
x;
}
if (x == b) {
x;
}
if (x == 1) {
x;
}
if (x == "") {
x;
}
if (x == "foo") {
x;
}
if (x == true) {
x;
}
if (x == false) {
x;
}
if (xAndObj == {}) {
xAndObj;
}
if (x == xAndObj) {
x;
xAndObj;
}
// Repro from #24991
function test(level) {
if (level == +level) {
var q2 = level; // error
return level;
}
return 0;
}
113 changes: 113 additions & 0 deletions tests/baselines/reference/narrowByEquality.symbols
@@ -0,0 +1,113 @@
=== tests/cases/compiler/narrowByEquality.ts ===
declare let x: number | string | boolean
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))

declare let n: number;
>n : Symbol(n, Decl(narrowByEquality.ts, 1, 11))

declare let s: string;
>s : Symbol(s, Decl(narrowByEquality.ts, 2, 11))

declare let b: boolean;
>b : Symbol(b, Decl(narrowByEquality.ts, 3, 11))

if (x == n) {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
>n : Symbol(n, Decl(narrowByEquality.ts, 1, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
}

if (x == s) {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
>s : Symbol(s, Decl(narrowByEquality.ts, 2, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
}

if (x == b) {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
>b : Symbol(b, Decl(narrowByEquality.ts, 3, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
}

if (x == 1) {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
}

if (x == "") {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
}

if (x == "foo") {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
}

if (x == true) {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
}

if (x == false) {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
}

declare let xAndObj: number | string | boolean | object
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))

if (xAndObj == {}) {
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))

xAndObj;
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))
}

if (x == xAndObj) {
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))

x;
>x : Symbol(x, Decl(narrowByEquality.ts, 0, 11))

xAndObj;
>xAndObj : Symbol(xAndObj, Decl(narrowByEquality.ts, 37, 11))
}

// Repro from #24991

function test(level: number | string):number {
>test : Symbol(test, Decl(narrowByEquality.ts, 46, 1))
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))

if (level == +level) {
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))

const q2: number = level; // error
>q2 : Symbol(q2, Decl(narrowByEquality.ts, 52, 13))
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))

return level;
>level : Symbol(level, Decl(narrowByEquality.ts, 50, 14))
}
return 0;
}

0 comments on commit 0949ad1

Please sign in to comment.