Merge pull request #792 from Microsoft/sigHelp

Various small fixes related to signature help
This commit is contained in:
Jason Freeman
2014-10-01 13:15:14 -07:00
10 changed files with 202 additions and 118 deletions

View File

@@ -4331,25 +4331,32 @@ module ts {
}
function signatureHasCorrectArity(node: CallExpression, signature: Signature): boolean {
var args = node.arguments || emptyArray;
var isCorrect = args.length >= signature.minArgumentCount &&
(signature.hasRestParameter || args.length <= signature.parameters.length) &&
(!node.typeArguments || signature.typeParameters && node.typeArguments.length === signature.typeParameters.length);
// For error recovery, since we have parsed OmittedExpressions for any extra commas
// in the argument list, if we see any OmittedExpressions, just return true.
// The reason this is ok is because omitted expressions here are syntactically
// illegal, and will cause a parse error.
// Note: It may be worth keeping the upper bound check on arity, but removing
// the lower bound check if there are omitted expressions.
if (!isCorrect) {
// Technically this type assertion is not safe because args could be initialized to emptyArray
// above.
if ((<NodeArray<Node>>args).hasTrailingComma || forEach(args, arg => arg.kind === SyntaxKind.OmittedExpression)) {
return true;
}
if (!node.arguments) {
// This only happens when we have something of the form:
// new C
//
return signature.minArgumentCount === 0;
}
return isCorrect;
// For IDE scenarios, since we may have an incomplete call, we make two modifications
// to arity checking.
// 1. A trailing comma is tantamount to adding another argument
// 2. If the call is incomplete (no closing paren) allow fewer arguments than expected
var args = node.arguments;
var numberOfArgs = args.hasTrailingComma ? args.length + 1 : args.length;
var hasTooManyArguments = !signature.hasRestParameter && numberOfArgs > signature.parameters.length;
var hasRightNumberOfTypeArguments = !node.typeArguments ||
(signature.typeParameters && node.typeArguments.length === signature.typeParameters.length);
if (hasTooManyArguments || !hasRightNumberOfTypeArguments) {
return false;
}
// If we are missing the close paren, the call is incomplete, and we should skip
// the lower bound check.
var callIsIncomplete = args.end === node.end;
var hasEnoughArguments = numberOfArgs >= signature.minArgumentCount;
return callIsIncomplete || hasEnoughArguments;
}
// If type has a single call signature and no other members, return that signature. Otherwise, return undefined.

View File

@@ -1073,7 +1073,7 @@ module ts {
case ParsingContext.TypeParameters:
return isIdentifier();
case ParsingContext.ArgumentExpressions:
return isExpression();
return token === SyntaxKind.CommaToken || isExpression();
case ParsingContext.ArrayLiteralMembers:
return token === SyntaxKind.CommaToken || isExpression();
case ParsingContext.Parameters:
@@ -1226,18 +1226,6 @@ module ts {
error(Diagnostics._0_expected, ",");
}
else if (isListTerminator(kind)) {
// Check if the last token was a comma.
if (commaStart >= 0) {
if (!allowTrailingComma) {
if (file.syntacticErrors.length === errorCountBeforeParsingList) {
// Report a grammar error so we don't affect lookahead
grammarErrorAtPos(commaStart, scanner.getStartPos() - commaStart, Diagnostics.Trailing_comma_not_allowed);
}
}
// Always preserve a trailing comma by marking it on the NodeArray
result.hasTrailingComma = true;
}
break;
}
else {
@@ -1248,6 +1236,23 @@ module ts {
nextToken();
}
}
// Recording the trailing comma is deliberately done after the previous
// loop, and not just if we see a list terminator. This is because the list
// may have ended incorrectly, but it is still important to know if there
// was a trailing comma.
// Check if the last token was a comma.
if (commaStart >= 0) {
if (!allowTrailingComma) {
if (file.syntacticErrors.length === errorCountBeforeParsingList) {
// Report a grammar error so we don't affect lookahead
grammarErrorAtPos(commaStart, scanner.getStartPos() - commaStart, Diagnostics.Trailing_comma_not_allowed);
}
}
// Always preserve a trailing comma by marking it on the NodeArray
result.hasTrailingComma = true;
}
result.end = getNodeEnd();
parsingContext = saveParsingContext;
return result;

View File

@@ -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';
@@ -944,29 +951,16 @@ module FourSlash {
this.validate("error", message, renameInfo.localizedErrorMessage);
}
//private getFormalParameter() {
// var help = this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition);
// return help.formal;
//}
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];
}

