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.

Sign up for GitHub

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

Jump to bottom

WIP: fix narrow type of relax compare #25174

Closed
wants to merge 1 commit into from
Closed

WIP: fix narrow type of relax compare #25174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 29 additions & 11 deletions src/compiler/checker.ts
View file
Open in desktop
Expand Up @@ -595,6 +595,7 @@ namespace ts {
const assignableRelation = createMap<RelationComparisonResult>();
const definitelyAssignableRelation = createMap<RelationComparisonResult>();
const comparableRelation = createMap<RelationComparisonResult>();
const relaxComparableRelation = createMap<RelationComparisonResult>();
const identityRelation = createMap<RelationComparisonResult>();
const enumRelation = createMap<boolean>();

Expand Down Expand Up @@ -10109,6 +10110,14 @@ namespace ts {
return checkTypeRelatedTo(source, target, assignableRelation, errorNode, headMessage, containingMessageChain);
}

function isTypeRelaxComparableTo(source: Type, target: Type): boolean {
return isTypeRelatedTo(source, target, relaxComparableRelation);
}

function areTypesRelaxComparable(type1: Type, type2: Type): boolean {
return isTypeRelaxComparableTo(type1, type2) || isTypeRelaxComparableTo(type2, type1);
}

/**
* This is *not* a bi-directional relationship.
* If one needs to check both directions for comparability, use a second call to this function or 'isTypeComparableTo'.
Expand Down Expand Up @@ -10371,6 +10380,14 @@ namespace ts {
const t = target.flags;
if (t & TypeFlags.AnyOrUnknown || s & TypeFlags.Never || source === wildcardType) return true;
if (t & TypeFlags.Never) return false;
if (relation === relaxComparableRelation) {
if (s & TypeFlags.NumberLike && t & TypeFlags.StringLike) return true;
if (s & TypeFlags.NumberLike && t & TypeFlags.BooleanLike) return true;
// if (s & TypeFlags.NumberLike && t & TypeFlags.Object) return true;
if (s & TypeFlags.StringLike && t & TypeFlags.BooleanLike) return true;
// if (s & TypeFlags.StringLike && t & TypeFlags.Object) return true;
// if (s & TypeFlags.BooleanLike && t & TypeFlags.Object) return true;
}
if (s & TypeFlags.StringLike && t & TypeFlags.String) return true;
if (s & TypeFlags.StringLiteral && s & TypeFlags.EnumLiteral &&
t & TypeFlags.StringLiteral && !(t & TypeFlags.EnumLiteral) &&
Expand All @@ -10392,7 +10409,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 === relaxComparableRelation)) {
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 @@ -10411,7 +10428,7 @@ namespace ts {
target = (<LiteralType>target).regularType;
}
if (source === target ||
relation === comparableRelation && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
(relation === comparableRelation || relation === relaxComparableRelation) && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
relation !== identityRelation && isSimpleTypeRelatedTo(source, target, relation)) {
return true;
}
Expand Down Expand Up @@ -10505,7 +10522,7 @@ namespace ts {
}

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

if (relation === comparableRelation && !(target.flags & TypeFlags.Never) && isSimpleTypeRelatedTo(target, source, relation) ||
if ((relation === comparableRelation || relation === relaxComparableRelation) && !(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 All @@ -10603,7 +10620,7 @@ namespace ts {
}
}

if (relation !== comparableRelation &&
if ((relation !== comparableRelation || relation === relaxComparableRelation) &&
!(source.flags & TypeFlags.UnionOrIntersection) &&
!(target.flags & TypeFlags.Union) &&
!isIntersectionConstituent &&
Expand Down Expand Up @@ -10634,7 +10651,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 === relaxComparableRelation) ?
someTypeRelatedToType(source as UnionType, target, reportErrors && !(source.flags & TypeFlags.Primitive)) :
eachTypeRelatedToType(source as UnionType, target, reportErrors && !(source.flags & TypeFlags.Primitive));
}
Expand Down Expand Up @@ -10755,7 +10772,7 @@ namespace ts {
function hasExcessProperties(source: FreshObjectLiteralType, target: Type, discriminant: Type | undefined, reportErrors: boolean): boolean {
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 === relaxComparableRelation)) &&
(isTypeSubsetOf(globalObjectType, target) || (!isComparingJsxAttributes && isEmptyObjectType(target)))) {
return false;
}
Expand Down Expand Up @@ -11268,7 +11285,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 === relaxComparableRelation) || (relation === identityRelation ? getMappedTypeModifiers(source) === getMappedTypeModifiers(target) :
getCombinedMappedTypeOptionality(source) <= getCombinedMappedTypeOptionality(target));
if (modifiersRelated) {
let result: Ternary;
Expand Down Expand Up @@ -11363,7 +11380,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 !== relaxComparableRelation) && 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 @@ -11483,7 +11500,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 === relaxComparableRelation) || !!compilerOptions.noStrictGenericChecks;
result = signatureRelatedTo(sourceSignatures[0], targetSignatures[0], eraseGenerics, reportErrors);
}
else {
Expand Down Expand Up @@ -14157,7 +14174,8 @@ namespace ts {
return type;
}
if (assumeTrue) {
const narrowedType = filterType(type, t => areTypesComparable(t, valueType));
const strictEquals = operator === SyntaxKind.EqualsEqualsEqualsToken || operator === SyntaxKind.ExclamationEqualsEqualsToken;
const narrowedType = filterType(type, t => strictEquals ? areTypesComparable(t, valueType) : areTypesRelaxComparable(t, valueType));
return narrowedType.flags & TypeFlags.Never ? type : replacePrimitivesWithLiterals(narrowedType, valueType);
}
if (isUnitType(valueType)) {
Expand Down
59 changes: 59 additions & 0 deletions tests/baselines/reference/typeGuardWithRelaxEquality.errors.txt
View file
Open in desktop
@@ -0,0 +1,59 @@
tests/cases/conformance/expressions/typeGuards/typeGuardWithRelaxEquality.ts(5,9): error TS2322: Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.


==== tests/cases/conformance/expressions/typeGuards/typeGuardWithRelaxEquality.ts (1 errors) ====
// Github issue #24991
function test(level: number | string):number {
if (level == +level) {
const q2 = level; // number | string
return level;
~~~~~~~~~~~~~
!!! error TS2322: Type 'string | number' is not assignable to type 'number'.
!!! error TS2322: Type 'string' is not assignable to type 'number'.
}
if (level === +level + 1) {
const q2 = level;
return level;
}
return 0;
}
alert(test(5) + 1);
alert(test("5") + 1)

declare const a: string | number | boolean | object | symbol | null | undefined;
declare const s: symbol;
declare const str: string;
declare const num: number;
declare const bool: boolean;

if (a == 1) {
const t = a
}
if (a == num) {
const t = a
}
if (a == '') {
const t = a
}
if (a == str) {
const t = a
}
if (a == false) {
const t = a
}
if (a == bool) {
const t = a
}
if (a == {}) {
const t = a
}
if (a == s) {
const t = a
}
if (a == null) {
const t = a
}
if (a == undefined) {
const t = a
}
99 changes: 99 additions & 0 deletions tests/baselines/reference/typeGuardWithRelaxEquality.js
View file
Open in desktop
@@ -0,0 +1,99 @@
//// [typeGuardWithRelaxEquality.ts]
// Github issue #24991
function test(level: number | string):number {
if (level == +level) {
const q2 = level; // number | string
return level;
}
if (level === +level + 1) {
const q2 = level;
return level;
}
return 0;
}
alert(test(5) + 1);
alert(test("5") + 1)

declare const a: string | number | boolean | object | symbol | null | undefined;
declare const s: symbol;
declare const str: string;
declare const num: number;
declare const bool: boolean;

if (a == 1) {
const t = a
}
if (a == num) {
const t = a
}
if (a == '') {
const t = a
}
if (a == str) {
const t = a
}
if (a == false) {
const t = a
}
if (a == bool) {
const t = a
}
if (a == {}) {
const t = a
}
if (a == s) {
const t = a
}
if (a == null) {
const t = a
}
if (a == undefined) {
const t = a
}

//// [typeGuardWithRelaxEquality.js]
"use strict";
// Github issue #24991
function test(level) {
if (level == +level) {
var q2 = level; // number | string
return level;
}
if (level === +level + 1) {
var q2 = level;
return level;
}
return 0;
}
alert(test(5) + 1);
alert(test("5") + 1);
if (a == 1) {
var t = a;
}
if (a == num) {
var t = a;
}
if (a == '') {
var t = a;
}
if (a == str) {
var t = a;
}
if (a == false) {
var t = a;
}
if (a == bool) {
var t = a;
}
if (a == {}) {
var t = a;
}
if (a == s) {
var t = a;
}
if (a == null) {
var t = a;
}
if (a == undefined) {
var t = a;
}