Fixed a regression related to determining argument index when spread elements are involved

src/services/completions.ts

@@ -3161,8 +3161,7 @@ function getContextualType(previousToken: Node, position: number, sourceFile: So
         default:
             const argInfo = SignatureHelp.getArgumentInfoForCompletions(previousToken, position, sourceFile, checker);
             return argInfo ?
-                // At `,`, treat this as the next argument after the comma.
-                checker.getContextualTypeForArgumentAtIndex(argInfo.invocation, argInfo.argumentIndex + (previousToken.kind === SyntaxKind.CommaToken ? 1 : 0)) :
+                checker.getContextualTypeForArgumentAtIndex(argInfo.invocation, argInfo.argumentIndex) :
                 isEqualityOperatorKind(previousToken.kind) && isBinaryExpression(parent) && isEqualityOperatorKind(parent.operatorToken.kind) ?
                 // completion at `x ===/**/` should be for the right side
                 checker.getTypeAtLocation(parent.left) :

src/services/signatureHelp.ts

@@ -6,7 +6,6 @@ import {
     canHaveSymbol,
     CheckFlags,
     contains,
-    countWhere,
     createPrinterWithRemoveComments,
     createTextSpan,
     createTextSpanFromBounds,
@@ -60,7 +59,6 @@ import {
     JsxTagNameExpression,
     last,
     lastOrUndefined,
-    length,
     ListFormat,
     map,
     mapToDisplayParts,
@@ -288,7 +286,7 @@ function getArgumentOrParameterListInfo(node: Node, position: number, sourceFile
     if (!info) return undefined;
     const { list, argumentIndex } = info;
 
-    const argumentCount = getArgumentCount(list, /*ignoreTrailingComma*/ isInString(sourceFile, position, node), checker);
+    const argumentCount = getArgumentCount(checker, list);
     if (argumentIndex !== 0) {
         Debug.assertLessThan(argumentIndex, argumentCount);
     }
@@ -309,7 +307,7 @@ function getArgumentOrParameterListAndIndex(node: Node, sourceFile: SourceFile,
         //   - On the target of the call (parent.func)
         //   - On the 'new' keyword in a 'new' expression
         const list = findContainingList(node);
-        return list && { list, argumentIndex: getArgumentIndex(list, node, checker) };
+        return list && { list, argumentIndex: getArgumentIndex(checker, list, node) };
     }
 }
 
@@ -481,37 +479,6 @@ function chooseBetterSymbol(s: Symbol): Symbol {
         : s;
 }
 
-function getArgumentIndex(argumentsList: Node, node: Node, checker: TypeChecker) {
-    // The list we got back can include commas.  In the presence of errors it may
-    // also just have nodes without commas.  For example "Foo(a b c)" will have 3
-    // args without commas. We want to find what index we're at.  So we count
-    // forward until we hit ourselves, only incrementing the index if it isn't a
-    // comma.
-    //
-    // Note: the subtlety around trailing commas (in getArgumentCount) does not apply
-    // here.  That's because we're only walking forward until we hit the node we're
-    // on.  In that case, even if we're after the trailing comma, we'll still see
-    // that trailing comma in the list, and we'll have generated the appropriate
-    // arg index.
-    const args = argumentsList.getChildren();
-    let argumentIndex = 0;
-    for (let pos = 0; pos < length(args); pos++) {
-        const child = args[pos];
-        if (child === node) {
-            break;
-        }
-        if (isSpreadElement(child)) {
-            argumentIndex = argumentIndex + getSpreadElementCount(child, checker) + (pos > 0 ? pos : 0);
-        }
-        else {
-            if (child.kind !== SyntaxKind.CommaToken) {
-                argumentIndex++;
-            }
-        }
-    }
-    return argumentIndex;
-}
-
 function getSpreadElementCount(node: SpreadElement, checker: TypeChecker) {
     const spreadType = checker.getTypeAtLocation(node.expression);
     if (checker.isTupleType(spreadType)) {
@@ -525,32 +492,54 @@ function getSpreadElementCount(node: SpreadElement, checker: TypeChecker) {
     return 0;
 }
 
-function getArgumentCount(argumentsList: Node, ignoreTrailingComma: boolean, checker: TypeChecker) {
-    // The argument count for a list is normally the number of non-comma children it has.
-    // For example, if you have "Foo(a,b)" then there will be three children of the arg
-    // list 'a' '<comma>' 'b'.  So, in this case the arg count will be 2.  However, there
-    // is a small subtlety.  If you have "Foo(a,)", then the child list will just have
-    // 'a' '<comma>'.  So, in the case where the last child is a comma, we increase the
-    // arg count by one to compensate.
-    //
-    // Note: this subtlety only applies to the last comma.  If you had "Foo(a,," then
-    // we'll have: 'a' '<comma>' '<missing>'
-    // That will give us 2 non-commas.  We then add one for the last comma, giving us an
-    // arg count of 3.
-    const listChildren = argumentsList.getChildren();
-
-    let argumentCount = 0;
-    for (const child of listChildren) {
+function getArgumentIndex(checker: TypeChecker, argumentsList: Node, node: Node) {
+    return getArgumentIndexOrCount(checker, argumentsList, node);
+}
+
+function getArgumentCount(checker: TypeChecker, argumentsList: Node) {
+    return getArgumentIndexOrCount(checker, argumentsList, /*node*/ undefined);
+}
+
+function getArgumentIndexOrCount(checker: TypeChecker, argumentsList: Node, node: Node | undefined) {
+    // The list we got back can include commas. In the presence of errors it may
+    // also just have nodes without commas. For example "Foo(a b c)" will have 3
+    // args without commas.
+    const args = argumentsList.getChildren();
+    let argumentIndex = 0;
+    let skipComma = false;
+    for (const child of args) {
+        if (node && child === node) {
+            if (!skipComma && child.kind === SyntaxKind.CommaToken) {
+                argumentIndex++;
+            }
+            return argumentIndex;
+        }
         if (isSpreadElement(child)) {
-            argumentCount = argumentCount + getSpreadElementCount(child, checker);
+            argumentIndex += getSpreadElementCount(child, checker);
+            skipComma = true;
+            continue;
+        }
+        if (child.kind !== SyntaxKind.CommaToken) {
+            argumentIndex++;
+            skipComma = true;
+            continue;
+        }
+        if (skipComma) {
+            skipComma = false;
+            continue;
         }
+        argumentIndex++;
     }
-
-    argumentCount = argumentCount + countWhere(listChildren, arg => arg.kind !== SyntaxKind.CommaToken);
-    if (!ignoreTrailingComma && listChildren.length > 0 && last(listChildren).kind === SyntaxKind.CommaToken) {
-        argumentCount++;
+    if (node) {
+        return argumentIndex;
     }
-    return argumentCount;
+    // The argument count for a list is normally the number of non-comma children it has.
+    // For example, if you have "Foo(a,b)" then there will be three children of the arg
+    // list 'a' '<comma>' 'b'. So, in this case the arg count will be 2. However, there
+    // is a small subtlety. If you have "Foo(a,)", then the child list will just have
+    // 'a' '<comma>'. So, in the case where the last child is a comma, we increase the
+    // arg count by one to compensate.
+    return args.length && last(args).kind === SyntaxKind.CommaToken ? argumentIndex + 1 : argumentIndex;
 }
 
 // spanIndex is either the index for a given template span.

tests/baselines/reference/signatureHelpRestArgs.baseline → tests/baselines/reference/signatureHelpRestArgs1.baseline

@@ -1,5 +1,5 @@
 // === SignatureHelp ===
-=== /tests/cases/fourslash/signatureHelpRestArgs.ts ===
+=== /tests/cases/fourslash/signatureHelpRestArgs1.ts ===
 // function fn(a: number, b: number, c: number) {}
 // const a = [1, 2] as const;
 // const b = [1] as const;
@@ -33,7 +33,7 @@
 [
   {
     "marker": {
-      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs.ts",
+      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs1.ts",
       "position": 109,
       "name": "1"
     },
@@ -163,12 +163,12 @@
       },
       "selectedItemIndex": 0,
       "argumentIndex": 2,
-      "argumentCount": 4
+      "argumentCount": 3
     }
   },
   {
     "marker": {
-      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs.ts",
+      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs1.ts",
       "position": 115,
       "name": "2"
     },
@@ -303,7 +303,7 @@
   },
   {
     "marker": {
-      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs.ts",
+      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs1.ts",
       "position": 134,
       "name": "3"
     },
@@ -433,12 +433,12 @@
       },
       "selectedItemIndex": 0,
       "argumentIndex": 1,
-      "argumentCount": 3
+      "argumentCount": 2
     }
   },
   {
     "marker": {
-      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs.ts",
+      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs1.ts",
       "position": 140,
       "name": "4"
     },
@@ -573,7 +573,7 @@
   },
   {
     "marker": {
-      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs.ts",
+      "fileName": "/tests/cases/fourslash/signatureHelpRestArgs1.ts",
       "position": 148,
       "name": "5"
     },