Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #24991: Weaken narrowing for == #29840

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 30 additions & 1 deletion src/compiler/checker.ts
Expand Up @@ -14168,6 +14168,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 @@ -16511,7 +16537,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;
}