Skip to content

Fix type parameter comparability to consistently allow comparisons on unconstrained type parameters #48861

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

Merged
merged 3 commits into from
May 9, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19347,6 +19347,20 @@ namespace ts {
}
}
}
if (relation === comparableRelation && sourceFlags & TypeFlags.TypeParameter) {
// This is a carve-out in comparability to essentially forbid comparing a type parameter
// with another type parameter unless one extends the other. (Remember: comparability is mostly bidirectional!)
let constraint = getConstraintOfTypeParameter(source);
if (constraint && hasNonCircularBaseConstraint(source)) {
while (constraint && constraint.flags & TypeFlags.TypeParameter) {
if (result = isRelatedTo(constraint, target, RecursionFlags.Source, /*reportErrors*/ false)) {
return result;
}
constraint = getConstraintOfTypeParameter(constraint);
}
}
return Ternary.False;
}
}
else if (targetFlags & TypeFlags.Index) {
const targetType = (target as IndexType).type;
Expand Down Expand Up @@ -19558,21 +19572,21 @@ namespace ts {
if (sourceFlags & TypeFlags.TypeVariable) {
// IndexedAccess comparisons are handled above in the `targetFlags & TypeFlage.IndexedAccess` branch
if (!(sourceFlags & TypeFlags.IndexedAccess && targetFlags & TypeFlags.IndexedAccess)) {
const constraint = getConstraintOfType(source as TypeVariable);
if (!constraint || (sourceFlags & TypeFlags.TypeParameter && constraint.flags & TypeFlags.Any)) {
const constraint = getConstraintOfType(source as TypeVariable) || unknownType;
if (!getConstraintOfType(source as TypeVariable) || (sourceFlags & TypeFlags.TypeParameter && constraint.flags & TypeFlags.Any)) {
// A type variable with no constraint is not related to the non-primitive object type.
if (result = isRelatedTo(emptyObjectType, extractTypesOfKind(target, ~TypeFlags.NonPrimitive), RecursionFlags.Both)) {
resetErrorInfo(saveErrorInfo);
return result;
}
}
// hi-speed no-this-instantiation check (less accurate, but avoids costly `this`-instantiation when the constraint will suffice), see #28231 for report on why this is needed
else if (result = isRelatedTo(constraint, target, RecursionFlags.Source, /*reportErrors*/ false, /*headMessage*/ undefined, intersectionState)) {
if (result = isRelatedTo(constraint, target, RecursionFlags.Source, /*reportErrors*/ false, /*headMessage*/ undefined, intersectionState)) {
resetErrorInfo(saveErrorInfo);
return result;
}
// slower, fuller, this-instantiated check (necessary when comparing raw `this` types from base classes), see `subclassWithPolymorphicThisIsAssignable.ts` test for example
else if (result = isRelatedTo(getTypeWithThisArgument(constraint, source), target, RecursionFlags.Source, reportErrors && !(targetFlags & sourceFlags & TypeFlags.TypeParameter), /*headMessage*/ undefined, intersectionState)) {
else if (result = isRelatedTo(getTypeWithThisArgument(constraint, source), target, RecursionFlags.Source, reportErrors && constraint !== unknownType && !(targetFlags & sourceFlags & TypeFlags.TypeParameter), /*headMessage*/ undefined, intersectionState)) {
resetErrorInfo(saveErrorInfo);
return result;
}
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
tests/cases/compiler/genericWithNoConstraintComparableWithCurlyCurly.ts(23,5): error TS2352: Conversion of type '{}' to type 'T' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
'T' could be instantiated with an arbitrary type which could be unrelated to '{}'.


==== tests/cases/compiler/genericWithNoConstraintComparableWithCurlyCurly.ts (1 errors) ====
function foo<T>() {
let x = {};
x as T;
}

function bar<T extends unknown>() {
let x = {};
x as T;
}

function baz<T extends {}>() {
let x = {};
x as T;
}

function bat<T extends object>() {
let x = {};
x as T;
}

function no<T extends null | undefined>() {
let x = {};
x as T; // should error
~~~~~~
!!! error TS2352: Conversion of type '{}' to type 'T' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
!!! error TS2352: 'T' could be instantiated with an arbitrary type which could be unrelated to '{}'.
}

function yes<T extends object | null | undefined>() {
let x = {};
x as T;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//// [genericWithNoConstraintComparableWithCurlyCurly.ts]
function foo<T>() {
let x = {};
x as T;
}

function bar<T extends unknown>() {
let x = {};
x as T;
}

function baz<T extends {}>() {
let x = {};
x as T;
}

function bat<T extends object>() {
let x = {};
x as T;
}

function no<T extends null | undefined>() {
let x = {};
x as T; // should error
}

function yes<T extends object | null | undefined>() {
let x = {};
x as T;
}

//// [genericWithNoConstraintComparableWithCurlyCurly.js]
"use strict";
function foo() {
var x = {};
x;
}
function bar() {
var x = {};
x;
}
function baz() {
var x = {};
x;
}
function bat() {
var x = {};
x;
}
function no() {
var x = {};
x; // should error
}
function yes() {
var x = {};
x;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
=== tests/cases/compiler/genericWithNoConstraintComparableWithCurlyCurly.ts ===
function foo<T>() {
>foo : Symbol(foo, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 0, 0))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 0, 13))

let x = {};
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 1, 7))

x as T;
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 1, 7))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 0, 13))
}

