From 762fff1f65d990aaf79202aaaa02e4a5d7ae7b83 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 23 Jun 2019 09:00:42 -1000 Subject: [PATCH 1/5] Remove duplicate call/construct signatures in intersections --- src/compiler/checker.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 9cb08b3e440a4..414d815d5d20e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7234,8 +7234,8 @@ namespace ts { function resolveIntersectionTypeMembers(type: IntersectionType) { // The members and properties collections are empty for intersection types. To get all properties of an // intersection type use getPropertiesOfType (only the language service uses this). - let callSignatures: ReadonlyArray = emptyArray; - let constructSignatures: ReadonlyArray = emptyArray; + let callSignatures: Signature[] | undefined; + let constructSignatures: Signature[] | undefined; let stringIndexInfo: IndexInfo | undefined; let numberIndexInfo: IndexInfo | undefined; const types = type.types; @@ -7257,13 +7257,22 @@ namespace ts { return clone; }); } - constructSignatures = concatenate(constructSignatures, signatures); + constructSignatures = appendSignatures(constructSignatures, signatures); } - callSignatures = concatenate(callSignatures, getSignaturesOfType(t, SignatureKind.Call)); + callSignatures = appendSignatures(callSignatures, getSignaturesOfType(t, SignatureKind.Call)); stringIndexInfo = intersectIndexInfos(stringIndexInfo, getIndexInfoOfType(t, IndexKind.String)); numberIndexInfo = intersectIndexInfos(numberIndexInfo, getIndexInfoOfType(t, IndexKind.Number)); } - setStructuredTypeMembers(type, emptySymbols, callSignatures, constructSignatures, stringIndexInfo, numberIndexInfo); + setStructuredTypeMembers(type, emptySymbols, callSignatures || emptyArray, constructSignatures || emptyArray, stringIndexInfo, numberIndexInfo); + } + + function appendSignatures(signatures: Signature[] | undefined, newSignatures: readonly Signature[]) { + for (const sig of newSignatures) { + if (!signatures || every(signatures, s => !compareSignaturesIdentical(s, sig, /*partialMatch*/ false, /*ignoreThisTypes*/ false, /*ignoreReturnTypes*/ false, compareTypesIdentical))) { + signatures = append(signatures, sig); + } + } + return signatures; } /** From 076d9ad2ab208ae57d59c6ad17654dcaac375b0d Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 23 Jun 2019 09:19:56 -1000 Subject: [PATCH 2/5] Add regression test --- .../conformance/types/keyof/keyofAndIndexedAccess2.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/cases/conformance/types/keyof/keyofAndIndexedAccess2.ts b/tests/cases/conformance/types/keyof/keyofAndIndexedAccess2.ts index b242aa3de60bc..a86f4718e35c7 100644 --- a/tests/cases/conformance/types/keyof/keyofAndIndexedAccess2.ts +++ b/tests/cases/conformance/types/keyof/keyofAndIndexedAccess2.ts @@ -158,3 +158,12 @@ type Bar = { [key: string]: { [K in keyof T]: [K] }[keyof T] }; type Baz> = { [K in keyof Q]: T[Q[K]] }; type Qux> = { [K in keyof Q]: T[Q[K]["0"]] }; + +// Repro from #32038 + +const actions = ['resizeTo', 'resizeBy'] as const; +for (const action of actions) { + window[action] = (x, y) => { + window[action](x, y); + } +} From be4147ddf33810f304fd97cc6e398bee12ba9350 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 23 Jun 2019 09:20:03 -1000 Subject: [PATCH 3/5] Accept new baselines --- .../keyofAndIndexedAccess2.errors.txt | 9 ++++++ .../reference/keyofAndIndexedAccess2.js | 16 ++++++++++ .../reference/keyofAndIndexedAccess2.symbols | 23 +++++++++++++ .../reference/keyofAndIndexedAccess2.types | 32 +++++++++++++++++++ 4 files changed, 80 insertions(+) diff --git a/tests/baselines/reference/keyofAndIndexedAccess2.errors.txt b/tests/baselines/reference/keyofAndIndexedAccess2.errors.txt index ff96efe3e13fc..b00409544decc 100644 --- a/tests/baselines/reference/keyofAndIndexedAccess2.errors.txt +++ b/tests/baselines/reference/keyofAndIndexedAccess2.errors.txt @@ -237,4 +237,13 @@ tests/cases/conformance/types/keyof/keyofAndIndexedAccess2.ts(108,5): error TS23 type Baz> = { [K in keyof Q]: T[Q[K]] }; type Qux> = { [K in keyof Q]: T[Q[K]["0"]] }; + + // Repro from #32038 + + const actions = ['resizeTo', 'resizeBy'] as const; + for (const action of actions) { + window[action] = (x, y) => { + window[action](x, y); + } + } \ No newline at end of file diff --git a/tests/baselines/reference/keyofAndIndexedAccess2.js b/tests/baselines/reference/keyofAndIndexedAccess2.js index 91cf2c89ff24b..a2d7f9b2fdde9 100644 --- a/tests/baselines/reference/keyofAndIndexedAccess2.js +++ b/tests/baselines/reference/keyofAndIndexedAccess2.js @@ -156,6 +156,15 @@ type Bar = { [key: string]: { [K in keyof T]: [K] }[keyof T] }; type Baz> = { [K in keyof Q]: T[Q[K]] }; type Qux> = { [K in keyof Q]: T[Q[K]["0"]] }; + +// Repro from #32038 + +const actions = ['resizeTo', 'resizeBy'] as const; +for (const action of actions) { + window[action] = (x, y) => { + window[action](x, y); + } +} //// [keyofAndIndexedAccess2.js] @@ -253,3 +262,10 @@ export class c { this["a"] = "b"; } } +// Repro from #32038 +const actions = ['resizeTo', 'resizeBy']; +for (const action of actions) { + window[action] = (x, y) => { + window[action](x, y); + }; +} diff --git a/tests/baselines/reference/keyofAndIndexedAccess2.symbols b/tests/baselines/reference/keyofAndIndexedAccess2.symbols index 4a7721ff801a0..58b5f470c030b 100644 --- a/tests/baselines/reference/keyofAndIndexedAccess2.symbols +++ b/tests/baselines/reference/keyofAndIndexedAccess2.symbols @@ -564,3 +564,26 @@ type Qux> = { [K in keyof Q]: T[Q[K]["0"]] }; >Q : Symbol(Q, Decl(keyofAndIndexedAccess2.ts, 156, 11)) >K : Symbol(K, Decl(keyofAndIndexedAccess2.ts, 156, 35)) +// Repro from #32038 + +const actions = ['resizeTo', 'resizeBy'] as const; +>actions : Symbol(actions, Decl(keyofAndIndexedAccess2.ts, 160, 5)) + +for (const action of actions) { +>action : Symbol(action, Decl(keyofAndIndexedAccess2.ts, 161, 10)) +>actions : Symbol(actions, Decl(keyofAndIndexedAccess2.ts, 160, 5)) + + window[action] = (x, y) => { +>window : Symbol(window, Decl(lib.dom.d.ts, --, --)) +>action : Symbol(action, Decl(keyofAndIndexedAccess2.ts, 161, 10)) +>x : Symbol(x, Decl(keyofAndIndexedAccess2.ts, 162, 19)) +>y : Symbol(y, Decl(keyofAndIndexedAccess2.ts, 162, 21)) + + window[action](x, y); +>window : Symbol(window, Decl(lib.dom.d.ts, --, --)) +>action : Symbol(action, Decl(keyofAndIndexedAccess2.ts, 161, 10)) +>x : Symbol(x, Decl(keyofAndIndexedAccess2.ts, 162, 19)) +>y : Symbol(y, Decl(keyofAndIndexedAccess2.ts, 162, 21)) + } +} + diff --git a/tests/baselines/reference/keyofAndIndexedAccess2.types b/tests/baselines/reference/keyofAndIndexedAccess2.types index b282588cc5159..5824619c6ee01 100644 --- a/tests/baselines/reference/keyofAndIndexedAccess2.types +++ b/tests/baselines/reference/keyofAndIndexedAccess2.types @@ -540,3 +540,35 @@ type Baz> = { [K in keyof Q]: T[Q[K]] }; type Qux> = { [K in keyof Q]: T[Q[K]["0"]] }; >Qux : Qux +// Repro from #32038 + +const actions = ['resizeTo', 'resizeBy'] as const; +>actions : readonly ["resizeTo", "resizeBy"] +>['resizeTo', 'resizeBy'] as const : readonly ["resizeTo", "resizeBy"] +>['resizeTo', 'resizeBy'] : readonly ["resizeTo", "resizeBy"] +>'resizeTo' : "resizeTo" +>'resizeBy' : "resizeBy" + +for (const action of actions) { +>action : "resizeTo" | "resizeBy" +>actions : readonly ["resizeTo", "resizeBy"] + + window[action] = (x, y) => { +>window[action] = (x, y) => { window[action](x, y); } : (x: number, y: number) => void +>window[action] : ((x: number, y: number) => void) & ((x: number, y: number) => void) +>window : Window +>action : "resizeTo" | "resizeBy" +>(x, y) => { window[action](x, y); } : (x: number, y: number) => void +>x : number +>y : number + + window[action](x, y); +>window[action](x, y) : void +>window[action] : ((x: number, y: number) => void) | ((x: number, y: number) => void) +>window : Window +>action : "resizeTo" | "resizeBy" +>x : number +>y : number + } +} + From ea0a6de82fa0bb4c4ee07a53233d6eabc506c413 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 26 Jun 2019 18:09:30 -1000 Subject: [PATCH 4/5] Compare type parameters, constraints, and defaults in signature identity --- src/compiler/checker.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 414d815d5d20e..41520ea899221 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -14322,20 +14322,25 @@ namespace ts { if (!(isMatchingSignature(source, target, partialMatch))) { return Ternary.False; } - // 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. + // Check that the two signatures have the same number of type parameters. if (length(source.typeParameters) !== length(target.typeParameters)) { 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); + // Check that type parameter constraints and defaults match. If they do, instantiate the source + // signature with the type parameters of the target signature and continue the comparison. + if (target.typeParameters) { + const mapper = createTypeMapper(source.typeParameters!, target.typeParameters); + for (let i = 0; i < target.typeParameters.length; i++) { + const s = source.typeParameters![i]; + const t = target.typeParameters[i]; + if (!(s === t || compareTypes(instantiateType(getConstraintFromTypeParameter(s), mapper) || unknownType, getConstraintFromTypeParameter(t) || unknownType) && + compareTypes(instantiateType(getDefaultFromTypeParameter(s), mapper) || unknownType, getDefaultFromTypeParameter(t) || unknownType))) { + return Ternary.False; + } + } + source = instantiateSignature(source, mapper, /*eraseTypeParameters*/ true); + } let result = Ternary.True; - if (!ignoreThisTypes) { const sourceThisType = getThisTypeOfSignature(source); if (sourceThisType) { @@ -14349,7 +14354,6 @@ namespace ts { } } } - const targetLen = getParameterCount(target); for (let i = 0; i < targetLen; i++) { const s = getTypeAtPosition(source, i); From 10c9fbc357a98c33bf04653d5086b540f243a2fa Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 26 Jun 2019 18:11:03 -1000 Subject: [PATCH 5/5] Accept new baselines --- .../genericSignatureIdentity.errors.txt | 36 +++++++++++++++++++ ...naturesWithTypeParametersAndAny.errors.txt | 6 +++- ...turesWithTypeParametersSwitched.errors.txt | 9 +++++ .../reference/promiseIdentity.errors.txt | 28 +++++++++++++++ .../promiseIdentityWithAny.errors.txt | 17 +++++++++ .../promiseIdentityWithConstraints.errors.txt | 17 +++++++++ 6 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/genericSignatureIdentity.errors.txt create mode 100644 tests/baselines/reference/identityForSignaturesWithTypeParametersSwitched.errors.txt create mode 100644 tests/baselines/reference/promiseIdentity.errors.txt create mode 100644 tests/baselines/reference/promiseIdentityWithAny.errors.txt create mode 100644 tests/baselines/reference/promiseIdentityWithConstraints.errors.txt diff --git a/tests/baselines/reference/genericSignatureIdentity.errors.txt b/tests/baselines/reference/genericSignatureIdentity.errors.txt new file mode 100644 index 0000000000000..6aed55a123719 --- /dev/null +++ b/tests/baselines/reference/genericSignatureIdentity.errors.txt @@ -0,0 +1,36 @@ +tests/cases/compiler/genericSignatureIdentity.ts(10,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type '(x: T) => T', but here has type '(x: T) => T'. +tests/cases/compiler/genericSignatureIdentity.ts(14,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type '(x: T) => T', but here has type '(x: T) => T'. +tests/cases/compiler/genericSignatureIdentity.ts(18,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type '(x: T) => T', but here has type '(x: any) => any'. + + +==== tests/cases/compiler/genericSignatureIdentity.ts (3 errors) ==== + // 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: { + ~ +!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type '(x: T) => T', but here has type '(x: T) => T'. +!!! related TS6203 tests/cases/compiler/genericSignatureIdentity.ts:6:5: 'x' was also declared here. + (x: T): T; + }; + + var x: { + ~ +!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type '(x: T) => T', but here has type '(x: T) => T'. +!!! related TS6203 tests/cases/compiler/genericSignatureIdentity.ts:6:5: 'x' was also declared here. + (x: T): T; + }; + + var x: { + ~ +!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type '(x: T) => T', but here has type '(x: any) => any'. +!!! related TS6203 tests/cases/compiler/genericSignatureIdentity.ts:6:5: 'x' was also declared here. + (x: any): any; + }; + \ No newline at end of file diff --git a/tests/baselines/reference/identityForSignaturesWithTypeParametersAndAny.errors.txt b/tests/baselines/reference/identityForSignaturesWithTypeParametersAndAny.errors.txt index 092bfaaeeb3c9..11fb2950aa80d 100644 --- a/tests/baselines/reference/identityForSignaturesWithTypeParametersAndAny.errors.txt +++ b/tests/baselines/reference/identityForSignaturesWithTypeParametersAndAny.errors.txt @@ -1,12 +1,16 @@ +tests/cases/compiler/identityForSignaturesWithTypeParametersAndAny.ts(2,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'f' must be of type '(x: T, y: U) => T', but here has type '(x: any, y: any) => any'. tests/cases/compiler/identityForSignaturesWithTypeParametersAndAny.ts(5,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'g' must be of type '(x: T, y: U) => T', but here has type '(x: any, y: any) => any'. tests/cases/compiler/identityForSignaturesWithTypeParametersAndAny.ts(8,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'h' must be of type '(x: T, y: U) => T', but here has type '(x: any, y: any) => any'. tests/cases/compiler/identityForSignaturesWithTypeParametersAndAny.ts(11,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'i' must be of type '(x: T, y: U) => T', but here has type '(x: any, y: string) => any'. tests/cases/compiler/identityForSignaturesWithTypeParametersAndAny.ts(14,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'j' must be of type '(x: T, y: U) => T', but here has type '(x: any, y: any) => string'. -==== tests/cases/compiler/identityForSignaturesWithTypeParametersAndAny.ts (4 errors) ==== +==== tests/cases/compiler/identityForSignaturesWithTypeParametersAndAny.ts (5 errors) ==== var f: (x: T, y: U) => T; var f: (x: any, y: any) => any; + ~ +!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'f' must be of type '(x: T, y: U) => T', but here has type '(x: any, y: any) => any'. +!!! related TS6203 tests/cases/compiler/identityForSignaturesWithTypeParametersAndAny.ts:1:5: 'f' was also declared here. var g: (x: T, y: U) => T; var g: (x: any, y: any) => any; diff --git a/tests/baselines/reference/identityForSignaturesWithTypeParametersSwitched.errors.txt b/tests/baselines/reference/identityForSignaturesWithTypeParametersSwitched.errors.txt new file mode 100644 index 0000000000000..81d0eb6c96148 --- /dev/null +++ b/tests/baselines/reference/identityForSignaturesWithTypeParametersSwitched.errors.txt @@ -0,0 +1,9 @@ +tests/cases/compiler/identityForSignaturesWithTypeParametersSwitched.ts(2,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'f' must be of type '(x: T, y: U) => T', but here has type '(x: U, y: T) => U'. + + +==== tests/cases/compiler/identityForSignaturesWithTypeParametersSwitched.ts (1 errors) ==== + var f: (x: T, y: U) => T; + var f: (x: U, y: T) => U; + ~ +!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'f' must be of type '(x: T, y: U) => T', but here has type '(x: U, y: T) => U'. +!!! related TS6203 tests/cases/compiler/identityForSignaturesWithTypeParametersSwitched.ts:1:5: 'f' was also declared here. \ No newline at end of file diff --git a/tests/baselines/reference/promiseIdentity.errors.txt b/tests/baselines/reference/promiseIdentity.errors.txt new file mode 100644 index 0000000000000..b104dc68e5f3a --- /dev/null +++ b/tests/baselines/reference/promiseIdentity.errors.txt @@ -0,0 +1,28 @@ +tests/cases/compiler/promiseIdentity.ts(21,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'y' must be of type 'IPromise2', but here has type 'Promise2'. + + +==== tests/cases/compiler/promiseIdentity.ts (1 errors) ==== + export interface IPromise { + then(callback: (x: T) => IPromise): IPromise; + } + interface Promise { + then(callback: (x: T) => Promise): Promise; + } + var x: IPromise; + var x: Promise; + + + interface IPromise2 { + then(callback: (x: T) => IPromise2): IPromise2; + } + interface Promise2 { + then(callback: (x: V) => Promise2): Promise2; // Uses V instead of T in callback's parameter + } + + // Ok because T in this particular Promise2 is any, as are all the U and W references. + // Also, the V of Promise2 happens to coincide with the T of IPromise2 (they are both string). + var y: IPromise2; + var y: Promise2; + ~ +!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'y' must be of type 'IPromise2', but here has type 'Promise2'. +!!! related TS6203 tests/cases/compiler/promiseIdentity.ts:20:5: 'y' was also declared here. \ No newline at end of file diff --git a/tests/baselines/reference/promiseIdentityWithAny.errors.txt b/tests/baselines/reference/promiseIdentityWithAny.errors.txt new file mode 100644 index 0000000000000..c318925c61756 --- /dev/null +++ b/tests/baselines/reference/promiseIdentityWithAny.errors.txt @@ -0,0 +1,17 @@ +tests/cases/compiler/promiseIdentityWithAny.ts(10,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'IPromise', but here has type 'Promise'. + + +==== tests/cases/compiler/promiseIdentityWithAny.ts (1 errors) ==== + export interface IPromise { + then(callback: (x: T) => IPromise): IPromise; + } + export interface Promise { + then(callback: (x: T) => Promise): Promise; + } + + // Should be ok because signature type parameters get erased to any + var x: IPromise; + var x: Promise; + ~ +!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'IPromise', but here has type 'Promise'. +!!! related TS6203 tests/cases/compiler/promiseIdentityWithAny.ts:9:5: 'x' was also declared here. \ No newline at end of file diff --git a/tests/baselines/reference/promiseIdentityWithConstraints.errors.txt b/tests/baselines/reference/promiseIdentityWithConstraints.errors.txt new file mode 100644 index 0000000000000..9b72f8363ed54 --- /dev/null +++ b/tests/baselines/reference/promiseIdentityWithConstraints.errors.txt @@ -0,0 +1,17 @@ +tests/cases/compiler/promiseIdentityWithConstraints.ts(10,5): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'IPromise', but here has type 'Promise'. + + +==== tests/cases/compiler/promiseIdentityWithConstraints.ts (1 errors) ==== + export interface IPromise { + then(callback: (x: T) => IPromise): IPromise; + } + export interface Promise { + then(callback: (x: T) => Promise): Promise; + } + + // Error because constraint V doesn't match + var x: IPromise; + var x: Promise; + ~ +!!! error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'IPromise', but here has type 'Promise'. +!!! related TS6203 tests/cases/compiler/promiseIdentityWithConstraints.ts:9:5: 'x' was also declared here. \ No newline at end of file