From 53eac1586c6f62127daf91b56c80df41e57628e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Mon, 4 Mar 2024 12:03:44 +0100 Subject: [PATCH 1/5] Fixed a regression related to determining argument index when spread elements are involved --- src/services/completions.ts | 3 +- src/services/signatureHelp.ts | 60 +- ...seline => signatureHelpRestArgs1.baseline} | 16 +- .../reference/signatureHelpRestArgs2.baseline | 201 +++++ .../signatureHelpSkippedArgs1.baseline | 702 ++++++++++++++++++ ...pRestArgs.ts => signatureHelpRestArgs1.ts} | 0 .../cases/fourslash/signatureHelpRestArgs2.ts | 18 + .../fourslash/signatureHelpSkippedArgs1.ts | 6 + 8 files changed, 972 insertions(+), 34 deletions(-) rename tests/baselines/reference/{signatureHelpRestArgs.baseline => signatureHelpRestArgs1.baseline} (97%) create mode 100644 tests/baselines/reference/signatureHelpRestArgs2.baseline create mode 100644 tests/baselines/reference/signatureHelpSkippedArgs1.baseline rename tests/cases/fourslash/{signatureHelpRestArgs.ts => signatureHelpRestArgs1.ts} (100%) create mode 100644 tests/cases/fourslash/signatureHelpRestArgs2.ts create mode 100644 tests/cases/fourslash/signatureHelpSkippedArgs1.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index 0e4075150830e..353816d58f9f1 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -3126,8 +3126,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) : diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 2caf2aaa8530c..535273034e70f 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -6,7 +6,6 @@ import { canHaveSymbol, CheckFlags, contains, - countWhere, createPrinterWithRemoveComments, createTextSpan, createTextSpanFromBounds, @@ -288,7 +287,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(list, checker); if (argumentIndex !== 0) { Debug.assertLessThan(argumentIndex, argumentCount); } @@ -485,8 +484,7 @@ 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. + // forward until we hit ourselves. // // 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 @@ -495,19 +493,30 @@ function getArgumentIndex(argumentsList: Node, node: Node, checker: TypeChecker) // arg index. const args = argumentsList.getChildren(); let argumentIndex = 0; + let skipComma = false for (let pos = 0; pos < length(args); pos++) { const child = args[pos]; if (child === node) { + if (!skipComma && child.kind === SyntaxKind.CommaToken) { + argumentIndex++; + } break; } if (isSpreadElement(child)) { - argumentIndex = argumentIndex + getSpreadElementCount(child, checker) + (pos > 0 ? pos : 0); + argumentIndex += getSpreadElementCount(child, checker); + skipComma = true; + continue } - else { - if (child.kind !== SyntaxKind.CommaToken) { - argumentIndex++; - } + if (child.kind !== SyntaxKind.CommaToken) { + argumentIndex++; + skipComma = true; + continue; + } + if (skipComma) { + skipComma = false; + continue; } + argumentIndex++; } return argumentIndex; } @@ -525,32 +534,35 @@ function getSpreadElementCount(node: SpreadElement, checker: TypeChecker) { return 0; } -function getArgumentCount(argumentsList: Node, ignoreTrailingComma: boolean, checker: TypeChecker) { +function getArgumentCount(argumentsList: Node, 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' '' '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' ''. 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' '' '' - // 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(); - + const args = argumentsList.getChildren(); let argumentCount = 0; - for (const child of listChildren) { + let skipComma = false + for (let pos = 0; pos < length(args); pos++) { + const child = args[pos]; if (isSpreadElement(child)) { - argumentCount = argumentCount + getSpreadElementCount(child, checker); + argumentCount += getSpreadElementCount(child, checker); + skipComma = true; + continue + } + if (child.kind !== SyntaxKind.CommaToken) { + argumentCount++; + skipComma = true; + continue; + } + if (skipComma) { + skipComma = false; + continue; } - } - - argumentCount = argumentCount + countWhere(listChildren, arg => arg.kind !== SyntaxKind.CommaToken); - if (!ignoreTrailingComma && listChildren.length > 0 && last(listChildren).kind === SyntaxKind.CommaToken) { argumentCount++; } - return argumentCount; + return args.length && last(args).kind === SyntaxKind.CommaToken ? argumentCount + 1 : argumentCount; } // spanIndex is either the index for a given template span. diff --git a/tests/baselines/reference/signatureHelpRestArgs.baseline b/tests/baselines/reference/signatureHelpRestArgs1.baseline similarity index 97% rename from tests/baselines/reference/signatureHelpRestArgs.baseline rename to tests/baselines/reference/signatureHelpRestArgs1.baseline index 9242555b3adcf..7514e1c5d29e7 100644 --- a/tests/baselines/reference/signatureHelpRestArgs.baseline +++ b/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" }, diff --git a/tests/baselines/reference/signatureHelpRestArgs2.baseline b/tests/baselines/reference/signatureHelpRestArgs2.baseline new file mode 100644 index 0000000000000..6fd5b762b558b --- /dev/null +++ b/tests/baselines/reference/signatureHelpRestArgs2.baseline @@ -0,0 +1,201 @@ +// === SignatureHelp === +=== /tests/cases/fourslash/index.js === +// const promisify = function (thisArg, fnName) { +// const fn = thisArg[fnName]; +// return function () { +// return new Promise((resolve) => { +// fn.call(thisArg, ...arguments, ); +// ^ +// | ---------------------------------------------------------------------- +// | Function.call(thisArg: any, **...argArray: any[]**): any +// | Calls a method of an object, substituting another object for the current object. +// | @param thisArg The object to be used as the current object. +// | @param argArray A list of arguments to be passed to the method. +// | ---------------------------------------------------------------------- +// }); +// }; +// }; + +[ + { + "marker": { + "fileName": "/tests/cases/fourslash/index.js", + "position": 189, + "name": "1" + }, + "item": { + "items": [ + { + "isVariadic": true, + "prefixDisplayParts": [ + { + "text": "Function", + "kind": "localName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "call", + "kind": "methodName" + }, + { + "text": "(", + "kind": "punctuation" + } + ], + "suffixDisplayParts": [ + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + } + ], + "separatorDisplayParts": [ + { + "text": ",", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + } + ], + "parameters": [ + { + "name": "thisArg", + "documentation": [ + { + "text": "The object to be used as the current object.", + "kind": "text" + } + ], + "displayParts": [ + { + "text": "thisArg", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "argArray", + "documentation": [ + { + "text": "A list of arguments to be passed to the method.", + "kind": "text" + } + ], + "displayParts": [ + { + "text": "...", + "kind": "punctuation" + }, + { + "text": "argArray", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + }, + { + "text": "[", + "kind": "punctuation" + }, + { + "text": "]", + "kind": "punctuation" + } + ], + "isOptional": false, + "isRest": false + } + ], + "documentation": [ + { + "text": "Calls a method of an object, substituting another object for the current object.", + "kind": "text" + } + ], + "tags": [ + { + "name": "param", + "text": [ + { + "text": "thisArg", + "kind": "parameterName" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "The object to be used as the current object.", + "kind": "text" + } + ] + }, + { + "name": "param", + "text": [ + { + "text": "argArray", + "kind": "parameterName" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "A list of arguments to be passed to the method.", + "kind": "text" + } + ] + } + ] + } + ], + "applicableSpan": { + "start": 166, + "length": 23 + }, + "selectedItemIndex": 0, + "argumentIndex": 1, + "argumentCount": 2 + } + } +] \ No newline at end of file diff --git a/tests/baselines/reference/signatureHelpSkippedArgs1.baseline b/tests/baselines/reference/signatureHelpSkippedArgs1.baseline new file mode 100644 index 0000000000000..4b2cc1546ef6c --- /dev/null +++ b/tests/baselines/reference/signatureHelpSkippedArgs1.baseline @@ -0,0 +1,702 @@ +// === SignatureHelp === +=== /tests/cases/fourslash/signatureHelpSkippedArgs1.ts === +// function fn(a: number, b: number, c: number) {} +// fn(, , , , ); +// ^ +// | ---------------------------------------------------------------------- +// | fn(**a: number**, b: number, c: number): void +// | ---------------------------------------------------------------------- +// ^ +// | ---------------------------------------------------------------------- +// | fn(a: number, **b: number**, c: number): void +// | ---------------------------------------------------------------------- +// ^ +// | ---------------------------------------------------------------------- +// | fn(a: number, b: number, **c: number**): void +// | ---------------------------------------------------------------------- +// ^ +// | ---------------------------------------------------------------------- +// | fn(a: number, b: number, c: number): void +// | ---------------------------------------------------------------------- +// ^ +// | ---------------------------------------------------------------------- +// | fn(a: number, b: number, c: number): void +// | ---------------------------------------------------------------------- + +[ + { + "marker": { + "fileName": "/tests/cases/fourslash/signatureHelpSkippedArgs1.ts", + "position": 51, + "name": "1" + }, + "item": { + "items": [ + { + "isVariadic": false, + "prefixDisplayParts": [ + { + "text": "fn", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + } + ], + "suffixDisplayParts": [ + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "void", + "kind": "keyword" + } + ], + "separatorDisplayParts": [ + { + "text": ",", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + } + ], + "parameters": [ + { + "name": "a", + "documentation": [], + "displayParts": [ + { + "text": "a", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "b", + "documentation": [], + "displayParts": [ + { + "text": "b", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "c", + "documentation": [], + "displayParts": [ + { + "text": "c", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + } + ], + "documentation": [], + "tags": [] + } + ], + "applicableSpan": { + "start": 51, + "length": 8 + }, + "selectedItemIndex": 0, + "argumentIndex": 0, + "argumentCount": 5 + } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/signatureHelpSkippedArgs1.ts", + "position": 53, + "name": "2" + }, + "item": { + "items": [ + { + "isVariadic": false, + "prefixDisplayParts": [ + { + "text": "fn", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + } + ], + "suffixDisplayParts": [ + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "void", + "kind": "keyword" + } + ], + "separatorDisplayParts": [ + { + "text": ",", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + } + ], + "parameters": [ + { + "name": "a", + "documentation": [], + "displayParts": [ + { + "text": "a", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "b", + "documentation": [], + "displayParts": [ + { + "text": "b", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "c", + "documentation": [], + "displayParts": [ + { + "text": "c", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + } + ], + "documentation": [], + "tags": [] + } + ], + "applicableSpan": { + "start": 51, + "length": 8 + }, + "selectedItemIndex": 0, + "argumentIndex": 1, + "argumentCount": 5 + } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/signatureHelpSkippedArgs1.ts", + "position": 55, + "name": "3" + }, + "item": { + "items": [ + { + "isVariadic": false, + "prefixDisplayParts": [ + { + "text": "fn", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + } + ], + "suffixDisplayParts": [ + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "void", + "kind": "keyword" + } + ], + "separatorDisplayParts": [ + { + "text": ",", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + } + ], + "parameters": [ + { + "name": "a", + "documentation": [], + "displayParts": [ + { + "text": "a", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "b", + "documentation": [], + "displayParts": [ + { + "text": "b", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "c", + "documentation": [], + "displayParts": [ + { + "text": "c", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + } + ], + "documentation": [], + "tags": [] + } + ], + "applicableSpan": { + "start": 51, + "length": 8 + }, + "selectedItemIndex": 0, + "argumentIndex": 2, + "argumentCount": 5 + } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/signatureHelpSkippedArgs1.ts", + "position": 57, + "name": "4" + }, + "item": { + "items": [ + { + "isVariadic": false, + "prefixDisplayParts": [ + { + "text": "fn", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + } + ], + "suffixDisplayParts": [ + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "void", + "kind": "keyword" + } + ], + "separatorDisplayParts": [ + { + "text": ",", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + } + ], + "parameters": [ + { + "name": "a", + "documentation": [], + "displayParts": [ + { + "text": "a", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "b", + "documentation": [], + "displayParts": [ + { + "text": "b", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "c", + "documentation": [], + "displayParts": [ + { + "text": "c", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + } + ], + "documentation": [], + "tags": [] + } + ], + "applicableSpan": { + "start": 51, + "length": 8 + }, + "selectedItemIndex": 0, + "argumentIndex": 3, + "argumentCount": 5 + } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/signatureHelpSkippedArgs1.ts", + "position": 59, + "name": "5" + }, + "item": { + "items": [ + { + "isVariadic": false, + "prefixDisplayParts": [ + { + "text": "fn", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + } + ], + "suffixDisplayParts": [ + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "void", + "kind": "keyword" + } + ], + "separatorDisplayParts": [ + { + "text": ",", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + } + ], + "parameters": [ + { + "name": "a", + "documentation": [], + "displayParts": [ + { + "text": "a", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "b", + "documentation": [], + "displayParts": [ + { + "text": "b", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "c", + "documentation": [], + "displayParts": [ + { + "text": "c", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "number", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + } + ], + "documentation": [], + "tags": [] + } + ], + "applicableSpan": { + "start": 51, + "length": 8 + }, + "selectedItemIndex": 0, + "argumentIndex": 4, + "argumentCount": 5 + } + } +] \ No newline at end of file diff --git a/tests/cases/fourslash/signatureHelpRestArgs.ts b/tests/cases/fourslash/signatureHelpRestArgs1.ts similarity index 100% rename from tests/cases/fourslash/signatureHelpRestArgs.ts rename to tests/cases/fourslash/signatureHelpRestArgs1.ts diff --git a/tests/cases/fourslash/signatureHelpRestArgs2.ts b/tests/cases/fourslash/signatureHelpRestArgs2.ts new file mode 100644 index 0000000000000..983e93de6d68f --- /dev/null +++ b/tests/cases/fourslash/signatureHelpRestArgs2.ts @@ -0,0 +1,18 @@ +/// + +// @strict: true +// @allowJs: true +// @checkJs: true + +// @filename: index.js + +//// const promisify = function (thisArg, fnName) { +//// const fn = thisArg[fnName]; +//// return function () { +//// return new Promise((resolve) => { +//// fn.call(thisArg, ...arguments, /*1*/); +//// }); +//// }; +//// }; + +verify.baselineSignatureHelp(); diff --git a/tests/cases/fourslash/signatureHelpSkippedArgs1.ts b/tests/cases/fourslash/signatureHelpSkippedArgs1.ts new file mode 100644 index 0000000000000..543991c31940e --- /dev/null +++ b/tests/cases/fourslash/signatureHelpSkippedArgs1.ts @@ -0,0 +1,6 @@ +/// + +//// function fn(a: number, b: number, c: number) {} +//// fn(/*1*/, /*2*/, /*3*/, /*4*/, /*5*/); + +verify.baselineSignatureHelp(); From aab5c0df3da0a0ffee6ef4b9990c4a648e2b5187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Mon, 4 Mar 2024 21:15:25 +0100 Subject: [PATCH 2/5] format --- src/services/signatureHelp.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 535273034e70f..4ec3f14352dba 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -493,7 +493,7 @@ function getArgumentIndex(argumentsList: Node, node: Node, checker: TypeChecker) // arg index. const args = argumentsList.getChildren(); let argumentIndex = 0; - let skipComma = false + let skipComma = false; for (let pos = 0; pos < length(args); pos++) { const child = args[pos]; if (child === node) { @@ -505,7 +505,7 @@ function getArgumentIndex(argumentsList: Node, node: Node, checker: TypeChecker) if (isSpreadElement(child)) { argumentIndex += getSpreadElementCount(child, checker); skipComma = true; - continue + continue; } if (child.kind !== SyntaxKind.CommaToken) { argumentIndex++; @@ -543,13 +543,13 @@ function getArgumentCount(argumentsList: Node, checker: TypeChecker) { // arg count by one to compensate. const args = argumentsList.getChildren(); let argumentCount = 0; - let skipComma = false + let skipComma = false; for (let pos = 0; pos < length(args); pos++) { const child = args[pos]; if (isSpreadElement(child)) { argumentCount += getSpreadElementCount(child, checker); skipComma = true; - continue + continue; } if (child.kind !== SyntaxKind.CommaToken) { argumentCount++; From 5f8e455e5172da42fe3fac6a4ed362a22d4baabf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 21 Mar 2024 08:07:53 +0100 Subject: [PATCH 3/5] use for-of --- src/services/signatureHelp.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 4ec3f14352dba..619bd1ce6f137 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -59,7 +59,6 @@ import { JsxTagNameExpression, last, lastOrUndefined, - length, ListFormat, map, mapToDisplayParts, @@ -494,8 +493,7 @@ function getArgumentIndex(argumentsList: Node, node: Node, checker: TypeChecker) const args = argumentsList.getChildren(); let argumentIndex = 0; let skipComma = false; - for (let pos = 0; pos < length(args); pos++) { - const child = args[pos]; + for (const child of args) { if (child === node) { if (!skipComma && child.kind === SyntaxKind.CommaToken) { argumentIndex++; @@ -544,8 +542,7 @@ function getArgumentCount(argumentsList: Node, checker: TypeChecker) { const args = argumentsList.getChildren(); let argumentCount = 0; let skipComma = false; - for (let pos = 0; pos < length(args); pos++) { - const child = args[pos]; + for (const child of args) { if (isSpreadElement(child)) { argumentCount += getSpreadElementCount(child, checker); skipComma = true; From 36cfea0bef1138ce6fe8fd3218619d1accba7bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 28 Mar 2024 09:50:10 +0100 Subject: [PATCH 4/5] unify both functions --- src/services/signatureHelp.ts | 80 ++++++++++++----------------------- 1 file changed, 26 insertions(+), 54 deletions(-) diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 619bd1ce6f137..e9cfe72651395 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -286,7 +286,7 @@ function getArgumentOrParameterListInfo(node: Node, position: number, sourceFile if (!info) return undefined; const { list, argumentIndex } = info; - const argumentCount = getArgumentCount(list, checker); + const argumentCount = getArgumentIndexOrCount(checker, list, /*node*/ undefined); if (argumentIndex !== 0) { Debug.assertLessThan(argumentIndex, argumentCount); } @@ -307,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: getArgumentIndexOrCount(checker, list, node) }; } } @@ -479,46 +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. - // - // 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; - let skipComma = false; - for (const child of args) { - if (child === node) { - if (!skipComma && child.kind === SyntaxKind.CommaToken) { - argumentIndex++; - } - break; - } - if (isSpreadElement(child)) { - argumentIndex += getSpreadElementCount(child, checker); - skipComma = true; - continue; - } - if (child.kind !== SyntaxKind.CommaToken) { - argumentIndex++; - skipComma = true; - continue; - } - if (skipComma) { - skipComma = false; - continue; - } - argumentIndex++; - } - return argumentIndex; -} - function getSpreadElementCount(node: SpreadElement, checker: TypeChecker) { const spreadType = checker.getTypeAtLocation(node.expression); if (checker.isTupleType(spreadType)) { @@ -532,24 +492,27 @@ function getSpreadElementCount(node: SpreadElement, checker: TypeChecker) { return 0; } -function getArgumentCount(argumentsList: Node, 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' '' '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' ''. So, in the case where the last child is a comma, we increase the - // arg count by one to compensate. +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 argumentCount = 0; + 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 += getSpreadElementCount(child, checker); + argumentIndex += getSpreadElementCount(child, checker); skipComma = true; continue; } if (child.kind !== SyntaxKind.CommaToken) { - argumentCount++; + argumentIndex++; skipComma = true; continue; } @@ -557,9 +520,18 @@ function getArgumentCount(argumentsList: Node, checker: TypeChecker) { skipComma = false; continue; } - argumentCount++; + argumentIndex++; + } + if (node) { + return argumentIndex; } - return args.length && last(args).kind === SyntaxKind.CommaToken ? argumentCount + 1 : 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' '' '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' ''. 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. From e7bd4fd33f6d016f7eeb8e66050e7c138a59fc74 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 28 Mar 2024 10:39:08 -0700 Subject: [PATCH 5/5] Pull functions back out to reduce confusion about how to call --- src/services/signatureHelp.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index e9cfe72651395..28f0d188854bb 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -286,7 +286,7 @@ function getArgumentOrParameterListInfo(node: Node, position: number, sourceFile if (!info) return undefined; const { list, argumentIndex } = info; - const argumentCount = getArgumentIndexOrCount(checker, list, /*node*/ undefined); + const argumentCount = getArgumentCount(checker, list); if (argumentIndex !== 0) { Debug.assertLessThan(argumentIndex, argumentCount); } @@ -307,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: getArgumentIndexOrCount(checker, list, node) }; + return list && { list, argumentIndex: getArgumentIndex(checker, list, node) }; } } @@ -492,6 +492,14 @@ function getSpreadElementCount(node: SpreadElement, checker: TypeChecker) { return 0; } +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