function bar<T extends unknown>() {
>bar : Symbol(bar, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 3, 1))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 5, 13))

let x = {};
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 6, 7))

x as T;
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 6, 7))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 5, 13))
}

function baz<T extends {}>() {
>baz : Symbol(baz, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 8, 1))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 10, 13))

let x = {};
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 11, 7))

x as T;
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 11, 7))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 10, 13))
}

function bat<T extends object>() {
>bat : Symbol(bat, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 13, 1))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 15, 13))

let x = {};
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 16, 7))

x as T;
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 16, 7))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 15, 13))
}

function no<T extends null | undefined>() {
>no : Symbol(no, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 18, 1))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 20, 12))

let x = {};
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 21, 7))

x as T; // should error
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 21, 7))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 20, 12))
}

function yes<T extends object | null | undefined>() {
>yes : Symbol(yes, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 23, 1))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 25, 13))

let x = {};
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 26, 7))

x as T;
>x : Symbol(x, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 26, 7))
>T : Symbol(T, Decl(genericWithNoConstraintComparableWithCurlyCurly.ts, 25, 13))
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
=== tests/cases/compiler/genericWithNoConstraintComparableWithCurlyCurly.ts ===
function foo<T>() {
>foo : <T>() => void

let x = {};
>x : {}
>{} : {}

x as T;
>x as T : T
>x : {}
}

function bar<T extends unknown>() {
>bar : <T extends unknown>() => void

let x = {};
>x : {}
>{} : {}

x as T;
>x as T : T
>x : {}
}

function baz<T extends {}>() {
>baz : <T extends {}>() => void

let x = {};
>x : {}
>{} : {}

x as T;
>x as T : T
>x : {}
}

function bat<T extends object>() {
>bat : <T extends object>() => void

let x = {};
>x : {}
>{} : {}

x as T;
>x as T : T
>x : {}
}

function no<T extends null | undefined>() {
>no : <T extends null | undefined>() => void
>null : null

let x = {};
>x : {}
>{} : {}

x as T; // should error
>x as T : T
>x : {}
}

