Skip to content

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

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.

Sign up for GitHub

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 1 commit into from
May 24, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);