From 5b63a8b09ca9ebf4a06002f52914379c17c61430 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Fri, 13 Oct 2017 08:51:48 -0700 Subject: [PATCH 1/3] Continue inferring single generic signature's type parameters Type inference initially skips context-sensitive arguments when inferring type parameters. After it has inferred all it can from the non-context-sensitive arguments, it enables the context-sensitive arguments one at a time and retries inference. Previously, this process would stop as soon as inferences resulted in an erroneous signature -- one that was not assignable to the original signature. But stopping early can miss inferences from context-sensitive arguments that might not result in errors. This changes `chooseOverload` to continue inferring from context-sensitive arguments, even when a previous set of inferences resulted in an erroneous signature. `chooseOverload` doesn't give up until it has collected inferences from all arguments. This improves inference in tricky cases like this: ```ts declare function inference(target: T, name: keyof T): void; inference({ a: 1, b: 2, c: 3, d(n) { return n } }, "d"); ``` Here, the only inferences come from the context-sensitive object literal -- inferring from `"d"` to `keyof T` gives nothing. But those inferences come in the second round, even though the first round is an error after inferring `T={}` and noticing that `"d"` is not in `keyof {}`. Notably, however, this improvement only applies to generic signatures without overloads. The resolution process interleaves overload checking with the progressive checking of context-sensitive arguments. Because of the way that inference can fix the types of context-sensitive arguments, checking more context-sensitive arguments after encountering an error can leave those arguments fixed to types that are known to be incorrect. For example: ```ts function forEachKey(map: Map<__String>, callback: (key: __String) => T | undefined): T | undefined; function forEachKey(map: Map, callback: (key: string) => T | undefined): T | undefined; forEachKey(createMap(), key => 101); ``` If this improvement were applied to multiple overloads, then, when `createMap()` failed to match the first overload, inference would continue on to inference from `key` => 101`. It would infer `T=number`, but it would also fix the type of `key` to `__String`. Then it would fail to match the first overload again (`createMap()` is still of type `Map` not `Map<__String>`). When checking the second overload, though, `key` would *still* be fixe to `__String`, so even though `createMap()` would match, `key` would not. --- src/compiler/checker.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 9347329e624b2..7e3d9f253d0f2 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -16350,10 +16350,15 @@ namespace ts { } if (!checkApplicableSignature(node, args, candidate, relation, excludeArgument, /*reportErrors*/ false)) { candidateForArgumentError = candidate; - break; + if (candidates.length > 1) { + break; + } } if (excludeCount === 0) { candidates[candidateIndex] = candidate; + if (candidate === candidateForArgumentError) { + break; + } return candidate; } excludeCount--; From 00fb692ebccf7fc10b9212a863627a575afd63b3 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Fri, 13 Oct 2017 09:10:52 -0700 Subject: [PATCH 2/3] Update baselines --- ...contextualTypingWithFixedTypeParameters1.errors.txt | 8 ++++---- .../contextualTypingWithFixedTypeParameters1.types | 10 +++++----- .../typeInferenceConflictingCandidates.errors.txt | 4 ++-- .../reference/typeInferenceConflictingCandidates.types | 6 +++--- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/baselines/reference/contextualTypingWithFixedTypeParameters1.errors.txt b/tests/baselines/reference/contextualTypingWithFixedTypeParameters1.errors.txt index 0fe1e69d0799c..807e73092b26c 100644 --- a/tests/baselines/reference/contextualTypingWithFixedTypeParameters1.errors.txt +++ b/tests/baselines/reference/contextualTypingWithFixedTypeParameters1.errors.txt @@ -1,6 +1,6 @@ tests/cases/compiler/contextualTypingWithFixedTypeParameters1.ts(2,22): error TS2339: Property 'foo' does not exist on type 'string'. -tests/cases/compiler/contextualTypingWithFixedTypeParameters1.ts(3,32): error TS2339: Property 'foo' does not exist on type '""'. -tests/cases/compiler/contextualTypingWithFixedTypeParameters1.ts(3,38): error TS2345: Argument of type '1' is not assignable to parameter of type '""'. +tests/cases/compiler/contextualTypingWithFixedTypeParameters1.ts(3,32): error TS2339: Property 'foo' does not exist on type 'string'. +tests/cases/compiler/contextualTypingWithFixedTypeParameters1.ts(3,38): error TS2345: Argument of type '1' is not assignable to parameter of type 'string'. ==== tests/cases/compiler/contextualTypingWithFixedTypeParameters1.ts (3 errors) ==== @@ -10,6 +10,6 @@ tests/cases/compiler/contextualTypingWithFixedTypeParameters1.ts(3,38): error TS !!! error TS2339: Property 'foo' does not exist on type 'string'. var r9 = f10('', () => (a => a.foo), 1); // error ~~~ -!!! error TS2339: Property 'foo' does not exist on type '""'. +!!! error TS2339: Property 'foo' does not exist on type 'string'. ~ -!!! error TS2345: Argument of type '1' is not assignable to parameter of type '""'. \ No newline at end of file +!!! error TS2345: Argument of type '1' is not assignable to parameter of type 'string'. \ No newline at end of file diff --git a/tests/baselines/reference/contextualTypingWithFixedTypeParameters1.types b/tests/baselines/reference/contextualTypingWithFixedTypeParameters1.types index c0100b060889f..84b2077523a9a 100644 --- a/tests/baselines/reference/contextualTypingWithFixedTypeParameters1.types +++ b/tests/baselines/reference/contextualTypingWithFixedTypeParameters1.types @@ -28,12 +28,12 @@ var r9 = f10('', () => (a => a.foo), 1); // error >f10('', () => (a => a.foo), 1) : any >f10 : (x: T, b: () => (a: T) => void, y: T) => T >'' : "" ->() => (a => a.foo) : () => (a: "") => any ->(a => a.foo) : (a: "") => any ->a => a.foo : (a: "") => any ->a : "" +>() => (a => a.foo) : () => (a: string) => any +>(a => a.foo) : (a: string) => any +>a => a.foo : (a: string) => any +>a : string >a.foo : any ->a : "" +>a : string >foo : any >1 : 1 diff --git a/tests/baselines/reference/typeInferenceConflictingCandidates.errors.txt b/tests/baselines/reference/typeInferenceConflictingCandidates.errors.txt index c98b39cb4fec6..752ed5235402c 100644 --- a/tests/baselines/reference/typeInferenceConflictingCandidates.errors.txt +++ b/tests/baselines/reference/typeInferenceConflictingCandidates.errors.txt @@ -1,4 +1,4 @@ -tests/cases/compiler/typeInferenceConflictingCandidates.ts(3,7): error TS2345: Argument of type '3' is not assignable to parameter of type '""'. +tests/cases/compiler/typeInferenceConflictingCandidates.ts(3,7): error TS2345: Argument of type '3' is not assignable to parameter of type 'string'. ==== tests/cases/compiler/typeInferenceConflictingCandidates.ts (1 errors) ==== @@ -6,4 +6,4 @@ tests/cases/compiler/typeInferenceConflictingCandidates.ts(3,7): error TS2345: A g("", 3, a => a); ~ -!!! error TS2345: Argument of type '3' is not assignable to parameter of type '""'. \ No newline at end of file +!!! error TS2345: Argument of type '3' is not assignable to parameter of type 'string'. \ No newline at end of file diff --git a/tests/baselines/reference/typeInferenceConflictingCandidates.types b/tests/baselines/reference/typeInferenceConflictingCandidates.types index c04a8045cd77b..3fb99cbbb1551 100644 --- a/tests/baselines/reference/typeInferenceConflictingCandidates.types +++ b/tests/baselines/reference/typeInferenceConflictingCandidates.types @@ -17,7 +17,7 @@ g("", 3, a => a); >g : (a: T, b: T, c: (t: T) => T) => T >"" : "" >3 : 3 ->a => a : (a: any) => any ->a : any ->a : any +>a => a : (a: string) => string +>a : string +>a : string From 3a234624d808844f5c03357a05fe628cb678bba8 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Fri, 13 Oct 2017 09:21:24 -0700 Subject: [PATCH 3/3] Test multi-round inference in presence of errors And update fourslash baselines --- .../multipleRoundInferenceAfterError.js | 8 ++++++ .../multipleRoundInferenceAfterError.symbols | 19 ++++++++++++++ .../multipleRoundInferenceAfterError.types | 25 +++++++++++++++++++ .../multipleRoundInferenceAfterError.ts | 3 +++ .../genericFunctionSignatureHelp3.ts | 24 +++++++++--------- .../genericFunctionSignatureHelp3MultiFile.ts | 16 ++++++------ 6 files changed, 75 insertions(+), 20 deletions(-) create mode 100644 tests/baselines/reference/multipleRoundInferenceAfterError.js create mode 100644 tests/baselines/reference/multipleRoundInferenceAfterError.symbols create mode 100644 tests/baselines/reference/multipleRoundInferenceAfterError.types create mode 100644 tests/cases/compiler/multipleRoundInferenceAfterError.ts diff --git a/tests/baselines/reference/multipleRoundInferenceAfterError.js b/tests/baselines/reference/multipleRoundInferenceAfterError.js new file mode 100644 index 0000000000000..372d154229872 --- /dev/null +++ b/tests/baselines/reference/multipleRoundInferenceAfterError.js @@ -0,0 +1,8 @@ +//// [multipleRoundInferenceAfterError.ts] +// Fixes #18715 +declare function inference(target: T, name: keyof T): void; +inference({ a: 1, b: 2, c: 3, d(n) { return n } }, "d"); + + +//// [multipleRoundInferenceAfterError.js] +inference({ a: 1, b: 2, c: 3, d: function (n) { return n; } }, "d"); diff --git a/tests/baselines/reference/multipleRoundInferenceAfterError.symbols b/tests/baselines/reference/multipleRoundInferenceAfterError.symbols new file mode 100644 index 0000000000000..05aae7f056991 --- /dev/null +++ b/tests/baselines/reference/multipleRoundInferenceAfterError.symbols @@ -0,0 +1,19 @@ +=== tests/cases/compiler/multipleRoundInferenceAfterError.ts === +// Fixes #18715 +declare function inference(target: T, name: keyof T): void; +>inference : Symbol(inference, Decl(multipleRoundInferenceAfterError.ts, 0, 0)) +>T : Symbol(T, Decl(multipleRoundInferenceAfterError.ts, 1, 27)) +>target : Symbol(target, Decl(multipleRoundInferenceAfterError.ts, 1, 30)) +>T : Symbol(T, Decl(multipleRoundInferenceAfterError.ts, 1, 27)) +>name : Symbol(name, Decl(multipleRoundInferenceAfterError.ts, 1, 40)) +>T : Symbol(T, Decl(multipleRoundInferenceAfterError.ts, 1, 27)) + +inference({ a: 1, b: 2, c: 3, d(n) { return n } }, "d"); +>inference : Symbol(inference, Decl(multipleRoundInferenceAfterError.ts, 0, 0)) +>a : Symbol(a, Decl(multipleRoundInferenceAfterError.ts, 2, 11)) +>b : Symbol(b, Decl(multipleRoundInferenceAfterError.ts, 2, 17)) +>c : Symbol(c, Decl(multipleRoundInferenceAfterError.ts, 2, 23)) +>d : Symbol(d, Decl(multipleRoundInferenceAfterError.ts, 2, 29)) +>n : Symbol(n, Decl(multipleRoundInferenceAfterError.ts, 2, 32)) +>n : Symbol(n, Decl(multipleRoundInferenceAfterError.ts, 2, 32)) + diff --git a/tests/baselines/reference/multipleRoundInferenceAfterError.types b/tests/baselines/reference/multipleRoundInferenceAfterError.types new file mode 100644 index 0000000000000..9878df88f5ef6 --- /dev/null +++ b/tests/baselines/reference/multipleRoundInferenceAfterError.types @@ -0,0 +1,25 @@ +=== tests/cases/compiler/multipleRoundInferenceAfterError.ts === +// Fixes #18715 +declare function inference(target: T, name: keyof T): void; +>inference : (target: T, name: keyof T) => void +>T : T +>target : T +>T : T +>name : keyof T +>T : T + +inference({ a: 1, b: 2, c: 3, d(n) { return n } }, "d"); +>inference({ a: 1, b: 2, c: 3, d(n) { return n } }, "d") : void +>inference : (target: T, name: keyof T) => void +>{ a: 1, b: 2, c: 3, d(n) { return n } } : { a: number; b: number; c: number; d(n: any): any; } +>a : number +>1 : 1 +>b : number +>2 : 2 +>c : number +>3 : 3 +>d : (n: any) => any +>n : any +>n : any +>"d" : "d" + diff --git a/tests/cases/compiler/multipleRoundInferenceAfterError.ts b/tests/cases/compiler/multipleRoundInferenceAfterError.ts new file mode 100644 index 0000000000000..b2bff4af5ed1a --- /dev/null +++ b/tests/cases/compiler/multipleRoundInferenceAfterError.ts @@ -0,0 +1,3 @@ +// Fixes #18715 +declare function inference(target: T, name: keyof T): void; +inference({ a: 1, b: 2, c: 3, d(n) { return n } }, "d"); diff --git a/tests/cases/fourslash/genericFunctionSignatureHelp3.ts b/tests/cases/fourslash/genericFunctionSignatureHelp3.ts index 5d4275d6ef944..053b0e684d08b 100644 --- a/tests/cases/fourslash/genericFunctionSignatureHelp3.ts +++ b/tests/cases/fourslash/genericFunctionSignatureHelp3.ts @@ -8,32 +8,32 @@ ////function foo6(x: number, callback: (y6: T) => number) { } ////function foo7(x: number, callback: (y7: T) => number) { } //// IDE shows the results on the right of each line, fourslash says different -////foo1(/*1*/ // signature help shows y as T +////foo1(/*1*/ // signature help shows y as {} ////foo2(1,/*2*/ // signature help shows y as {} -////foo3(1, (/*3*/ // signature help shows y as T +////foo3(1, (/*3*/ // signature help shows y as {} ////foo4(1,/*4*/ // signature help shows y as string -////foo5(1, (/*5*/ // signature help shows y as T +////foo5(1, (/*5*/ // signature help shows y as string ////foo6(1, (/*7*/ // signature help shows y as T +////foo7(1, (/*7*/ // signature help shows y as {} goTo.marker('1'); -verify.currentSignatureHelpIs('foo1(x: number, callback: (y1: T) => number): void'); +verify.currentSignatureHelpIs('foo1(x: number, callback: (y1: {}) => number): void'); -// goTo.marker('2'); -// verify.currentSignatureHelpIs('foo2(x: number, callback: (y2: {}) => number): void'); +goTo.marker('2'); +verify.currentSignatureHelpIs('foo2(x: number, callback: (y2: {}) => number): void'); goTo.marker('3'); -verify.currentSignatureHelpIs('foo3(x: number, callback: (y3: T) => number): void'); +verify.currentSignatureHelpIs('foo3(x: number, callback: (y3: {}) => number): void'); -// goTo.marker('4'); -// verify.currentSignatureHelpIs('foo4(x: number, callback: (y4: string) => number): void'); +goTo.marker('4'); +verify.currentSignatureHelpIs('foo4(x: number, callback: (y4: string) => number): void'); goTo.marker('5'); verify.currentSignatureHelpIs('foo5(x: number, callback: (y5: string) => number): void'); goTo.marker('6'); -// verify.currentSignatureHelpIs('foo6(x: number, callback: (y6: {}) => number): void'); +verify.currentSignatureHelpIs('foo6(x: number, callback: (y6: {}) => number): void'); edit.insert('string>(null,null);'); // need to make this line parse so we can get reasonable LS answers to later tests goTo.marker('7'); -verify.currentSignatureHelpIs('foo7(x: number, callback: (y7: T) => number): void'); +verify.currentSignatureHelpIs('foo7(x: number, callback: (y7: {}) => number): void'); diff --git a/tests/cases/fourslash/genericFunctionSignatureHelp3MultiFile.ts b/tests/cases/fourslash/genericFunctionSignatureHelp3MultiFile.ts index b82d5e10664bc..d1476660654a9 100644 --- a/tests/cases/fourslash/genericFunctionSignatureHelp3MultiFile.ts +++ b/tests/cases/fourslash/genericFunctionSignatureHelp3MultiFile.ts @@ -24,23 +24,23 @@ ////foo7(1, (/*7*/ // signature help shows y as T goTo.marker('1'); -verify.currentSignatureHelpIs('foo1(x: number, callback: (y1: T) => number): void'); +verify.currentSignatureHelpIs('foo1(x: number, callback: (y1: {}) => number): void'); -// goTo.marker('2'); -// verify.currentSignatureHelpIs('foo2(x: number, callback: (y2: {}) => number): void'); +goTo.marker('2'); +verify.currentSignatureHelpIs('foo2(x: number, callback: (y2: {}) => number): void'); goTo.marker('3'); -verify.currentSignatureHelpIs('foo3(x: number, callback: (y3: T) => number): void'); +verify.currentSignatureHelpIs('foo3(x: number, callback: (y3: {}) => number): void'); -// goTo.marker('4'); -// verify.currentSignatureHelpIs('foo4(x: number, callback: (y4: string) => number): void'); +goTo.marker('4'); +verify.currentSignatureHelpIs('foo4(x: number, callback: (y4: string) => number): void'); goTo.marker('5'); verify.currentSignatureHelpIs('foo5(x: number, callback: (y5: string) => number): void'); goTo.marker('6'); -// verify.currentSignatureHelpIs('foo6(x: number, callback: (y6: {}) => number): void'); +verify.currentSignatureHelpIs('foo6(x: number, callback: (y6: {}) => number): void'); edit.insert('string>(null,null);'); // need to make this line parse so we can get reasonable LS answers to later tests goTo.marker('7'); -verify.currentSignatureHelpIs('foo7(x: number, callback: (y7: T) => number): void'); +verify.currentSignatureHelpIs('foo7(x: number, callback: (y7: {}) => number): void');