View File

@@ -172,15 +172,15 @@ module ts.SignatureHelp {
return undefined;
}
var argumentList = getContainingArgumentList(startingToken);
var argumentInfo = getContainingArgumentInfo(startingToken);
cancellationToken.throwIfCancellationRequested();
// Semantic filtering of signature help
if (!argumentList) {
if (!argumentInfo) {
return undefined;
}
var call = <CallExpression>argumentList.parent;
var call = <CallExpression>argumentInfo.list.parent;
var candidates = <Signature[]>[];
var resolvedSignature = typeInfoResolver.getResolvedSignature(call, candidates);
cancellationToken.throwIfCancellationRequested();
@@ -189,13 +189,13 @@ module ts.SignatureHelp {
return undefined;
}
return createSignatureHelpItems(candidates, resolvedSignature, argumentList);
return createSignatureHelpItems(candidates, resolvedSignature, argumentInfo);
/**
* If node is an argument, returns its index in the argument list.
* If not, returns -1.
*/
function getImmediatelyContainingArgumentList(node: Node): Node {
function getImmediatelyContainingArgumentInfo(node: Node): ListItemInfo {
if (node.parent.kind !== SyntaxKind.CallExpression && node.parent.kind !== SyntaxKind.NewExpression) {
return undefined;
}
@@ -216,10 +216,14 @@ module ts.SignatureHelp {
var parent = <CallExpression>node.parent;
// Find out if 'node' is an argument, a type argument, or neither
if (node.kind === SyntaxKind.LessThanToken || node.kind === SyntaxKind.OpenParenToken) {
// Find the list that starts right *after* the < or ( token
// Find the list that starts right *after* the < or ( token.
// If the user has just opened a list, consider this item 0.
var list = getChildListThatStartsWithOpenerToken(parent, node, sourceFile);
Debug.assert(list);
return list;
return {
list: list,
listItemIndex: 0
};
}
if (node.kind === SyntaxKind.GreaterThanToken
@@ -228,18 +232,18 @@ module ts.SignatureHelp {
return undefined;
}
return findContainingList(node);
return findListItemInfo(node);
}
function getContainingArgumentList(node: Node): Node {
function getContainingArgumentInfo(node: Node): ListItemInfo {
for (var n = node; n.kind !== SyntaxKind.SourceFile; n = n.parent) {
if (n.kind === SyntaxKind.FunctionBlock) {
return undefined;
}
var argumentList = getImmediatelyContainingArgumentList(n);
if (argumentList) {
return argumentList;
var argumentInfo = getImmediatelyContainingArgumentInfo(n);
if (argumentInfo) {
return argumentInfo;
}
@@ -248,7 +252,35 @@ module ts.SignatureHelp {
return undefined;
}
function createSignatureHelpItems(candidates: Signature[], bestSignature: Signature, argumentListOrTypeArgumentList: Node): SignatureHelpItems {
/**
* 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 => {
var parameters = candidateSignature.parameters;
var parameterHelpItems: SignatureHelpParameter[] = parameters.length === 0 ? emptyArray : map(parameters, p => {
@@ -321,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,
//
@@ -338,63 +365,38 @@ module ts.SignatureHelp {
var applicableSpanEnd = skipTrivia(sourceFile.text, argumentListOrTypeArgumentList.end, /*stopAfterLineBreak*/ false);
var applicableSpan = new TypeScript.TextSpan(applicableSpanStart, applicableSpanEnd - applicableSpanStart);
var state = getSignatureHelpCurrentArgumentState(sourceFile, position, applicableSpanStart);
// 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 = (argumentInfoOrTypeArgumentInfo.listItemIndex + 1) >> 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,
selectedItemIndex: selectedItemIndex,
argumentIndex: state.argumentIndex,
argumentCount: state.argumentCount
argumentIndex: argumentIndex,
argumentCount: argumentCount
};
}
}
function getSignatureHelpCurrentArgumentState(sourceFile: SourceFile, position: number, applicableSpanStart: number): { argumentIndex: number; argumentCount: number } {
var tokenPrecedingSpanStart = findPrecedingToken(applicableSpanStart, sourceFile);
if (!tokenPrecedingSpanStart) {
return undefined;
}
if (tokenPrecedingSpanStart.kind !== SyntaxKind.OpenParenToken && tokenPrecedingSpanStart.kind !== SyntaxKind.LessThanToken) {
// The span start must have moved backward in the file (for example if the open paren was backspaced)
return undefined;
}
var tokenPrecedingCurrentPosition = findPrecedingToken(position, sourceFile);
var call = <CallExpression>tokenPrecedingSpanStart.parent;
Debug.assert(call.kind === SyntaxKind.CallExpression || call.kind === SyntaxKind.NewExpression, "wrong call kind " + SyntaxKind[call.kind]);
if (tokenPrecedingCurrentPosition.kind === SyntaxKind.CloseParenToken || tokenPrecedingCurrentPosition.kind === SyntaxKind.GreaterThanToken) {
if (tokenPrecedingCurrentPosition.parent === call) {
// This call expression is complete. Stop signature help.
return undefined;
}
}
var argumentListOrTypeArgumentList = getChildListThatStartsWithOpenerToken(call, tokenPrecedingSpanStart, sourceFile);
// Debug.assert(argumentListOrTypeArgumentList.getChildCount() === 0 || argumentListOrTypeArgumentList.getChildCount() % 2 === 1, "Even number of children");
// The call might be finished, but incorrectly. Check if we are still within the bounds of the call
if (position > skipTrivia(sourceFile.text, argumentListOrTypeArgumentList.end, /*stopAfterLineBreak*/ false)) {
return undefined;
}
var numberOfCommas = countWhere(argumentListOrTypeArgumentList.getChildren(), arg => arg.kind === SyntaxKind.CommaToken);
var argumentCount = numberOfCommas + 1;
if (argumentCount <= 1) {
return { argumentIndex: 0, argumentCount: argumentCount };
}
var indexOfNodeContainingPosition = findListItemIndexContainingPosition(argumentListOrTypeArgumentList, position);
// indexOfNodeContainingPosition checks that position is between pos and end of each child, so it is
// possible that we are to the right of all children. Assume that we are still within
// the applicable span and that we are typing the last argument
// Alternatively, we could be in range of one of the arguments, in which case we need to divide
// by 2 to exclude commas. Use bit shifting in order to take the floor of the division.
var argumentIndex = indexOfNodeContainingPosition < 0 ? argumentCount - 1 : indexOfNodeContainingPosition >> 1;
return { argumentIndex: argumentIndex, argumentCount: argumentCount };
}
function getChildListThatStartsWithOpenerToken(parent: Node, openerToken: Node, sourceFile: SourceFile): Node {
var children = parent.getChildren(sourceFile);
var indexOfOpenerToken = children.indexOf(openerToken);

View File

@@ -1,18 +1,24 @@
tests/cases/compiler/trailingSeparatorInFunctionCall.ts(4,7): error TS1009: Trailing comma not allowed.
tests/cases/compiler/trailingSeparatorInFunctionCall.ts(9,8): error TS1009: Trailing comma not allowed.
tests/cases/compiler/trailingSeparatorInFunctionCall.ts(4,1): error TS2346: Supplied parameters do not match any signature of call target.
tests/cases/compiler/trailingSeparatorInFunctionCall.ts(9,1): error TS2346: Supplied parameters do not match any signature of call target.
==== tests/cases/compiler/trailingSeparatorInFunctionCall.ts (2 errors) ====
==== tests/cases/compiler/trailingSeparatorInFunctionCall.ts (4 errors) ====
function f(x, y) {
}
f(1, 2, );
~
!!! error TS1009: Trailing comma not allowed.
~~~~~~~~~
!!! error TS2346: Supplied parameters do not match any signature of call target.
function f2<T>(x: T, y: T) {
}
f2(1, 2, );
~
!!! error TS1009: Trailing comma not allowed.
!!! error TS1009: Trailing comma not allowed.
~~~~~~~~~~
!!! error TS2346: Supplied parameters do not match any signature of call target.

View File

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

View File

@@ -13,6 +13,6 @@ verify.currentParameterSpanIs("x: string");
edit.insert("'',");
verify.signatureHelpCountIs(2);
// verify.currentSignatureHelpIs("fn(x: string, y: number): any");
// verify.currentParameterHelpArgumentNameIs("y");
// verify.currentParameterSpanIs("y: number");
verify.currentSignatureHelpIs("fn(x: string, y: number): any");
verify.currentParameterHelpArgumentNameIs("y");
verify.currentParameterSpanIs("y: number");

View File

@@ -0,0 +1,20 @@
/// <reference path='fourslash.ts'/>
////declare function f(s: string);
////declare function f(n: number);
////declare function f(s: string, b: boolean);
////declare function f(n: number, b: boolean);
////
////f(1/**/
goTo.marker();
verify.signatureHelpCountIs(4);
verify.currentSignatureHelpIs("f(n: number): any");
verify.currentParameterHelpArgumentNameIs("n");
verify.currentParameterSpanIs("n: number");
edit.insert(", ");
verify.signatureHelpCountIs(4);
verify.currentSignatureHelpIs("f(n: number, b: boolean): any");
verify.currentParameterHelpArgumentNameIs("b");
verify.currentParameterSpanIs("b: boolean");

View File

@@ -0,0 +1,20 @@
/// <reference path='fourslash.ts'/>
////declare function f(s: string);
////declare function f(n: number);
////declare function f(s: string, b: boolean);
////declare function f(n: number, b: boolean);
////
////f(1/**/ var
goTo.marker();
verify.signatureHelpCountIs(4);
verify.currentSignatureHelpIs("f(n: number): any");
verify.currentParameterHelpArgumentNameIs("n");
verify.currentParameterSpanIs("n: number");
edit.insert(", ");
verify.signatureHelpCountIs(4);
verify.currentSignatureHelpIs("f(n: number, b: boolean): any");
verify.currentParameterHelpArgumentNameIs("b");
verify.currentParameterSpanIs("b: boolean");

View File

@@ -0,0 +1,26 @@
/// <reference path='fourslash.ts'/>
////declare function f();
////declare function f(s: string);
////declare function f(s: string, b: boolean);
////declare function f(n: number, b: boolean);
////
////f(/**/
goTo.marker();
verify.signatureHelpCountIs(4);
verify.currentSignatureHelpIs("f(): any");
verify.currentSignatureParamterCountIs(0);
verify.signatureHelpArgumentCountIs(0);
edit.insert(", ");
verify.signatureHelpCountIs(4);
verify.currentSignatureHelpIs("f(s: string, b: boolean): any");
verify.currentSignatureParamterCountIs(2);
verify.currentParameterHelpArgumentNameIs("b");
verify.currentParameterSpanIs("b: boolean");
edit.insert(", ");
verify.signatureHelpCountIs(4);
verify.currentSignatureHelpIs("f(s: string, b: boolean): any");
verify.currentSignatureParamterCountIs(2);