Fix argumentCount and selectedItemIndex

Author: JsonFreeman

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];
         }
 

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,

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);
         }

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);
+verify.currentSignatureHelpIs("f(s: string, b: boolean): any");
+verify.currentSignatureParamterCountIs(2);