function yes<T extends object | null | undefined>() {
>yes : <T extends object | null | undefined>() => void
>null : null

let x = {};
>x : {}
>{} : {}

x as T;
>x as T : T
>x : {}
}
4 changes: 2 additions & 2 deletions tests/baselines/reference/intersectionNarrowing.types
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ function f4<T>(x: T & 1 | T & 2) {

case 1: x; break; // T & 1
>1 : 1
>x : T & 1
>x : (T & 1) | (T & 2)

case 2: x; break; // T & 2
>2 : 2
>x : T & 2
>x : (T & 1) | (T & 2)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallout from permissive comparability: We use comparability in switch-case narrowing (in narrowTypeBySwitchOnDiscriminant) and since T is unconstrained and thus extends unknown, both the 1 and 2 intersections are considered comparable with both 1 and 2 because of the T part. Some extra work could possibly be done to prevent this by changing how comparability works over intersections, probably.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It's likely a similar change occurs in narrowing assignments, since we also use bidirectional comparability there, and maybe in other places more subtly since we use unidirectional comparability in narrowTypeByDiscriminant)


default: x; // Should narrow to never
>x : never
Expand Down
6 changes: 0 additions & 6 deletions tests/baselines/reference/unknownType1.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ tests/cases/conformance/types/unknown/unknownType1.ts(112,9): error TS2322: Type
tests/cases/conformance/types/unknown/unknownType1.ts(113,9): error TS2322: Type 'unknown' is not assignable to type '{}'.
tests/cases/conformance/types/unknown/unknownType1.ts(114,9): error TS2322: Type 'unknown' is not assignable to type '{} | null | undefined'.
tests/cases/conformance/types/unknown/unknownType1.ts(120,9): error TS2322: Type 'T' is not assignable to type 'object'.
Type 'unknown' is not assignable to type 'object'.
tests/cases/conformance/types/unknown/unknownType1.ts(128,5): error TS2322: Type 'number[]' is not assignable to type '{ [x: string]: unknown; }'.
Index signature for type 'string' is missing in type 'number[]'.
tests/cases/conformance/types/unknown/unknownType1.ts(129,5): error TS2322: Type 'number' is not assignable to type '{ [x: string]: unknown; }'.
Expand All @@ -26,9 +25,7 @@ tests/cases/conformance/types/unknown/unknownType1.ts(150,17): error TS2355: A f
tests/cases/conformance/types/unknown/unknownType1.ts(156,14): error TS2700: Rest types may only be created from object types.
tests/cases/conformance/types/unknown/unknownType1.ts(162,5): error TS2564: Property 'a' has no initializer and is not definitely assigned in the constructor.
tests/cases/conformance/types/unknown/unknownType1.ts(171,9): error TS2322: Type 'U' is not assignable to type '{}'.
Type 'unknown' is not assignable to type '{}'.
tests/cases/conformance/types/unknown/unknownType1.ts(181,5): error TS2322: Type 'T' is not assignable to type '{}'.
Type 'unknown' is not assignable to type '{}'.


==== tests/cases/conformance/types/unknown/unknownType1.ts (27 errors) ====
Expand Down Expand Up @@ -188,7 +185,6 @@ tests/cases/conformance/types/unknown/unknownType1.ts(181,5): error TS2322: Type
let y: object = x; // Error
~
!!! error TS2322: Type 'T' is not assignable to type 'object'.
!!! error TS2322: Type 'unknown' is not assignable to type 'object'.
}

// Anything fresh but primitive assignable to { [x: string]: unknown }
Expand Down Expand Up @@ -257,7 +253,6 @@ tests/cases/conformance/types/unknown/unknownType1.ts(181,5): error TS2322: Type
let y: {} = u;
~
!!! error TS2322: Type 'U' is not assignable to type '{}'.
!!! error TS2322: Type 'unknown' is not assignable to type '{}'.
}

// Repro from #26796
Expand All @@ -270,6 +265,5 @@ tests/cases/conformance/types/unknown/unknownType1.ts(181,5): error TS2322: Type
return arg; // Error
~~~~~~~~~~~
!!! error TS2322: Type 'T' is not assignable to type '{}'.
!!! error TS2322: Type 'unknown' is not assignable to type '{}'.
}

2 changes: 0 additions & 2 deletions tests/baselines/reference/variadicTuples2.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ tests/cases/conformance/types/tuple/variadicTuples2.ts(73,5): error TS2322: Type
tests/cases/conformance/types/tuple/variadicTuples2.ts(74,5): error TS2322: Type '[number, ...T]' is not assignable to type '[number, ...number[]]'.
Type at position 1 in source is not compatible with type at position 1 in target.
Type 'T[number]' is not assignable to type 'number'.
Type 'unknown' is not assignable to type 'number'.
tests/cases/conformance/types/tuple/variadicTuples2.ts(107,16): error TS2345: Argument of type '[1, 2, 3, 4]' is not assignable to parameter of type '[...number[], (...values: number[]) => void]'.
Type at position 3 in source is not compatible with type at position 1 in target.
Type 'number' is not assignable to type '(...values: number[]) => void'.
Expand Down Expand Up @@ -206,7 +205,6 @@ tests/cases/conformance/types/tuple/variadicTuples2.ts(130,25): error TS2345: Ar
!!! error TS2322: Type '[number, ...T]' is not assignable to type '[number, ...number[]]'.
!!! error TS2322: Type at position 1 in source is not compatible with type at position 1 in target.
!!! error TS2322: Type 'T[number]' is not assignable to type 'number'.
!!! error TS2322: Type 'unknown' is not assignable to type 'number'.
}

// Inference
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// @strict: true
function foo<T>() {
let x = {};
x as T;
}

function bar<T extends unknown>() {
let x = {};
x as T;
}

function baz<T extends {}>() {
let x = {};
x as T;
}

function bat<T extends object>() {
let x = {};
x as T;
}

function no<T extends null | undefined>() {
let x = {};
x as T; // should error
}

function yes<T extends object | null | undefined>() {
let x = {};
x as T;
}