From b2bca72bb65cbfcc360020753875c1b8a34c9ae4 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Tue, 30 Sep 2014 17:13:27 -0700 Subject: [PATCH] Fix argumentCount and selectedItemIndex --- src/harness/fourslash.ts | 19 ++++---- src/services/signatureHelp.ts | 46 ++++++++++++++++--- tests/cases/fourslash/fourslash.ts | 4 ++ ...signatureHelpOnOverloadsDifferentArity3.ts | 6 +-- 4 files changed, 55 insertions(+), 20 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 64d87f45d1b..d33b5c0715d 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -891,6 +891,13 @@ module FourSlash { assert.equal(actual, expected); } + public verifySignatureHelpArgumentCount(expected: number) { + this.taoInvalidReason = 'verifySignatureHelpArgumentCount NYI'; + var signatureHelpItems = this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition); + var actual = signatureHelpItems.argumentCount; + assert.equal(actual, expected); + } + public verifySignatureHelpPresent(shouldBePresent = true) { this.taoInvalidReason = 'verifySignatureHelpPresent NYI'; @@ -946,22 +953,14 @@ module FourSlash { private getActiveSignatureHelpItem() { var help = this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition); - - // If the signature hasn't been narrowed down yet (e.g. no parameters have yet been entered), - // 'activeFormal' will be -1 (even if there is only 1 signature). Signature help will show the - // first signature in the signature group, so go with that - var index = help.selectedItemIndex < 0 ? 0 : help.selectedItemIndex; - + var index = help.selectedItemIndex; return help.items[index]; } private getActiveParameter(): ts.SignatureHelpParameter { var help = this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition); - var item = help.items[help.selectedItemIndex]; - - // Same logic as in getActiveSignatureHelp - this value might be -1 until a parameter value actually gets typed - var currentParam = help.argumentIndex < 0 ? 0 : help.argumentIndex; + var currentParam = help.argumentIndex; return item.parameters[currentParam]; } diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 3af419b3f90..080e4704b83 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -252,6 +252,33 @@ module ts.SignatureHelp { return undefined; } + /** + * The selectedItemIndex could be negative for several reasons. + * 1. There are too many arguments for all of the overloads + * 2. None of the overloads were type compatible + * The solution here is to try to pick the best overload by picking + * either the first one that has an appropriate number of parameters, + * or the one with the most parameters. + */ + function selectBestInvalidOverloadIndex(candidates: Signature[], argumentCount: number): number { + var maxParamsSignatureIndex = -1; + var maxParams = -1; + for (var i = 0; i < candidates.length; i++) { + var candidate = candidates[i]; + + if (candidate.hasRestParameter || candidate.parameters.length >= argumentCount) { + return i; + } + + if (candidate.parameters.length > maxParams) { + maxParams = candidate.parameters.length; + maxParamsSignatureIndex = i; + } + } + + return maxParamsSignatureIndex; + } + function createSignatureHelpItems(candidates: Signature[], bestSignature: Signature, argumentInfoOrTypeArgumentInfo: ListItemInfo): SignatureHelpItems { var argumentListOrTypeArgumentList = argumentInfoOrTypeArgumentInfo.list; var items: SignatureHelpItem[] = map(candidates, candidateSignature => { @@ -326,11 +353,6 @@ module ts.SignatureHelp { }; }); - var selectedItemIndex = candidates.indexOf(bestSignature); - if (selectedItemIndex < 0) { - selectedItemIndex = 0; - } - // We use full start and skip trivia on the end because we want to include trivia on // both sides. For example, // @@ -353,8 +375,18 @@ module ts.SignatureHelp { // But if we are on a comma, we also want to pretend we are on the argument *following* // the comma. That amounts to taking the ceiling of half the index. var argumentIndex = (argumentInfoOrTypeArgumentInfo.listItemIndex + 1) >> 1; - var numberOfCommas = countWhere(argumentListOrTypeArgumentList.getChildren(), arg => arg.kind === SyntaxKind.CommaToken); - var argumentCount = numberOfCommas + 1; + + // argumentCount is the number of commas plus one, unless the list is completely empty, + // in which case there are 0. + var argumentCount = argumentListOrTypeArgumentList.getChildCount() === 0 + ? 0 + : 1 + countWhere(argumentListOrTypeArgumentList.getChildren(), arg => arg.kind === SyntaxKind.CommaToken); + + var selectedItemIndex = candidates.indexOf(bestSignature); + if (selectedItemIndex < 0) { + selectedItemIndex = selectBestInvalidOverloadIndex(candidates, argumentCount); + } + return { items: items, applicableSpan: applicableSpan, diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 35b6894b1ce..c204a98c4a4 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -294,6 +294,10 @@ module FourSlashInterface { FourSlash.currentTestState.verifySignatureHelpCount(expected); } + public signatureHelpArgumentCountIs(expected: number) { + FourSlash.currentTestState.verifySignatureHelpArgumentCount(expected); + } + public currentSignatureParamterCountIs(expected: number) { FourSlash.currentTestState.verifyCurrentSignatureHelpParameterCount(expected); } diff --git a/tests/cases/fourslash/signatureHelpOnOverloadsDifferentArity3.ts b/tests/cases/fourslash/signatureHelpOnOverloadsDifferentArity3.ts index 4d2db0922ee..c217a12ef58 100644 --- a/tests/cases/fourslash/signatureHelpOnOverloadsDifferentArity3.ts +++ b/tests/cases/fourslash/signatureHelpOnOverloadsDifferentArity3.ts @@ -11,6 +11,7 @@ goTo.marker(); verify.signatureHelpCountIs(4); verify.currentSignatureHelpIs("f(): any"); verify.currentSignatureParamterCountIs(0); +verify.signatureHelpArgumentCountIs(0); edit.insert(", "); verify.signatureHelpCountIs(4); @@ -19,8 +20,7 @@ verify.currentSignatureParamterCountIs(2); verify.currentParameterHelpArgumentNameIs("b"); verify.currentParameterSpanIs("b: boolean"); -// What should the intended behavior be if there are too many arguments? edit.insert(", "); verify.signatureHelpCountIs(4); -verify.currentSignatureHelpIs("f(): any"); -verify.currentSignatureParamterCountIs(0); \ No newline at end of file +verify.currentSignatureHelpIs("f(s: string, b: boolean): any"); +verify.currentSignatureParamterCountIs(2); \ No newline at end of file