Skip to content

Intersect 'this' types in union signatures #32538

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 2 commits into from
Jul 26, 2019
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
13 changes: 7 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7169,8 +7169,9 @@ namespace ts {
}
let result: Signature[] | undefined;
for (let i = 0; i < signatureLists.length; i++) {
// Allow matching non-generic signatures to have excess parameters and different return types
const match = i === listIndex ? signature : findMatchingSignature(signatureLists[i], signature, /*partialMatch*/ true, /*ignoreThisTypes*/ true, /*ignoreReturnTypes*/ true);
// Allow matching non-generic signatures to have excess parameters and different return types.
// Prefer matching this types if possible.
const match = i === listIndex ? signature : findMatchingSignature(signatureLists[i], signature, /*partialMatch*/ true, /*ignoreThisTypes*/ false, /*ignoreReturnTypes*/ true);
if (!match) {
return undefined;
}
Expand All @@ -7193,7 +7194,7 @@ namespace ts {
}
for (const signature of signatureLists[i]) {
// Only process signatures with parameter lists that aren't already in the result list
if (!result || !findMatchingSignature(result, signature, /*partialMatch*/ false, /*ignoreThisTypes*/ true, /*ignoreReturnTypes*/ true)) {
if (!result || !findMatchingSignature(result, signature, /*partialMatch*/ false, /*ignoreThisTypes*/ false, /*ignoreReturnTypes*/ true)) {
const unionSignatures = findMatchingSignatures(signatureLists, signature, i);
if (unionSignatures) {
let s = signature;
Expand All @@ -7202,7 +7203,7 @@ namespace ts {
let thisParameter = signature.thisParameter;
const firstThisParameterOfUnionSignatures = forEach(unionSignatures, sig => sig.thisParameter);
if (firstThisParameterOfUnionSignatures) {
const thisType = getUnionType(map(unionSignatures, sig => sig.thisParameter ? getTypeOfSymbol(sig.thisParameter) : anyType), UnionReduction.Subtype);
const thisType = getIntersectionType(mapDefined(unionSignatures, sig => sig.thisParameter && getTypeOfSymbol(sig.thisParameter)));
thisParameter = createSymbolWithType(firstThisParameterOfUnionSignatures, thisType);
}
s = createUnionSignature(signature, unionSignatures);
Expand Down Expand Up @@ -7241,8 +7242,8 @@ namespace ts {
}
// A signature `this` type might be a read or a write position... It's very possible that it should be invariant
// and we should refuse to merge signatures if there are `this` types and they do not match. However, so as to be
// permissive when calling, for now, we'll union the `this` types just like the overlapping-union-signature check does
const thisType = getUnionType([getTypeOfSymbol(left), getTypeOfSymbol(right)], UnionReduction.Subtype);
// permissive when calling, for now, we'll intersect the `this` types just like we do for param types in union signatures.
const thisType = getIntersectionType([getTypeOfSymbol(left), getTypeOfSymbol(right)]);
return createSymbolWithType(left, thisType);
}

Expand Down
34 changes: 34 additions & 0 deletions tests/baselines/reference/unionThisTypeInFunctions.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
tests/cases/conformance/types/thisType/unionThisTypeInFunctions.ts(10,5): error TS2684: The 'this' context of type 'Real | Fake' is not assignable to method's 'this' of type 'Real & Fake'.
Type 'Real' is not assignable to type 'Real & Fake'.
Type 'Real' is not assignable to type 'Fake'.
Types of property 'method' are incompatible.
Type '(this: Real, n: number) => void' is not assignable to type '(this: Fake, n: number) => void'.
The 'this' types of each signature are incompatible.
Type 'Fake' is not assignable to type 'Real'.
Types of property 'data' are incompatible.
Type 'number' is not assignable to type 'string'.


==== tests/cases/conformance/types/thisType/unionThisTypeInFunctions.ts (1 errors) ====
interface Real {
method(this: this, n: number): void;
data: string;
}
interface Fake {
method(this: this, n: number): void;
data: number;
}
function test(r: Real | Fake) {
r.method(12); // error
Copy link
Member

Choose a reason for hiding this comment

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

Not a big fan of this, since it's prima facie Obviously Legal.

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean, this is the crux of the change 😅

Copy link
Member

Choose a reason for hiding this comment

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

Well, it's Obviously Legal and Technically Incorrect, and in this case I think Technically Incorrect should win.

~
!!! error TS2684: The 'this' context of type 'Real | Fake' is not assignable to method's 'this' of type 'Real & Fake'.
!!! error TS2684: Type 'Real' is not assignable to type 'Real & Fake'.
!!! error TS2684: Type 'Real' is not assignable to type 'Fake'.
!!! error TS2684: Types of property 'method' are incompatible.
!!! error TS2684: Type '(this: Real, n: number) => void' is not assignable to type '(this: Fake, n: number) => void'.
!!! error TS2684: The 'this' types of each signature are incompatible.
!!! error TS2684: Type 'Fake' is not assignable to type 'Real'.
!!! error TS2684: Types of property 'data' are incompatible.
!!! error TS2684: Type 'number' is not assignable to type 'string'.
}

4 changes: 2 additions & 2 deletions tests/baselines/reference/unionThisTypeInFunctions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ interface Fake {
data: number;
}
function test(r: Real | Fake) {
r.method(12);
r.method(12); // error
}


//// [unionThisTypeInFunctions.js]
function test(r) {
r.method(12);
r.method(12); // error
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/unionThisTypeInFunctions.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function test(r: Real | Fake) {
>Real : Symbol(Real, Decl(unionThisTypeInFunctions.ts, 0, 0))
>Fake : Symbol(Fake, Decl(unionThisTypeInFunctions.ts, 3, 1))

r.method(12);
r.method(12); // error
>r.method : Symbol(method, Decl(unionThisTypeInFunctions.ts, 0, 16), Decl(unionThisTypeInFunctions.ts, 4, 16))
>r : Symbol(r, Decl(unionThisTypeInFunctions.ts, 8, 14))
>method : Symbol(method, Decl(unionThisTypeInFunctions.ts, 0, 16), Decl(unionThisTypeInFunctions.ts, 4, 16))
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/unionThisTypeInFunctions.types
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ function test(r: Real | Fake) {
>test : (r: Real | Fake) => void
>r : Real | Fake

r.method(12);
>r.method(12) : void
r.method(12); // error
>r.method(12) : any
>r.method : ((this: Real, n: number) => void) | ((this: Fake, n: number) => void)
>r : Real | Fake
>method : ((this: Real, n: number) => void) | ((this: Fake, n: number) => void)
Expand Down
19 changes: 19 additions & 0 deletions tests/baselines/reference/unionTypeCallSignatures5.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
tests/cases/conformance/types/union/unionTypeCallSignatures5.ts(12,1): error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'never'.


==== tests/cases/conformance/types/union/unionTypeCallSignatures5.ts (1 errors) ====
// #31485
interface A {
(this: void, b?: number): void;
}
interface B {
(this: number, b?: number): void;
}
interface C {
(i: number): void;
}
declare const fn: A | B | C;
fn(0);
~~~~~
!!! error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'never'.
Copy link
Member Author

Choose a reason for hiding this comment

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

I’m not sure what to do with this: void. It doesn’t have any special handling right now, so it intersects with number to produce never. But it feels fairly synonymous with completely omitting a this type, in which case I would drop it out of the intersection. I’m a bit fuzzy on how to interpret the void type outside of return types.

Copy link
Member

Choose a reason for hiding this comment

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

void is usually best treated as synonymous with undefined in all non-return positions - at least that's what we've done thus far (despite the fact that any return type is assignable to a void returning function, thus you cannot guarantee that a void returning function returns undefined). It's a bit inconsistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok; by that logic I think the intersection stands.

Copy link
Member

Choose a reason for hiding this comment

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

In this parameters, void is definitely a synonym of undefined. I wrote the feature before undefined existed. this: void is (was?) the idiomatic way to specify that there will be no this provided.

never seems right because it arises from an union of signatures that require both (1) no this provided (2) some this provided.


2 changes: 1 addition & 1 deletion tests/baselines/reference/unionTypeCallSignatures5.types
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ declare const fn: A | B | C;
>fn : A | B | C

fn(0);
>fn(0) : void
>fn(0) : any
>fn : A | B | C
>0 : 0

98 changes: 98 additions & 0 deletions tests/baselines/reference/unionTypeCallSignatures6.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
tests/cases/conformance/types/union/unionTypeCallSignatures6.ts(11,1): error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B'.
Type 'void' is not assignable to type 'A'.
tests/cases/conformance/types/union/unionTypeCallSignatures6.ts(13,1): error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'A'.
tests/cases/conformance/types/union/unionTypeCallSignatures6.ts(38,4): error TS2349: This expression is not callable.
Each member of the union type 'F3 | F4' has signatures, but none of those signatures are compatible with each other.
tests/cases/conformance/types/union/unionTypeCallSignatures6.ts(39,1): error TS2684: The 'this' context of type 'A & C & { f0: F0 | F3; f1: F1 | F3; f2: F1 | F4; f3: F3 | F4; f4: F3 | F5; }' is not assignable to method's 'this' of type 'B'.
Property 'b' is missing in type 'A & C & { f0: F0 | F3; f1: F1 | F3; f2: F1 | F4; f3: F3 | F4; f4: F3 | F5; }' but required in type 'B'.
tests/cases/conformance/types/union/unionTypeCallSignatures6.ts(48,1): error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B'.
Type 'void' is not assignable to type 'A'.
tests/cases/conformance/types/union/unionTypeCallSignatures6.ts(55,1): error TS2769: No overload matches this call.
Overload 1 of 2, '(this: A & B & C): void', gave the following error.
The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B & C'.
Type 'void' is not assignable to type 'A'.
Overload 2 of 2, '(this: A & B): void', gave the following error.
The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B'.
Type 'void' is not assignable to type 'A'.


==== tests/cases/conformance/types/union/unionTypeCallSignatures6.ts (6 errors) ====
type A = { a: string };
type B = { b: number };
type C = { c: string };
type D = { d: number };
type F0 = () => void;

// #31547
type F1 = (this: A) => void;
type F2 = (this: B) => void;
declare var f1: F1 | F2;
f1(); // error
~~~~
!!! error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B'.
!!! error TS2684: Type 'void' is not assignable to type 'A'.
declare var f2: F0 | F1;
f2(); // error
~~~~
!!! error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'A'.

interface F3 {
(this: A): void;
(this: B): void;
}
interface F4 {
(this: C): void;
(this: D): void;
}
interface F5 {
(this: C): void;
(this: B): void;
}

declare var x1: A & C & {
f0: F0 | F3;
f1: F1 | F3;
f2: F1 | F4;
f3: F3 | F4;
f4: F3 | F5;
}
x1.f0();
x1.f1();
x1.f2();
x1.f3(); // error
~~
!!! error TS2349: This expression is not callable.
!!! error TS2349: Each member of the union type 'F3 | F4' has signatures, but none of those signatures are compatible with each other.
x1.f4(); // error
~~
!!! error TS2684: The 'this' context of type 'A & C & { f0: F0 | F3; f1: F1 | F3; f2: F1 | F4; f3: F3 | F4; f4: F3 | F5; }' is not assignable to method's 'this' of type 'B'.
!!! error TS2684: Property 'b' is missing in type 'A & C & { f0: F0 | F3; f1: F1 | F3; f2: F1 | F4; f3: F3 | F4; f4: F3 | F5; }' but required in type 'B'.
!!! related TS2728 tests/cases/conformance/types/union/unionTypeCallSignatures6.ts:2:12: 'b' is declared here.

declare var x2: A & B & {
f4: F3 | F5;
}
x2.f4();

type F6 = (this: A & B) => void;
declare var f3: F1 | F6;
f3(); // error
~~~~
!!! error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B'.
!!! error TS2684: Type 'void' is not assignable to type 'A'.

interface F7 {
(this: A & B & C): void;
(this: A & B): void;
}
declare var f4: F6 | F7;
f4(); // error
~~~~
!!! error TS2769: No overload matches this call.
!!! error TS2769: Overload 1 of 2, '(this: A & B & C): void', gave the following error.
!!! error TS2769: The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B & C'.
!!! error TS2769: Type 'void' is not assignable to type 'A'.
!!! error TS2769: Overload 2 of 2, '(this: A & B): void', gave the following error.
!!! error TS2769: The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B'.
!!! error TS2769: Type 'void' is not assignable to type 'A'.

69 changes: 69 additions & 0 deletions tests/baselines/reference/unionTypeCallSignatures6.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//// [unionTypeCallSignatures6.ts]
type A = { a: string };
type B = { b: number };
type C = { c: string };
type D = { d: number };
type F0 = () => void;

// #31547
type F1 = (this: A) => void;
type F2 = (this: B) => void;
declare var f1: F1 | F2;
f1(); // error
declare var f2: F0 | F1;
f2(); // error

interface F3 {
(this: A): void;
(this: B): void;
}
interface F4 {
(this: C): void;
(this: D): void;
}
interface F5 {
(this: C): void;
(this: B): void;
}

declare var x1: A & C & {
f0: F0 | F3;
f1: F1 | F3;
f2: F1 | F4;
f3: F3 | F4;
f4: F3 | F5;
}
x1.f0();
x1.f1();
x1.f2();
x1.f3(); // error
x1.f4(); // error

declare var x2: A & B & {
f4: F3 | F5;
}
x2.f4();

type F6 = (this: A & B) => void;
declare var f3: F1 | F6;
f3(); // error

interface F7 {
(this: A & B & C): void;
(this: A & B): void;
}
declare var f4: F6 | F7;
f4(); // error


//// [unionTypeCallSignatures6.js]
f1(); // error
f2(); // error
x1.f0();
x1.f1();
x1.f2();
x1.f3(); // error
x1.f4(); // error
x2.f4();
f3(); // error
f4(); // error
Loading