diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 6f4f38aa73610..52a7b41d8a7a6 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -238,7 +238,7 @@ module ts.SignatureHelp { invocation: callExpression, argumentsSpan: getApplicableSpanForArguments(list), argumentIndex: 0, - argumentCount: getCommaBasedArgCount(list) + argumentCount: getArgumentCount(list) }; } @@ -253,20 +253,11 @@ module ts.SignatureHelp { var list = listItemInfo.list; var isTypeArgList = callExpression.typeArguments && callExpression.typeArguments.pos === list.pos; - // The listItemIndex we got back includes commas. Our goal is to return the index of the proper - // item (not including commas). Here are some examples: - // 1. foo(a, b, c #) -> the listItemIndex is 4, we want to return 2 - // 2. foo(a, b, # c) -> listItemIndex is 3, we want to return 2 - // 3. foo(#a) -> listItemIndex is 0, we want to return 0 - // - // In general, we want to subtract the number of commas before the current index. - // 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 = (listItemInfo.listItemIndex + 1) >> 1; + var argumentIndex = getArgumentIndex(list, node); + var argumentCount = getArgumentCount(list); - var argumentCount = getCommaBasedArgCount(list); - - Debug.assert(argumentIndex === 0 || argumentIndex < argumentCount, `argumentCount < argumentIndex, ${argumentCount} < ${argumentIndex}`); + Debug.assert(argumentIndex === 0 || argumentIndex < argumentCount, + `argumentCount < argumentIndex, ${argumentCount} < ${argumentIndex}`); return { kind: isTypeArgList ? ArgumentListKind.TypeArguments : ArgumentListKind.CallArguments, @@ -313,12 +304,53 @@ module ts.SignatureHelp { return undefined; } - function getCommaBasedArgCount(argumentsList: Node) { - // The number of arguments is the number of commas plus one, unless the list - // is completely empty, in which case there are 0 arguments. - return argumentsList.getChildCount() === 0 - ? 0 - : 1 + countWhere(argumentsList.getChildren(), arg => arg.kind === SyntaxKind.CommaToken); + function getArgumentIndex(argumentsList: Node, node: Node) { + // 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. + var argumentIndex = 0; + var listChildren = argumentsList.getChildren(); + for (var i = 0, n = listChildren.length; i < n; i++) { + var child = listChildren[i]; + if (child === node) { + break; + } + if (child.kind !== SyntaxKind.CommaToken) { + argumentIndex++; + } + } + + return argumentIndex; + } + + function getArgumentCount(argumentsList: Node) { + // 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, givin us an + // arg count of 3. + var listChildren = argumentsList.getChildren(); + + var argumentCount = countWhere(listChildren, arg => arg.kind !== SyntaxKind.CommaToken); + if (listChildren.length > 0 && lastOrUndefined(listChildren).kind === SyntaxKind.CommaToken) { + argumentCount++; + } + + return argumentCount; } // spanIndex is either the index for a given template span. diff --git a/src/services/utilities.ts b/src/services/utilities.ts index e0ff529354667..f2403217c2496 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -95,6 +95,8 @@ module ts { } }); + // Either we didn't find an appropriate list, or the list must contain us. + Debug.assert(!syntaxList || contains(syntaxList.getChildren(), node)); return syntaxList; } diff --git a/tests/cases/fourslash/signatureHelpWithInvalidArgumentList1.ts b/tests/cases/fourslash/signatureHelpWithInvalidArgumentList1.ts new file mode 100644 index 0000000000000..f280adb1d9e2e --- /dev/null +++ b/tests/cases/fourslash/signatureHelpWithInvalidArgumentList1.ts @@ -0,0 +1,9 @@ +/// + +////function foo(a) { } +////foo(hello my name /**/is + +goTo.marker(); +verify.signatureHelpPresent(); +verify.signatureHelpCountIs(1); +