From 40f6ad12ac502984d351bcbf51d8806c587fdbf8 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 20 Jun 2018 11:54:02 -0700 Subject: [PATCH] In services, when overload resolution fails, create a union signature (2) --- src/compiler/checker.ts | 140 +++++++++++++----- src/compiler/utilities.ts | 16 ++ .../reference/api/tsserverlibrary.d.ts | 4 + .../fourslash/automaticConstructorToggling.ts | 4 +- .../fourslash/completionsCombineOverloads.ts | 9 ++ ...mpletionsCombineOverloads_restParameter.ts | 15 ++ .../completionsCombineOverloads_returnType.ts | 9 ++ .../genericFunctionSignatureHelp3.ts | 6 +- .../genericFunctionSignatureHelp3MultiFile.ts | 8 +- .../fourslash/jsDocFunctionSignatures10.ts | 2 +- tests/cases/fourslash/jsdocReturnsTag.ts | 2 +- ...Info_errorSignatureFillsInTypeParameter.ts | 6 + .../signatureHelpExplicitTypeArguments.ts | 2 +- .../signatureHelpOnTypePredicates.ts | 2 +- 14 files changed, 176 insertions(+), 49 deletions(-) create mode 100644 tests/cases/fourslash/completionsCombineOverloads.ts create mode 100644 tests/cases/fourslash/completionsCombineOverloads_restParameter.ts create mode 100644 tests/cases/fourslash/completionsCombineOverloads_returnType.ts create mode 100644 tests/cases/fourslash/quickInfo_errorSignatureFillsInTypeParameter.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index eb23835ffc4a9..ca41baf1c73f6 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7602,9 +7602,13 @@ namespace ts { return !signature.resolvedReturnType && findResolutionCycleStartIndex(signature, TypeSystemPropertyName.ResolvedReturnType) >= 0; } - function getRestTypeOfSignature(signature: Signature) { + function getRestTypeOfSignature(signature: Signature): Type { + return tryGetRestTypeOfSignature(signature) || anyType; + } + + function tryGetRestTypeOfSignature(signature: Signature): Type | undefined { const type = getTypeOfRestParameter(signature); - return type && getIndexTypeOfType(type, IndexKind.Number) || anyType; + return type && getIndexTypeOfType(type, IndexKind.Number); } function getSignatureInstantiation(signature: Signature, typeArguments: Type[] | undefined, isJavascript: boolean): Signature { @@ -18769,38 +18773,7 @@ namespace ts { diagnostics.add(createDiagnosticForNode(node, fallbackError)); } - // No signature was applicable. We have already reported the errors for the invalid signature. - // If this is a type resolution session, e.g. Language Service, try to get better information than anySignature. - // Pick the longest signature. This way we can get a contextual type for cases like: - // declare function f(a: { xa: number; xb: number; }, b: number); - // f({ | - // Also, use explicitly-supplied type arguments if they are provided, so we can get a contextual signature in cases like: - // declare function f(k: keyof T); - // f(" - if (!produceDiagnostics) { - Debug.assert(candidates.length > 0); // Else would have exited above. - const bestIndex = getLongestCandidateIndex(candidates, apparentArgumentCount === undefined ? args!.length : apparentArgumentCount); - const candidate = candidates[bestIndex]; - - const { typeParameters } = candidate; - if (typeParameters && callLikeExpressionMayHaveTypeArguments(node) && node.typeArguments) { - const typeArguments = node.typeArguments.map(getTypeOfNode) as Type[]; // TODO: GH#18217 - while (typeArguments.length > typeParameters.length) { - typeArguments.pop(); - } - while (typeArguments.length < typeParameters.length) { - typeArguments.push(getDefaultTypeArgumentType(isInJavaScriptFile(node))); - } - - const instantiated = createSignatureInstantiation(candidate, typeArguments); - candidates[bestIndex] = instantiated; - return instantiated; - } - - return candidate; - } - - return resolveErrorCall(node); + return produceDiagnostics || !args ? resolveErrorCall(node) : getCandidateForOverloadFailure(node, candidates, args, !!candidatesOutArray); function chooseOverload(candidates: Signature[], relation: Map, signatureHelpTrailingComma = false) { candidateForArgumentError = undefined; @@ -18871,6 +18844,97 @@ namespace ts { } } + // No signature was applicable. We have already reported the errors for the invalid signature. + // If this is a type resolution session, e.g. Language Service, try to get better information than anySignature. + function getCandidateForOverloadFailure( + node: CallLikeExpression, + candidates: Signature[], + args: ReadonlyArray, + hasCandidatesOutArray: boolean, + ): Signature { + Debug.assert(candidates.length > 0); // Else should not have called this. + // Normally we will combine overloads. Skip this if they have type parameters since that's hard to combine. + // Don't do this if there is a `candidatesOutArray`, + // because then we want the chosen best candidate to be one of the overloads, not a combination. + return hasCandidatesOutArray || candidates.length === 1 || candidates.some(c => !!c.typeParameters) + ? pickLongestCandidateSignature(node, candidates, args) + : createUnionOfSignaturesForOverloadFailure(candidates); + } + + function createUnionOfSignaturesForOverloadFailure(candidates: ReadonlyArray): Signature { + const thisParameters = mapDefined(candidates, c => c.thisParameter); + let thisParameter: Symbol | undefined; + if (thisParameters.length) { + thisParameter = createCombinedSymbolFromTypes(thisParameters, thisParameters.map(getTypeOfParameter)); + } + const { min: minArgumentCount, max: maxNonRestParam } = minAndMax(candidates, getNumNonRestParameters); + const parameters: Symbol[] = []; + for (let i = 0; i < maxNonRestParam; i++) { + const symbols = mapDefined(candidates, ({ parameters, hasRestParameter }) => hasRestParameter ? + i < parameters.length - 1 ? parameters[i] : last(parameters) : + i < parameters.length ? parameters[i] : undefined); + Debug.assert(symbols.length !== 0); + parameters.push(createCombinedSymbolFromTypes(symbols, mapDefined(candidates, candidate => tryGetTypeAtPosition(candidate, i)))); + } + const restParameterSymbols = mapDefined(candidates, c => c.hasRestParameter ? last(c.parameters) : undefined); + const hasRestParameter = restParameterSymbols.length !== 0; + if (hasRestParameter) { + const type = createArrayType(getUnionType(mapDefined(candidates, tryGetRestTypeOfSignature), UnionReduction.Subtype)); + parameters.push(createCombinedSymbolForOverloadFailure(restParameterSymbols, type)); + } + return createSignature( + candidates[0].declaration, + /*typeParameters*/ undefined, // Before calling this we tested for `!candidates.some(c => !!c.typeParameters)`. + thisParameter, + parameters, + /*resolvedReturnType*/ getIntersectionType(candidates.map(getReturnTypeOfSignature)), + /*typePredicate*/ undefined, + minArgumentCount, + hasRestParameter, + /*hasLiteralTypes*/ candidates.some(c => c.hasLiteralTypes)); + } + + function getNumNonRestParameters(signature: Signature): number { + const numParams = signature.parameters.length; + return signature.hasRestParameter ? numParams - 1 : numParams; + } + + function createCombinedSymbolFromTypes(sources: ReadonlyArray, types: Type[]): Symbol { + return createCombinedSymbolForOverloadFailure(sources, getUnionType(types, UnionReduction.Subtype)); + } + + function createCombinedSymbolForOverloadFailure(sources: ReadonlyArray, type: Type): Symbol { + // This function is currently only used for erroneous overloads, so it's good enough to just use the first source. + return createSymbolWithType(first(sources), type); + } + + function pickLongestCandidateSignature(node: CallLikeExpression, candidates: Signature[], args: ReadonlyArray): Signature { + // Pick the longest signature. This way we can get a contextual type for cases like: + // declare function f(a: { xa: number; xb: number; }, b: number); + // f({ | + // Also, use explicitly-supplied type arguments if they are provided, so we can get a contextual signature in cases like: + // declare function f(k: keyof T); + // f(" + const bestIndex = getLongestCandidateIndex(candidates, apparentArgumentCount === undefined ? args.length : apparentArgumentCount); + const candidate = candidates[bestIndex]; + const { typeParameters } = candidate; + if (!typeParameters) { + return candidate; + } + + const typeArgumentNodes: ReadonlyArray = callLikeExpressionMayHaveTypeArguments(node) ? node.typeArguments || emptyArray : emptyArray; + const typeArguments = typeArgumentNodes.map(n => getTypeOfNode(n) || anyType); + while (typeArguments.length > typeParameters.length) { + typeArguments.pop(); + } + while (typeArguments.length < typeParameters.length) { + typeArguments.push(getConstraintFromTypeParameter(typeParameters[typeArguments.length]) || getDefaultTypeArgumentType(isInJavaScriptFile(node))); + } + const instantiated = createSignatureInstantiation(candidate, typeArguments); + candidates[bestIndex] = instantiated; + return instantiated; + } + function getLongestCandidateIndex(candidates: Signature[], argsCount: number): number { let maxParamsIndex = -1; let maxParams = -1; @@ -19626,6 +19690,10 @@ namespace ts { } function getTypeAtPosition(signature: Signature, pos: number): Type { + return tryGetTypeAtPosition(signature, pos) || anyType; + } + + function tryGetTypeAtPosition(signature: Signature, pos: number): Type | undefined { const paramCount = signature.parameters.length - (signature.hasRestParameter ? 1 : 0); if (pos < paramCount) { return getTypeOfParameter(signature.parameters[pos]); @@ -19641,9 +19709,9 @@ namespace ts { return tupleRestType; } } - return getIndexTypeOfType(restType, IndexKind.Number) || anyType; + return getIndexTypeOfType(restType, IndexKind.Number); } - return anyType; + return undefined; } function getTypeOfRestParameter(signature: Signature) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 9c933b44d0155..e743acc0ec77e 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -8098,4 +8098,20 @@ namespace ts { return findBestPatternMatch(patterns, _ => _, candidate); } + + export function minAndMax(arr: ReadonlyArray, getValue: (value: T) => number): { readonly min: number, readonly max: number } { + Debug.assert(arr.length !== 0); + let min = getValue(arr[0]); + let max = min; + for (let i = 1; i < arr.length; i++) { + const value = getValue(arr[i]); + if (value < min) { + min = value; + } + else if (value > max) { + max = value; + } + } + return { min, max }; + } } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 89214bdb96c2c..b3d3c05b3585b 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -7399,6 +7399,10 @@ declare namespace ts { * (These are verified by verifyCompilerOptions to have 0 or 1 "*" characters.) */ function matchPatternOrExact(patternStrings: ReadonlyArray, candidate: string): string | Pattern | undefined; + function minAndMax(arr: ReadonlyArray, getValue: (value: T) => number): { + readonly min: number; + readonly max: number; + }; } declare namespace ts { function createNode(kind: SyntaxKind, pos?: number, end?: number): Node; diff --git a/tests/cases/fourslash/automaticConstructorToggling.ts b/tests/cases/fourslash/automaticConstructorToggling.ts index 647a564c52625..62015a09eb9cf 100644 --- a/tests/cases/fourslash/automaticConstructorToggling.ts +++ b/tests/cases/fourslash/automaticConstructorToggling.ts @@ -28,7 +28,7 @@ edit.deleteAtCaret('constructor(val: T) { }'.length); verify.quickInfos({ Asig: "constructor A(): A", Bsig: "constructor B(val: string): B", - Csig: "constructor C(): C", // Cannot resolve signature + Csig: "constructor C<{}>(): C<{}>", // Cannot resolve signature Dsig: "constructor D(val: string): D" // Cannot resolve signature }); @@ -37,6 +37,6 @@ edit.deleteAtCaret("val: T".length); verify.quickInfos({ Asig: "constructor A(): A", Bsig: "constructor B(val: string): B", - Csig: "constructor C(): C", // Cannot resolve signature + Csig: "constructor C<{}>(): C<{}>", // Cannot resolve signature Dsig: "constructor D(): D" }); diff --git a/tests/cases/fourslash/completionsCombineOverloads.ts b/tests/cases/fourslash/completionsCombineOverloads.ts new file mode 100644 index 0000000000000..9f0ac3eaf15c7 --- /dev/null +++ b/tests/cases/fourslash/completionsCombineOverloads.ts @@ -0,0 +1,9 @@ +/// + +////interface A { a: number } +////interface B { b: number } +////declare function f(a: A): void; +////declare function f(b: B): void; +////f({ /**/ }); + +verify.completions({ marker: "", exact: ["a", "b"] }); diff --git a/tests/cases/fourslash/completionsCombineOverloads_restParameter.ts b/tests/cases/fourslash/completionsCombineOverloads_restParameter.ts new file mode 100644 index 0000000000000..3a74c31ba7095 --- /dev/null +++ b/tests/cases/fourslash/completionsCombineOverloads_restParameter.ts @@ -0,0 +1,15 @@ +/// + +////interface A { a: number } +////interface B { b: number } +////interface C { c: number } +////declare function f(a: A): void; +////declare function f(...bs: B[]): void; +////declare function f(...cs: C[]): void; +////f({ /*1*/ }); +////f({ a: 1 }, { /*2*/ }); + +verify.completions( + { marker: "1", exact: ["a", "b", "c"] }, + { marker: "2", exact: ["b", "c"] }, +); diff --git a/tests/cases/fourslash/completionsCombineOverloads_returnType.ts b/tests/cases/fourslash/completionsCombineOverloads_returnType.ts new file mode 100644 index 0000000000000..c6f17eca5fe4b --- /dev/null +++ b/tests/cases/fourslash/completionsCombineOverloads_returnType.ts @@ -0,0 +1,9 @@ +/// + +////interface A { a: number } +////interface B { b: number } +////declare function f(n: number): A; +////declare function f(s: string): B; +////f()./**/ + +verify.completions({ marker: "", exact: ["a", "b"] }); diff --git a/tests/cases/fourslash/genericFunctionSignatureHelp3.ts b/tests/cases/fourslash/genericFunctionSignatureHelp3.ts index dafa4f42a5d73..e4a07637e9598 100644 --- a/tests/cases/fourslash/genericFunctionSignatureHelp3.ts +++ b/tests/cases/fourslash/genericFunctionSignatureHelp3.ts @@ -17,10 +17,10 @@ ////foo7(1, (/*7*/ // signature help shows y as T verify.signatureHelp( - { marker: "1", text: "foo1(x: number, callback: (y1: T) => number): void" }, + { marker: "1", text: "foo1(x: number, callback: (y1: {}) => number): void" }, // TODO: GH#23631 // { marker: "2", text: "foo2(x: number, callback: (y2: {}) => number): void" }, - { marker: "3", text: "foo3(x: number, callback: (y3: T) => number): void" }, + { marker: "3", text: "foo3(x: number, callback: (y3: {}) => number): void" }, // TODO: GH#23631 // { marker: "4", text: "foo4(x: number, callback: (y4: string) => number): void" }, { marker: "5", text: "foo5(x: number, callback: (y5: string) => number): void" }, @@ -31,4 +31,4 @@ goTo.marker('6'); // verify.signatureHelp({ text: "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 -verify.signatureHelp({ marker: "7", text: "foo7(x: number, callback: (y7: T) => number): void" }); +verify.signatureHelp({ marker: "7", text: "foo7(x: number, callback: (y7: {}) => number): void" }); diff --git a/tests/cases/fourslash/genericFunctionSignatureHelp3MultiFile.ts b/tests/cases/fourslash/genericFunctionSignatureHelp3MultiFile.ts index 4cffb1a4f417d..f84440bbd91fa 100644 --- a/tests/cases/fourslash/genericFunctionSignatureHelp3MultiFile.ts +++ b/tests/cases/fourslash/genericFunctionSignatureHelp3MultiFile.ts @@ -24,9 +24,9 @@ ////foo7(1, (/*7*/ // signature help shows y as T verify.signatureHelp( - { marker: "1", text: "foo1(x: number, callback: (y1: T) => number): void" }, - { marker: "2", text: "foo2(x: number, callback: (y2: T) => number): void" }, - { marker: "3", text: "foo3(x: number, callback: (y3: T) => number): void" }, + { marker: "1", text: "foo1(x: number, callback: (y1: {}) => number): void" }, + { marker: "2", text: "foo2(x: number, callback: (y2: {}) => number): void" }, + { marker: "3", text: "foo3(x: number, callback: (y3: {}) => number): void" }, { marker: "4", text: "foo4(x: number, callback: (y4: string) => number): void" }, { marker: "5", text: "foo5(x: number, callback: (y5: string) => number): void" }, ); @@ -35,4 +35,4 @@ goTo.marker('6'); verify.signatureHelp({ text: "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 -verify.signatureHelp({ marker: "7", text: "foo7(x: number, callback: (y7: T) => number): void" }) +verify.signatureHelp({ marker: "7", text: "foo7(x: number, callback: (y7: {}) => number): void" }) diff --git a/tests/cases/fourslash/jsDocFunctionSignatures10.ts b/tests/cases/fourslash/jsDocFunctionSignatures10.ts index 60810c46e70dc..87c8777f27a7f 100644 --- a/tests/cases/fourslash/jsDocFunctionSignatures10.ts +++ b/tests/cases/fourslash/jsDocFunctionSignatures10.ts @@ -12,4 +12,4 @@ ////fo/**/o() goTo.marker(); -verify.quickInfoIs("function foo(x: T): void", "Do some foo things"); +verify.quickInfoIs("function foo(x: any): void", "Do some foo things"); diff --git a/tests/cases/fourslash/jsdocReturnsTag.ts b/tests/cases/fourslash/jsdocReturnsTag.ts index 3f78e275e93fe..53082b2d81d90 100644 --- a/tests/cases/fourslash/jsdocReturnsTag.ts +++ b/tests/cases/fourslash/jsdocReturnsTag.ts @@ -14,7 +14,7 @@ verify.signatureHelp({ marker: "", - text: "find(l: T[], x: T): T", + text: "find(l: any[], x: any): any", docComment: "Find an item", tags: [ // TODO: GH#24130 (see PR #24600's commits for potential fix) diff --git a/tests/cases/fourslash/quickInfo_errorSignatureFillsInTypeParameter.ts b/tests/cases/fourslash/quickInfo_errorSignatureFillsInTypeParameter.ts new file mode 100644 index 0000000000000..72a81a400f32f --- /dev/null +++ b/tests/cases/fourslash/quickInfo_errorSignatureFillsInTypeParameter.ts @@ -0,0 +1,6 @@ +/// + +////declare function f(x: number): T; +////const x/**/ = f(); + +verify.quickInfoAt("", "const x: {}"); diff --git a/tests/cases/fourslash/signatureHelpExplicitTypeArguments.ts b/tests/cases/fourslash/signatureHelpExplicitTypeArguments.ts index 2e5854e20135f..2511cfecc1921 100644 --- a/tests/cases/fourslash/signatureHelpExplicitTypeArguments.ts +++ b/tests/cases/fourslash/signatureHelpExplicitTypeArguments.ts @@ -8,7 +8,7 @@ verify.signatureHelp( { marker: "1", text: "f(x: number, y: string): number" }, - { marker: "2", text: "f(x: T, y: U): T" }, + { marker: "2", text: "f(x: {}, y: {}): {}" }, // too few -- fill in rest with {} { marker: "3", text: "f(x: number, y: {}): number" }, // too many -- ignore extra type arguments diff --git a/tests/cases/fourslash/signatureHelpOnTypePredicates.ts b/tests/cases/fourslash/signatureHelpOnTypePredicates.ts index 4e7423128bad9..1eb0a31c75711 100644 --- a/tests/cases/fourslash/signatureHelpOnTypePredicates.ts +++ b/tests/cases/fourslash/signatureHelpOnTypePredicates.ts @@ -9,6 +9,6 @@ verify.signatureHelp( { marker: "1", text: "f1(a: any): a is number" }, - { marker: "2", text: "f2(a: any): a is T" }, + { marker: "2", text: "f2(a: any): a is {}" }, { marker: "3", text: "f3(a: any, ...b: any[]): a is number", isVariadic: true }, )