From 181c10a78fa116e791602215b51dd527cb425ffb Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 2 Dec 2015 10:23:28 -0800 Subject: [PATCH 1/4] Ensure that different type parameters are never considered identical --- src/compiler/checker.ts | 35 +++++++---------------------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e737cd58bd560..523af078c0196 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5073,9 +5073,6 @@ namespace ts { } return objectTypeRelatedTo(source, source, target, /*reportErrors*/ false); } - if (source.flags & TypeFlags.TypeParameter && target.flags & TypeFlags.TypeParameter) { - return typeParameterIdenticalTo(source, target); - } if (source.flags & TypeFlags.Union && target.flags & TypeFlags.Union || source.flags & TypeFlags.Intersection && target.flags & TypeFlags.Intersection) { if (result = eachTypeRelatedToSomeType(source, target)) { @@ -5206,17 +5203,6 @@ namespace ts { return result; } - function typeParameterIdenticalTo(source: TypeParameter, target: TypeParameter): Ternary { - // covers case when both type parameters does not have constraint (both equal to noConstraintType) - if (source.constraint === target.constraint) { - return Ternary.True; - } - if (source.constraint === noConstraintType || target.constraint === noConstraintType) { - return Ternary.False; - } - return isIdenticalTo(source.constraint, target.constraint); - } - // Determine if two object types are related by structure. First, check if the result is already available in the global cache. // Second, check if we have already started a comparison of the given two types in which case we assume the result to be true. // Third, check if both types are part of deeply nested chains of generic type instantiations and if so assume the types are @@ -5765,26 +5751,19 @@ namespace ts { if (!(isMatchingSignature(source, target, partialMatch))) { return Ternary.False; } - let result = Ternary.True; - if (source.typeParameters && target.typeParameters) { - if (source.typeParameters.length !== target.typeParameters.length) { - return Ternary.False; - } - for (let i = 0, len = source.typeParameters.length; i < len; ++i) { - const related = compareTypes(source.typeParameters[i], target.typeParameters[i]); - if (!related) { - return Ternary.False; - } - result &= related; - } - } - else if (source.typeParameters || target.typeParameters) { + // Check that the two signatures have the same number of type parameters. We might consider + // also checking that any type parameter constraints match, but that would require instantiating + // the constraints with a common set of type arguments to get relatable entities in places where + // type parameters occur in the constraints. The complexity of doing that doesn't seem worthwhile, + // particularly as we're comparing erased versions of the signatures below. + if ((source.typeParameters ? source.typeParameters.length : 0) !== (target.typeParameters ? target.typeParameters.length : 0)) { return Ternary.False; } // Spec 1.0 Section 3.8.3 & 3.8.4: // M and N (the signatures) are instantiated using type Any as the type argument for all type parameters declared by M and N source = getErasedSignature(source); target = getErasedSignature(target); + let result = Ternary.True; const targetLen = target.parameters.length; for (let i = 0; i < targetLen; i++) { const s = isRestParameterIndex(source, i) ? getRestTypeOfSignature(source) : getTypeOfSymbol(source.parameters[i]); From 561360d550de590129fd899452c15d2e5d081d9d Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 2 Dec 2015 10:23:49 -0800 Subject: [PATCH 2/4] Adding regression test --- .../reference/unionTypeParameterInference.js | 14 ++++++++ .../unionTypeParameterInference.symbols | 33 ++++++++++++++++++ .../unionTypeParameterInference.types | 34 +++++++++++++++++++ .../compiler/unionTypeParameterInference.ts | 7 ++++ 4 files changed, 88 insertions(+) create mode 100644 tests/baselines/reference/unionTypeParameterInference.js create mode 100644 tests/baselines/reference/unionTypeParameterInference.symbols create mode 100644 tests/baselines/reference/unionTypeParameterInference.types create mode 100644 tests/cases/compiler/unionTypeParameterInference.ts diff --git a/tests/baselines/reference/unionTypeParameterInference.js b/tests/baselines/reference/unionTypeParameterInference.js new file mode 100644 index 0000000000000..cc797c511c889 --- /dev/null +++ b/tests/baselines/reference/unionTypeParameterInference.js @@ -0,0 +1,14 @@ +//// [unionTypeParameterInference.ts] +interface Foo { prop: T; } + +declare function lift(value: U | Foo): Foo; + +function unlift(value: U | Foo): U { + return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. +} + + +//// [unionTypeParameterInference.js] +function unlift(value) { + return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. +} diff --git a/tests/baselines/reference/unionTypeParameterInference.symbols b/tests/baselines/reference/unionTypeParameterInference.symbols new file mode 100644 index 0000000000000..ee540379284d5 --- /dev/null +++ b/tests/baselines/reference/unionTypeParameterInference.symbols @@ -0,0 +1,33 @@ +=== tests/cases/compiler/unionTypeParameterInference.ts === +interface Foo { prop: T; } +>Foo : Symbol(Foo, Decl(unionTypeParameterInference.ts, 0, 0)) +>T : Symbol(T, Decl(unionTypeParameterInference.ts, 0, 14)) +>prop : Symbol(prop, Decl(unionTypeParameterInference.ts, 0, 18)) +>T : Symbol(T, Decl(unionTypeParameterInference.ts, 0, 14)) + +declare function lift(value: U | Foo): Foo; +>lift : Symbol(lift, Decl(unionTypeParameterInference.ts, 0, 29)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 2, 22)) +>value : Symbol(value, Decl(unionTypeParameterInference.ts, 2, 25)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 2, 22)) +>Foo : Symbol(Foo, Decl(unionTypeParameterInference.ts, 0, 0)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 2, 22)) +>Foo : Symbol(Foo, Decl(unionTypeParameterInference.ts, 0, 0)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 2, 22)) + +function unlift(value: U | Foo): U { +>unlift : Symbol(unlift, Decl(unionTypeParameterInference.ts, 2, 52)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 16)) +>value : Symbol(value, Decl(unionTypeParameterInference.ts, 4, 19)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 16)) +>Foo : Symbol(Foo, Decl(unionTypeParameterInference.ts, 0, 0)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 16)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 16)) + + return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. +>lift(value).prop : Symbol(Foo.prop, Decl(unionTypeParameterInference.ts, 0, 18)) +>lift : Symbol(lift, Decl(unionTypeParameterInference.ts, 0, 29)) +>value : Symbol(value, Decl(unionTypeParameterInference.ts, 4, 19)) +>prop : Symbol(Foo.prop, Decl(unionTypeParameterInference.ts, 0, 18)) +} + diff --git a/tests/baselines/reference/unionTypeParameterInference.types b/tests/baselines/reference/unionTypeParameterInference.types new file mode 100644 index 0000000000000..a4b6a09cf2c7f --- /dev/null +++ b/tests/baselines/reference/unionTypeParameterInference.types @@ -0,0 +1,34 @@ +=== tests/cases/compiler/unionTypeParameterInference.ts === +interface Foo { prop: T; } +>Foo : Foo +>T : T +>prop : T +>T : T + +declare function lift(value: U | Foo): Foo; +>lift : (value: U | Foo) => Foo +>U : U +>value : U | Foo +>U : U +>Foo : Foo +>U : U +>Foo : Foo +>U : U + +function unlift(value: U | Foo): U { +>unlift : (value: U | Foo) => U +>U : U +>value : U | Foo +>U : U +>Foo : Foo +>U : U +>U : U + + return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. +>lift(value).prop : U +>lift(value) : Foo +>lift : (value: U | Foo) => Foo +>value : U | Foo +>prop : U +} + diff --git a/tests/cases/compiler/unionTypeParameterInference.ts b/tests/cases/compiler/unionTypeParameterInference.ts new file mode 100644 index 0000000000000..221b0891b9865 --- /dev/null +++ b/tests/cases/compiler/unionTypeParameterInference.ts @@ -0,0 +1,7 @@ +interface Foo { prop: T; } + +declare function lift(value: U | Foo): Foo; + +function unlift(value: U | Foo): U { + return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. +} From 3e6d40f3fee5a47fe3b1aa39ae3322db1d3e63ed Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 2 Dec 2015 15:41:37 -0800 Subject: [PATCH 3/4] Removing comment from test --- .../reference/unionTypeParameterInference.js | 7 +++- .../unionTypeParameterInference.symbols | 42 ++++++++++--------- .../unionTypeParameterInference.types | 4 +- .../compiler/unionTypeParameterInference.ts | 4 +- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/tests/baselines/reference/unionTypeParameterInference.js b/tests/baselines/reference/unionTypeParameterInference.js index cc797c511c889..c96e809cd4dc8 100644 --- a/tests/baselines/reference/unionTypeParameterInference.js +++ b/tests/baselines/reference/unionTypeParameterInference.js @@ -1,14 +1,17 @@ //// [unionTypeParameterInference.ts] +// Regression test for #5861 + interface Foo { prop: T; } declare function lift(value: U | Foo): Foo; function unlift(value: U | Foo): U { - return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. + return lift(value).prop; } //// [unionTypeParameterInference.js] +// Regression test for #5861 function unlift(value) { - return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. + return lift(value).prop; } diff --git a/tests/baselines/reference/unionTypeParameterInference.symbols b/tests/baselines/reference/unionTypeParameterInference.symbols index ee540379284d5..f2dbaac31ffb5 100644 --- a/tests/baselines/reference/unionTypeParameterInference.symbols +++ b/tests/baselines/reference/unionTypeParameterInference.symbols @@ -1,33 +1,35 @@ === tests/cases/compiler/unionTypeParameterInference.ts === +// Regression test for #5861 + interface Foo { prop: T; } >Foo : Symbol(Foo, Decl(unionTypeParameterInference.ts, 0, 0)) ->T : Symbol(T, Decl(unionTypeParameterInference.ts, 0, 14)) ->prop : Symbol(prop, Decl(unionTypeParameterInference.ts, 0, 18)) ->T : Symbol(T, Decl(unionTypeParameterInference.ts, 0, 14)) +>T : Symbol(T, Decl(unionTypeParameterInference.ts, 2, 14)) +>prop : Symbol(prop, Decl(unionTypeParameterInference.ts, 2, 18)) +>T : Symbol(T, Decl(unionTypeParameterInference.ts, 2, 14)) declare function lift(value: U | Foo): Foo; ->lift : Symbol(lift, Decl(unionTypeParameterInference.ts, 0, 29)) ->U : Symbol(U, Decl(unionTypeParameterInference.ts, 2, 22)) ->value : Symbol(value, Decl(unionTypeParameterInference.ts, 2, 25)) ->U : Symbol(U, Decl(unionTypeParameterInference.ts, 2, 22)) +>lift : Symbol(lift, Decl(unionTypeParameterInference.ts, 2, 29)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 22)) +>value : Symbol(value, Decl(unionTypeParameterInference.ts, 4, 25)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 22)) >Foo : Symbol(Foo, Decl(unionTypeParameterInference.ts, 0, 0)) ->U : Symbol(U, Decl(unionTypeParameterInference.ts, 2, 22)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 22)) >Foo : Symbol(Foo, Decl(unionTypeParameterInference.ts, 0, 0)) ->U : Symbol(U, Decl(unionTypeParameterInference.ts, 2, 22)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 22)) function unlift(value: U | Foo): U { ->unlift : Symbol(unlift, Decl(unionTypeParameterInference.ts, 2, 52)) ->U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 16)) ->value : Symbol(value, Decl(unionTypeParameterInference.ts, 4, 19)) ->U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 16)) +>unlift : Symbol(unlift, Decl(unionTypeParameterInference.ts, 4, 52)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 6, 16)) +>value : Symbol(value, Decl(unionTypeParameterInference.ts, 6, 19)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 6, 16)) >Foo : Symbol(Foo, Decl(unionTypeParameterInference.ts, 0, 0)) ->U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 16)) ->U : Symbol(U, Decl(unionTypeParameterInference.ts, 4, 16)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 6, 16)) +>U : Symbol(U, Decl(unionTypeParameterInference.ts, 6, 16)) - return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. ->lift(value).prop : Symbol(Foo.prop, Decl(unionTypeParameterInference.ts, 0, 18)) ->lift : Symbol(lift, Decl(unionTypeParameterInference.ts, 0, 29)) ->value : Symbol(value, Decl(unionTypeParameterInference.ts, 4, 19)) ->prop : Symbol(Foo.prop, Decl(unionTypeParameterInference.ts, 0, 18)) + return lift(value).prop; +>lift(value).prop : Symbol(Foo.prop, Decl(unionTypeParameterInference.ts, 2, 18)) +>lift : Symbol(lift, Decl(unionTypeParameterInference.ts, 2, 29)) +>value : Symbol(value, Decl(unionTypeParameterInference.ts, 6, 19)) +>prop : Symbol(Foo.prop, Decl(unionTypeParameterInference.ts, 2, 18)) } diff --git a/tests/baselines/reference/unionTypeParameterInference.types b/tests/baselines/reference/unionTypeParameterInference.types index a4b6a09cf2c7f..54eaed90ecc42 100644 --- a/tests/baselines/reference/unionTypeParameterInference.types +++ b/tests/baselines/reference/unionTypeParameterInference.types @@ -1,4 +1,6 @@ === tests/cases/compiler/unionTypeParameterInference.ts === +// Regression test for #5861 + interface Foo { prop: T; } >Foo : Foo >T : T @@ -24,7 +26,7 @@ function unlift(value: U | Foo): U { >U : U >U : U - return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. + return lift(value).prop; >lift(value).prop : U >lift(value) : Foo >lift : (value: U | Foo) => Foo diff --git a/tests/cases/compiler/unionTypeParameterInference.ts b/tests/cases/compiler/unionTypeParameterInference.ts index 221b0891b9865..79c4f3cc0e52a 100644 --- a/tests/cases/compiler/unionTypeParameterInference.ts +++ b/tests/cases/compiler/unionTypeParameterInference.ts @@ -1,7 +1,9 @@ +// Regression test for #5861 + interface Foo { prop: T; } declare function lift(value: U | Foo): Foo; function unlift(value: U | Foo): U { - return lift(value).prop; // error TS2322: Type '{}' is not assignable to type 'U'. + return lift(value).prop; } From 86d4a4c11f2abfd7d20fb3cd69e2e41051c5479b Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 2 Dec 2015 15:42:25 -0800 Subject: [PATCH 4/4] Adding test to demonstrate limits of signature identity checking --- .../reference/genericSignatureIdentity.js | 32 ++++++++++++ .../genericSignatureIdentity.symbols | 49 +++++++++++++++++++ .../reference/genericSignatureIdentity.types | 49 +++++++++++++++++++ .../compiler/genericSignatureIdentity.ts | 20 ++++++++ 4 files changed, 150 insertions(+) create mode 100644 tests/baselines/reference/genericSignatureIdentity.js create mode 100644 tests/baselines/reference/genericSignatureIdentity.symbols create mode 100644 tests/baselines/reference/genericSignatureIdentity.types create mode 100644 tests/cases/compiler/genericSignatureIdentity.ts diff --git a/tests/baselines/reference/genericSignatureIdentity.js b/tests/baselines/reference/genericSignatureIdentity.js new file mode 100644 index 0000000000000..9a51a61edbd53 --- /dev/null +++ b/tests/baselines/reference/genericSignatureIdentity.js @@ -0,0 +1,32 @@ +//// [genericSignatureIdentity.ts] +// This test is here to remind us of our current limits of type identity checking. +// Ideally all of the below declarations would be considered different (and thus errors) +// but they aren't because we erase type parameters to type any and don't check that +// constraints are identical. + +var x: { + (x: T): T; +}; + +var x: { + (x: T): T; +}; + +var x: { + (x: T): T; +}; + +var x: { + (x: any): any; +}; + + +//// [genericSignatureIdentity.js] +// This test is here to remind us of our current limits of type identity checking. +// Ideally all of the below declarations would be considered different (and thus errors) +// but they aren't because we erase type parameters to type any and don't check that +// constraints are identical. +var x; +var x; +var x; +var x; diff --git a/tests/baselines/reference/genericSignatureIdentity.symbols b/tests/baselines/reference/genericSignatureIdentity.symbols new file mode 100644 index 0000000000000..afd12ec266a18 --- /dev/null +++ b/tests/baselines/reference/genericSignatureIdentity.symbols @@ -0,0 +1,49 @@ +=== tests/cases/compiler/genericSignatureIdentity.ts === +// This test is here to remind us of our current limits of type identity checking. +// Ideally all of the below declarations would be considered different (and thus errors) +// but they aren't because we erase type parameters to type any and don't check that +// constraints are identical. + +var x: { +>x : Symbol(x, Decl(genericSignatureIdentity.ts, 5, 3), Decl(genericSignatureIdentity.ts, 9, 3), Decl(genericSignatureIdentity.ts, 13, 3), Decl(genericSignatureIdentity.ts, 17, 3)) + + (x: T): T; +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 6, 5)) +>Date : Symbol(Date, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --)) +>x : Symbol(x, Decl(genericSignatureIdentity.ts, 6, 21)) +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 6, 5)) +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 6, 5)) + +}; + +var x: { +>x : Symbol(x, Decl(genericSignatureIdentity.ts, 5, 3), Decl(genericSignatureIdentity.ts, 9, 3), Decl(genericSignatureIdentity.ts, 13, 3), Decl(genericSignatureIdentity.ts, 17, 3)) + + (x: T): T; +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 10, 5)) +>x : Symbol(x, Decl(genericSignatureIdentity.ts, 10, 23)) +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 10, 5)) +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 10, 5)) + +}; + +var x: { +>x : Symbol(x, Decl(genericSignatureIdentity.ts, 5, 3), Decl(genericSignatureIdentity.ts, 9, 3), Decl(genericSignatureIdentity.ts, 13, 3), Decl(genericSignatureIdentity.ts, 17, 3)) + + (x: T): T; +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 14, 5)) +>x : Symbol(x, Decl(genericSignatureIdentity.ts, 14, 8)) +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 14, 5)) +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 14, 5)) + +}; + +var x: { +>x : Symbol(x, Decl(genericSignatureIdentity.ts, 5, 3), Decl(genericSignatureIdentity.ts, 9, 3), Decl(genericSignatureIdentity.ts, 13, 3), Decl(genericSignatureIdentity.ts, 17, 3)) + + (x: any): any; +>T : Symbol(T, Decl(genericSignatureIdentity.ts, 18, 5)) +>x : Symbol(x, Decl(genericSignatureIdentity.ts, 18, 8)) + +}; + diff --git a/tests/baselines/reference/genericSignatureIdentity.types b/tests/baselines/reference/genericSignatureIdentity.types new file mode 100644 index 0000000000000..9385718a36127 --- /dev/null +++ b/tests/baselines/reference/genericSignatureIdentity.types @@ -0,0 +1,49 @@ +=== tests/cases/compiler/genericSignatureIdentity.ts === +// This test is here to remind us of our current limits of type identity checking. +// Ideally all of the below declarations would be considered different (and thus errors) +// but they aren't because we erase type parameters to type any and don't check that +// constraints are identical. + +var x: { +>x : (x: T) => T + + (x: T): T; +>T : T +>Date : Date +>x : T +>T : T +>T : T + +}; + +var x: { +>x : (x: T) => T + + (x: T): T; +>T : T +>x : T +>T : T +>T : T + +}; + +var x: { +>x : (x: T) => T + + (x: T): T; +>T : T +>x : T +>T : T +>T : T + +}; + +var x: { +>x : (x: T) => T + + (x: any): any; +>T : T +>x : any + +}; + diff --git a/tests/cases/compiler/genericSignatureIdentity.ts b/tests/cases/compiler/genericSignatureIdentity.ts new file mode 100644 index 0000000000000..c685b8cc57363 --- /dev/null +++ b/tests/cases/compiler/genericSignatureIdentity.ts @@ -0,0 +1,20 @@ +// This test is here to remind us of our current limits of type identity checking. +// Ideally all of the below declarations would be considered different (and thus errors) +// but they aren't because we erase type parameters to type any and don't check that +// constraints are identical. + +var x: { + (x: T): T; +}; + +var x: { + (x: T): T; +}; + +var x: { + (x: T): T; +}; + +var x: { + (x: any): any; +};