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.

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

Fix crash when creating a union signature from signatures that do and don’t have this types #31560

Merged
merged 1 commit into from May 24, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/compiler/checker.ts
Expand Up @@ -7052,10 +7052,10 @@ namespace ts {
// Union the result types when more than one signature matches
if (unionSignatures.length > 1) {
let thisParameter = signature.thisParameter;
if (forEach(unionSignatures, sig => sig.thisParameter)) {
// TODO: GH#18217 We tested that *some* has thisParameter and now act as if *all* do
const firstThisParameterOfUnionSignatures = forEach(unionSignatures, sig => sig.thisParameter);
if (firstThisParameterOfUnionSignatures) {
const thisType = getUnionType(map(unionSignatures, sig => sig.thisParameter ? getTypeOfSymbol(sig.thisParameter) : anyType), UnionReduction.Subtype);
thisParameter = createSymbolWithType(signature.thisParameter!, thisType);
thisParameter = createSymbolWithType(firstThisParameterOfUnionSignatures, thisType);
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 feels fairly arbitrary to just pick the first one of these as the source for the new transient symbol, but as far as I can tell it doesn't matter. I thought maybe it would cause an odd behavior for go-to-definition, but then I discovered that go-to-definition on union signatures is already arbitrarily limited to a single signature (#31559), and it doesn't seem to be affected by this change. I briefly experimented with doing a proper merge of every thisParameter symbol, but since I couldn't find any observable effect and go-to-definition on the call expression is already kinda broken, I opted not to use the extra memory and CPU cycles to perform a merge that you probably can't observe anyway. It might be worth revisiting this in #31547 or #31559.

}
s = createUnionSignature(signature, unionSignatures);
s.thisParameter = thisParameter;
Expand Down
17 changes: 17 additions & 0 deletions tests/baselines/reference/unionTypeCallSignatures5.js
@@ -0,0 +1,17 @@
//// [unionTypeCallSignatures5.ts]
// #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);


//// [unionTypeCallSignatures5.js]
fn(0);
31 changes: 31 additions & 0 deletions tests/baselines/reference/unionTypeCallSignatures5.symbols
@@ -0,0 +1,31 @@
=== tests/cases/conformance/types/union/unionTypeCallSignatures5.ts ===
// #31485
interface A {
>A : Symbol(A, Decl(unionTypeCallSignatures5.ts, 0, 0))

(this: void, b?: number): void;
>this : Symbol(this, Decl(unionTypeCallSignatures5.ts, 2, 3))
>b : Symbol(b, Decl(unionTypeCallSignatures5.ts, 2, 14))
}
interface B {
>B : Symbol(B, Decl(unionTypeCallSignatures5.ts, 3, 1))

(this: number, b?: number): void;
>this : Symbol(this, Decl(unionTypeCallSignatures5.ts, 5, 3))
>b : Symbol(b, Decl(unionTypeCallSignatures5.ts, 5, 16))
}
interface C {
>C : Symbol(C, Decl(unionTypeCallSignatures5.ts, 6, 1))

(i: number): void;
>i : Symbol(i, Decl(unionTypeCallSignatures5.ts, 8, 3))
}
declare const fn: A | B | C;
>fn : Symbol(fn, Decl(unionTypeCallSignatures5.ts, 10, 13))
>A : Symbol(A, Decl(unionTypeCallSignatures5.ts, 0, 0))
>B : Symbol(B, Decl(unionTypeCallSignatures5.ts, 3, 1))
>C : Symbol(C, Decl(unionTypeCallSignatures5.ts, 6, 1))

fn(0);
>fn : Symbol(fn, Decl(unionTypeCallSignatures5.ts, 10, 13))

24 changes: 24 additions & 0 deletions tests/baselines/reference/unionTypeCallSignatures5.types
@@ -0,0 +1,24 @@
=== tests/cases/conformance/types/union/unionTypeCallSignatures5.ts ===
// #31485
interface A {
(this: void, b?: number): void;
>this : void
>b : number
}
interface B {
(this: number, b?: number): void;
>this : number
>b : number
}
interface C {
(i: number): void;
>i : number
}
declare const fn: A | B | C;
>fn : A | B | C

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

12 changes: 12 additions & 0 deletions tests/cases/conformance/types/union/unionTypeCallSignatures5.ts
@@ -0,0 +1,12 @@
// #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);