From 390b92dac05d90c2dd364b1f0c7262b78d003012 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Wed, 1 Oct 2014 17:09:50 -0700 Subject: [PATCH 1/8] Move getChildListThatStartsWithOpenerToken inside getSignatureHelpItems --- src/services/signatureHelp.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 273cfda99b8..f5f678c3021 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -258,6 +258,12 @@ module ts.SignatureHelp { return undefined; } + function getChildListThatStartsWithOpenerToken(parent: Node, openerToken: Node, sourceFile: SourceFile): Node { + var children = parent.getChildren(sourceFile); + var indexOfOpenerToken = children.indexOf(openerToken); + return children[indexOfOpenerToken + 1]; + } + /** * The selectedItemIndex could be negative for several reasons. * 1. There are too many arguments for all of the overloads @@ -402,10 +408,4 @@ module ts.SignatureHelp { }; } } - - function getChildListThatStartsWithOpenerToken(parent: Node, openerToken: Node, sourceFile: SourceFile): Node { - var children = parent.getChildren(sourceFile); - var indexOfOpenerToken = children.indexOf(openerToken); - return children[indexOfOpenerToken + 1]; - } } \ No newline at end of file From 09f0755dc1ac2e1bc89ed27fb74eb6a98d505544 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Wed, 1 Oct 2014 17:10:10 -0700 Subject: [PATCH 2/8] Reenable some basic generics tests --- tests/cases/fourslash/genericParameterHelp.ts | 36 +++++++++---------- .../signatureHelpOnNestedOverloads.ts | 6 ++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/cases/fourslash/genericParameterHelp.ts b/tests/cases/fourslash/genericParameterHelp.ts index a1df1cc2f32..5e6b0dc5293 100644 --- a/tests/cases/fourslash/genericParameterHelp.ts +++ b/tests/cases/fourslash/genericParameterHelp.ts @@ -38,17 +38,17 @@ // verify.currentParameterHelpArgumentNameIs("U"); // verify.currentParameterSpanIs("U"); - goTo.marker("3"); - verify.currentParameterHelpArgumentNameIs("a"); - verify.currentParameterSpanIs("a: T"); +goTo.marker("3"); +verify.currentParameterHelpArgumentNameIs("a"); +verify.currentParameterSpanIs("a: T"); - // goTo.marker("4"); - // verify.currentParameterHelpArgumentNameIs("M"); - // verify.currentParameterSpanIs("M extends IFoo"); +goTo.marker("4"); +verify.currentParameterHelpArgumentNameIs("M"); +verify.currentParameterSpanIs("M extends IFoo"); - // goTo.marker("5"); - // verify.currentParameterHelpArgumentNameIs("M"); - // verify.currentParameterSpanIs("M extends IFoo"); +goTo.marker("5"); +verify.currentParameterHelpArgumentNameIs("M"); +verify.currentParameterSpanIs("M extends IFoo"); // goTo.marker("construcor1"); // verify.currentSignatureHelpIs("testClass(a: T, b: U, c: M): testClass"); @@ -59,17 +59,17 @@ // verify.currentParameterHelpArgumentNameIs("U"); // verify.currentParameterSpanIs("U"); - //goTo.marker("construcor3"); - //verify.currentParameterHelpArgumentNameIs("T"); - //verify.currentParameterSpanIs("T extends IFoo"); +goTo.marker("construcor3"); +verify.currentParameterHelpArgumentNameIs("T"); +verify.currentParameterSpanIs("T extends IFoo"); - // goTo.marker("construcor4"); - // verify.currentParameterHelpArgumentNameIs("M"); - // verify.currentParameterSpanIs("M extends IFoo"); +goTo.marker("construcor4"); +verify.currentParameterHelpArgumentNameIs("M"); +verify.currentParameterSpanIs("M extends IFoo"); - // goTo.marker("construcor5"); - // verify.currentParameterHelpArgumentNameIs("U"); - // verify.currentParameterSpanIs("U"); +goTo.marker("construcor5"); +verify.currentParameterHelpArgumentNameIs("U"); +verify.currentParameterSpanIs("U"); // goTo.marker("type1"); // verify.signatureHelpCountIs(1); diff --git a/tests/cases/fourslash/signatureHelpOnNestedOverloads.ts b/tests/cases/fourslash/signatureHelpOnNestedOverloads.ts index 476d79a8243..e657000c113 100644 --- a/tests/cases/fourslash/signatureHelpOnNestedOverloads.ts +++ b/tests/cases/fourslash/signatureHelpOnNestedOverloads.ts @@ -15,6 +15,6 @@ verify.currentParameterSpanIs("x: string"); edit.insert("'',"); verify.signatureHelpCountIs(2); -// verify.currentSignatureHelpIs("fn2(x: string, y: number): any"); -// verify.currentParameterHelpArgumentNameIs("y"); -// verify.currentParameterSpanIs("y: number"); +verify.currentSignatureHelpIs("fn2(x: string, y: number): any"); +verify.currentParameterHelpArgumentNameIs("y"); +verify.currentParameterSpanIs("y: number"); From 08f9a0148a22c8fd6a65349f684db2b7b0e9bbf9 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Thu, 2 Oct 2014 11:24:11 -0700 Subject: [PATCH 3/8] Parser changes to parse omitted types correctly --- src/compiler/parser.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index b7582b9ed06..3c39a5a0bef 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -1078,7 +1078,7 @@ module ts { return isParameter(); case ParsingContext.TypeArguments: case ParsingContext.TupleElementTypes: - return isType(); + return token === SyntaxKind.CommaToken || isType(); } Debug.fail("Non-exhaustive case in 'isListElement'."); @@ -2344,13 +2344,24 @@ module ts { function parseTypeArguments(): NodeArray { var typeArgumentListStart = scanner.getTokenPos(); var errorCountBeforeTypeParameterList = file.syntacticErrors.length; - var result = parseBracketedList(ParsingContext.TypeArguments, parseType, SyntaxKind.LessThanToken, SyntaxKind.GreaterThanToken); + var result = parseBracketedList(ParsingContext.TypeArguments, parseSingleTypeArgument, SyntaxKind.LessThanToken, SyntaxKind.GreaterThanToken); if (!result.length && file.syntacticErrors.length === errorCountBeforeTypeParameterList) { grammarErrorAtPos(typeArgumentListStart, scanner.getStartPos() - typeArgumentListStart, Diagnostics.Type_argument_list_cannot_be_empty); } return result; } + function parseSingleTypeArgument(): TypeNode { + if (token === SyntaxKind.CommaToken) { + var errorStart = scanner.getTokenPos(); + var errorLength = scanner.getTextPos() - errorStart; + grammarErrorAtPos(errorStart, errorLength, Diagnostics.Type_expected); + return createNode(SyntaxKind.OmittedType); + } + + return parseType(); + } + function parsePrimaryExpression(): Expression { switch (token) { case SyntaxKind.ThisKeyword: From 464f2a074526ffce4a0381789bbb1efa448efd69 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Thu, 2 Oct 2014 12:21:06 -0700 Subject: [PATCH 4/8] Split genericParameterHelp test into multiple tests --- tests/cases/fourslash/genericParameterHelp.ts | 58 +------------------ .../genericParameterHelpConstructorCalls.ts | 35 +++++++++++ .../genericParameterHelpTypeReferences.ts | 34 +++++++++++ 3 files changed, 70 insertions(+), 57 deletions(-) create mode 100644 tests/cases/fourslash/genericParameterHelpConstructorCalls.ts create mode 100644 tests/cases/fourslash/genericParameterHelpTypeReferences.ts diff --git a/tests/cases/fourslash/genericParameterHelp.ts b/tests/cases/fourslash/genericParameterHelp.ts index 5e6b0dc5293..17882485fd7 100644 --- a/tests/cases/fourslash/genericParameterHelp.ts +++ b/tests/cases/fourslash/genericParameterHelp.ts @@ -6,26 +6,12 @@ //// return null; ////} //// -////class testClass { -//// constructor(a:T, b:U, c:M){ } -////} ////// Function calls ////testFunction(/*3*/ ////testFunction(null, null, null); ////testFunction<, ,/*5*/>(null, null, null); -////// Constructor calls -////new testClass(null, null, null) -////new testClass<,,/*construcor4*/>(null, null, null) -////new testClass(null, null, null) -////// Generic types -////testClass extends testClass; // goTo.marker("1"); // verify.currentSignatureParamterCountIs(3); @@ -48,46 +34,4 @@ verify.currentParameterSpanIs("M extends IFoo"); goTo.marker("5"); verify.currentParameterHelpArgumentNameIs("M"); -verify.currentParameterSpanIs("M extends IFoo"); - - // goTo.marker("construcor1"); - // verify.currentSignatureHelpIs("testClass(a: T, b: U, c: M): testClass"); - // verify.currentParameterHelpArgumentNameIs("T"); - // verify.currentParameterSpanIs("T extends IFoo"); - - // goTo.marker("construcor2"); - // verify.currentParameterHelpArgumentNameIs("U"); - // verify.currentParameterSpanIs("U"); - -goTo.marker("construcor3"); -verify.currentParameterHelpArgumentNameIs("T"); -verify.currentParameterSpanIs("T extends IFoo"); - -goTo.marker("construcor4"); -verify.currentParameterHelpArgumentNameIs("M"); -verify.currentParameterSpanIs("M extends IFoo"); - -goTo.marker("construcor5"); -verify.currentParameterHelpArgumentNameIs("U"); -verify.currentParameterSpanIs("U"); - - // goTo.marker("type1"); - // verify.signatureHelpCountIs(1); - // verify.currentSignatureHelpIs("testClass"); - // verify.currentParameterHelpArgumentNameIs("T"); - // verify.currentParameterSpanIs("T extends IFoo"); - - // goTo.marker("type2"); - // verify.signatureHelpCountIs(1); - // verify.currentParameterHelpArgumentNameIs("T"); - // verify.currentParameterSpanIs("T extends IFoo"); - - // goTo.marker("type3"); - // verify.signatureHelpCountIs(1); - // verify.currentParameterHelpArgumentNameIs("T"); - // verify.currentParameterSpanIs("T extends IFoo"); - - // goTo.marker("type4"); - // verify.signatureHelpCountIs(1); - // verify.currentParameterHelpArgumentNameIs("M"); - // verify.currentParameterSpanIs("M extends IFoo"); \ No newline at end of file +verify.currentParameterSpanIs("M extends IFoo"); \ No newline at end of file diff --git a/tests/cases/fourslash/genericParameterHelpConstructorCalls.ts b/tests/cases/fourslash/genericParameterHelpConstructorCalls.ts new file mode 100644 index 00000000000..4cbe141fcce --- /dev/null +++ b/tests/cases/fourslash/genericParameterHelpConstructorCalls.ts @@ -0,0 +1,35 @@ +/// + +////interface IFoo { } +//// +////class testClass { +//// constructor(a:T, b:U, c:M){ } +////} +//// +////// Constructor calls +////new testClass(null, null, null) +////new testClass<,,/*construcor4*/>(null, null, null) +////new testClass(null, null, null) + + // goTo.marker("construcor1"); + // verify.currentSignatureHelpIs("testClass(a: T, b: U, c: M): testClass"); + // verify.currentParameterHelpArgumentNameIs("T"); + // verify.currentParameterSpanIs("T extends IFoo"); + + // goTo.marker("construcor2"); + // verify.currentParameterHelpArgumentNameIs("U"); + // verify.currentParameterSpanIs("U"); + +goTo.marker("construcor3"); +verify.currentParameterHelpArgumentNameIs("T"); +verify.currentParameterSpanIs("T extends IFoo"); + +goTo.marker("construcor4"); +verify.currentParameterHelpArgumentNameIs("M"); +verify.currentParameterSpanIs("M extends IFoo"); + +goTo.marker("construcor5"); +verify.currentParameterHelpArgumentNameIs("U"); +verify.currentParameterSpanIs("U"); diff --git a/tests/cases/fourslash/genericParameterHelpTypeReferences.ts b/tests/cases/fourslash/genericParameterHelpTypeReferences.ts new file mode 100644 index 00000000000..346cf7191c1 --- /dev/null +++ b/tests/cases/fourslash/genericParameterHelpTypeReferences.ts @@ -0,0 +1,34 @@ +/// + +////interface IFoo { } +//// +////class testClass { +//// constructor(a:T, b:U, c:M){ } +////} +//// +////// Generic types +////testClass extends testClass; + + // goTo.marker("type1"); + // verify.signatureHelpCountIs(1); + // verify.currentSignatureHelpIs("testClass"); + // verify.currentParameterHelpArgumentNameIs("T"); + // verify.currentParameterSpanIs("T extends IFoo"); + + // goTo.marker("type2"); + // verify.signatureHelpCountIs(1); + // verify.currentParameterHelpArgumentNameIs("T"); + // verify.currentParameterSpanIs("T extends IFoo"); + + // goTo.marker("type3"); + // verify.signatureHelpCountIs(1); + // verify.currentParameterHelpArgumentNameIs("T"); + // verify.currentParameterSpanIs("T extends IFoo"); + + // goTo.marker("type4"); + // verify.signatureHelpCountIs(1); + // verify.currentParameterHelpArgumentNameIs("M"); + // verify.currentParameterSpanIs("M extends IFoo"); \ No newline at end of file From 628fa4c7b286b2634dbfb61c7203bf99934e8d8a Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Thu, 2 Oct 2014 14:39:10 -0700 Subject: [PATCH 5/8] Remove OmittedType --- src/compiler/parser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 3c39a5a0bef..f71af45c7de 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -2356,7 +2356,7 @@ module ts { var errorStart = scanner.getTokenPos(); var errorLength = scanner.getTextPos() - errorStart; grammarErrorAtPos(errorStart, errorLength, Diagnostics.Type_expected); - return createNode(SyntaxKind.OmittedType); + return createNode(SyntaxKind.Missing); } return parseType(); From 808ded88d62f8c4199c44015492889457f0dddd7 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Wed, 15 Oct 2014 15:25:29 -0700 Subject: [PATCH 6/8] Signature Help relies on display building routines in checker --- src/compiler/checker.ts | 115 +++++++++++++++++++--------------- src/compiler/types.ts | 3 + src/services/services.ts | 2 +- src/services/signatureHelp.ts | 49 +++------------ 4 files changed, 79 insertions(+), 90 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 097d0c76c08..29ded915826 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -995,14 +995,26 @@ module ts { function getSymbolDisplayBuilder(): SymbolDisplayBuilder { // Enclosing declaration is optional when we don't want to get qualified name in the enclosing declaration scope // Meaning needs to be specified if the enclosing declaration is given + function buildPlainSymbolName(symbol: Symbol, writer: SymbolWriter): void { + if (symbol.declarations && symbol.declarations.length > 0) { + var declaration = symbol.declarations[0]; + if (declaration.name) { + writer.writeSymbol(identifierToString(declaration.name), symbol); + return; + } + } + + writer.writeSymbol(symbol.name, symbol); + } + function buildSymbolDisplay(symbol: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, meaning?: SymbolFlags, flags?: SymbolFormatFlags): void { var parentSymbol: Symbol; - function writeSymbolName(symbol: Symbol): void { + function appendSymbolName(symbol: Symbol): void { if (parentSymbol) { // Write type arguments of instantiated class/interface here if (flags & SymbolFormatFlags.WriteTypeParametersOrArguments) { if (symbol.flags & SymbolFlags.Instantiated) { - buildTypeArgumentListDisplay(getTypeParametersOfClassOrInterface(parentSymbol), + buildDisplayForTypeArgumentsAndDelimiters(getTypeParametersOfClassOrInterface(parentSymbol), (symbol).mapper, writer, enclosingDeclaration); } else { @@ -1012,15 +1024,7 @@ module ts { writePunctuation(writer, SyntaxKind.DotToken); } parentSymbol = symbol; - if (symbol.declarations && symbol.declarations.length > 0) { - var declaration = symbol.declarations[0]; - if (declaration.name) { - writer.writeSymbol(identifierToString(declaration.name), symbol); - return; - } - } - - writer.writeSymbol(symbol.name, symbol); + buildPlainSymbolName(symbol, writer); } // Let the writer know we just wrote out a symbol. The declaration emitter writer uses @@ -1046,7 +1050,7 @@ module ts { if (accessibleSymbolChain) { for (var i = 0, n = accessibleSymbolChain.length; i < n; i++) { - writeSymbolName(accessibleSymbolChain[i]); + appendSymbolName(accessibleSymbolChain[i]); } } else { @@ -1060,7 +1064,7 @@ module ts { return; } - writeSymbolName(symbol); + appendSymbolName(symbol); } } } @@ -1074,7 +1078,7 @@ module ts { return; } - return writeSymbolName(symbol); + return appendSymbolName(symbol); } function buildTypeDisplay(type: Type, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { @@ -1302,8 +1306,15 @@ module ts { } } + function buildTypeParameterDisplayFromSymbol(symbol: Symbol, writer: SymbolWriter, enclosingDeclaraiton?: Node, flags?: TypeFormatFlags) { + var targetSymbol = getTargetSymbol(symbol); + if (targetSymbol.flags & SymbolFlags.Class || targetSymbol.flags & SymbolFlags.Interface) { + buildDisplayForTypeParametersAndDelimiters(getTypeParametersOfClassOrInterface(symbol), writer, enclosingDeclaraiton, flags); + } + } + function buildTypeParameterDisplay(tp: TypeParameter, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { - buildSymbolDisplay(tp.symbol, writer); + buildPlainSymbolName(tp.symbol, writer); var constraint = getConstraintOfTypeParameter(tp); if (constraint) { writeSpace(writer); @@ -1313,7 +1324,21 @@ module ts { } } - function buildTypeParameterListDisplay(typeParameters: TypeParameter[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { + function buildParameterDisplay(p: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { + if (getDeclarationFlagsFromSymbol(p) & NodeFlags.Rest) { + writePunctuation(writer, SyntaxKind.DotDotDotToken); + } + buildPlainSymbolName(p, writer); + if (p.valueDeclaration.flags & NodeFlags.QuestionMark || (p.valueDeclaration).initializer) { + writePunctuation(writer, SyntaxKind.QuestionToken); + } + writePunctuation(writer, SyntaxKind.ColonToken); + writeSpace(writer); + + buildTypeDisplay(getTypeOfSymbol(p), writer, enclosingDeclaration, flags, typeStack); + } + + function buildDisplayForTypeParametersAndDelimiters(typeParameters: TypeParameter[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { if (typeParameters && typeParameters.length) { writePunctuation(writer, SyntaxKind.LessThanToken); for (var i = 0; i < typeParameters.length; i++) { @@ -1327,7 +1352,7 @@ module ts { } } - function buildTypeArgumentListDisplay(typeParameters: TypeParameter[], mapper: TypeMapper, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { + function buildDisplayForTypeArgumentsAndDelimiters(typeParameters: TypeParameter[], mapper: TypeMapper, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { if (typeParameters && typeParameters.length) { writePunctuation(writer, SyntaxKind.LessThanToken); for (var i = 0; i < typeParameters.length; i++) { @@ -1341,42 +1366,19 @@ module ts { } } - function buildTypeParameterDisplayFromSymbol(symbol: Symbol, writer: SymbolWriter, enclosingDeclaraiton?: Node, flags?: TypeFormatFlags) { - var targetSymbol = getTargetSymbol(symbol); - if (targetSymbol.flags & SymbolFlags.Class || targetSymbol.flags & SymbolFlags.Interface) { - buildTypeParameterListDisplay(getTypeParametersOfClassOrInterface(symbol), writer, enclosingDeclaraiton, flags); - } - } - - function buildSignatureDisplay(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { - if (signature.target && (flags & TypeFormatFlags.WriteTypeArgumentsOfSignature)) { - // Instantiated signature, write type arguments instead - buildTypeArgumentListDisplay(signature.target.typeParameters, signature.mapper, writer, enclosingDeclaration); - } - else { - buildTypeParameterListDisplay(signature.typeParameters, writer, enclosingDeclaration, flags, typeStack); - } + function buildDisplayForParametersAndDelimiters(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { writePunctuation(writer, SyntaxKind.OpenParenToken); for (var i = 0; i < signature.parameters.length; i++) { if (i > 0) { writePunctuation(writer, SyntaxKind.CommaToken); writeSpace(writer); } - var p = signature.parameters[i]; - if (getDeclarationFlagsFromSymbol(p) & NodeFlags.Rest) { - writePunctuation(writer, SyntaxKind.DotDotDotToken); - } - buildSymbolDisplay(p, writer); - if (p.valueDeclaration.flags & NodeFlags.QuestionMark || (p.valueDeclaration).initializer) { - writePunctuation(writer, SyntaxKind.QuestionToken); - } - writePunctuation(writer, SyntaxKind.ColonToken); - writeSpace(writer); - - buildTypeDisplay(getTypeOfSymbol(p), writer, enclosingDeclaration, flags, typeStack); + buildParameterDisplay(signature.parameters[i], writer, enclosingDeclaration, flags, typeStack); } - writePunctuation(writer, SyntaxKind.CloseParenToken); + } + + function buildReturnTypeDisplay(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { if (flags & TypeFormatFlags.WriteArrowStyleSignature) { writeSpace(writer); writePunctuation(writer, SyntaxKind.EqualsGreaterThanToken); @@ -1385,9 +1387,22 @@ module ts { writePunctuation(writer, SyntaxKind.ColonToken); } writeSpace(writer); - buildTypeDisplay(getReturnTypeOfSignature(signature), writer, enclosingDeclaration, flags, typeStack); } + + function buildSignatureDisplay(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { + if (signature.target && (flags & TypeFormatFlags.WriteTypeArgumentsOfSignature)) { + // Instantiated signature, write type arguments instead + // This is achieved by passing in the mapper separately + buildDisplayForTypeArgumentsAndDelimiters(signature.target.typeParameters, signature.mapper, writer, enclosingDeclaration); + } + else { + buildDisplayForTypeParametersAndDelimiters(signature.typeParameters, writer, enclosingDeclaration, flags, typeStack); + } + + buildDisplayForParametersAndDelimiters(signature, writer, enclosingDeclaration, flags, typeStack); + buildReturnTypeDisplay(signature, writer, enclosingDeclaration, flags, typeStack); + } return _displayBuilder || (_displayBuilder = { symbolToString: symbolToString, @@ -1395,10 +1410,12 @@ module ts { buildSymbolDisplay: buildSymbolDisplay, buildTypeDisplay: buildTypeDisplay, buildTypeParameterDisplay: buildTypeParameterDisplay, - buildTypeParameterListDisplay: buildTypeParameterListDisplay, - buildTypeArgumentListDisplay: buildTypeArgumentListDisplay, + buildParameterDisplay: buildParameterDisplay, + buildDisplayForTypeParametersAndDelimiters: buildDisplayForTypeParametersAndDelimiters, + buildDisplayForTypeArgumentsAndDelimiters: buildDisplayForTypeArgumentsAndDelimiters, buildTypeParameterDisplayFromSymbol: buildTypeParameterDisplayFromSymbol, - buildSignatureDisplay: buildSignatureDisplay + buildSignatureDisplay: buildSignatureDisplay, + buildReturnTypeDisplay: buildReturnTypeDisplay }); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index d062da142d7..25532c94f9c 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -672,8 +672,11 @@ module ts { buildTypeDisplay(type: Type, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; buildSymbolDisplay(symbol: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, meaning?: SymbolFlags, flags?: SymbolFormatFlags): void; buildSignatureDisplay(signatures: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; + buildParameterDisplay(parameter: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; buildTypeParameterDisplay(tp: TypeParameter, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; buildTypeParameterDisplayFromSymbol(symbol: Symbol, writer: SymbolWriter, enclosingDeclaraiton?: Node, flags?: TypeFormatFlags): void; + buildDisplayForTypeParametersAndDelimiters(typeParameters: TypeParameter[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]): void; + buildReturnTypeDisplay(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]): void; } export interface SymbolWriter { diff --git a/src/services/services.ts b/src/services/services.ts index 6b38cbd5b3b..5411cac8d0c 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1460,7 +1460,7 @@ module ts { } } - function mapToDisplayParts(writeDisplayParts: (writer: DisplayPartsSymbolWriter) => void): SymbolDisplayPart[] { + export function mapToDisplayParts(writeDisplayParts: (writer: DisplayPartsSymbolWriter) => void): SymbolDisplayPart[] { writeDisplayParts(displayPartWriter); var result = displayPartWriter.displayParts(); displayPartWriter.clear(); diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index f5f678c3021..327a61477e7 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -296,24 +296,10 @@ module ts.SignatureHelp { var items: SignatureHelpItem[] = map(candidates, candidateSignature => { var parameters = candidateSignature.parameters; var parameterHelpItems: SignatureHelpParameter[] = parameters.length === 0 ? emptyArray : map(parameters, p => { - var displayParts: SymbolDisplayPart[] = []; - - if (candidateSignature.hasRestParameter && parameters[parameters.length - 1] === p) { - displayParts.push(punctuationPart(SyntaxKind.DotDotDotToken)); - } - - displayParts.push(symbolPart(p.name, p)); + var displayParts = mapToDisplayParts(writer => + typeInfoResolver.getSymbolDisplayBuilder().buildParameterDisplay(p, writer, argumentListOrTypeArgumentList)); var isOptional = !!(p.valueDeclaration.flags & NodeFlags.QuestionMark); - if (isOptional) { - displayParts.push(punctuationPart(SyntaxKind.QuestionToken)); - } - - displayParts.push(punctuationPart(SyntaxKind.ColonToken)); - displayParts.push(spacePart()); - - var typeParts = typeToDisplayParts(typeInfoResolver, typeInfoResolver.getTypeOfSymbol(p), argumentListOrTypeArgumentList); - displayParts.push.apply(displayParts, typeParts); return { name: p.name, @@ -328,38 +314,21 @@ module ts.SignatureHelp { var prefixParts = callTargetSymbol ? symbolToDisplayParts(typeInfoResolver, callTargetSymbol, /*enclosingDeclaration*/ undefined, /*meaning*/ undefined) : []; - var separatorParts = [punctuationPart(SyntaxKind.CommaToken), spacePart()]; - - // TODO(jfreeman): Constraints? - if (candidateSignature.typeParameters && candidateSignature.typeParameters.length) { - prefixParts.push(punctuationPart(SyntaxKind.LessThanToken)); - - for (var i = 0, n = candidateSignature.typeParameters.length; i < n; i++) { - if (i) { - prefixParts.push.apply(prefixParts, separatorParts); - } - - var tp = candidateSignature.typeParameters[i].symbol; - prefixParts.push(symbolPart(tp.name, tp)); - } - - prefixParts.push(punctuationPart(SyntaxKind.GreaterThanToken)); - } - + var typeParameterParts = mapToDisplayParts(writer => + typeInfoResolver.getSymbolDisplayBuilder().buildDisplayForTypeParametersAndDelimiters(candidateSignature.typeParameters, writer, argumentListOrTypeArgumentList)); + prefixParts.push.apply(prefixParts, typeParameterParts); prefixParts.push(punctuationPart(SyntaxKind.OpenParenToken)); var suffixParts = [punctuationPart(SyntaxKind.CloseParenToken)]; - suffixParts.push(punctuationPart(SyntaxKind.ColonToken)); - suffixParts.push(spacePart()); - - var typeParts = typeToDisplayParts(typeInfoResolver, candidateSignature.getReturnType(), argumentListOrTypeArgumentList); - suffixParts.push.apply(suffixParts, typeParts); + var returnTypeParts = mapToDisplayParts(writer => + typeInfoResolver.getSymbolDisplayBuilder().buildReturnTypeDisplay(candidateSignature, writer, argumentListOrTypeArgumentList)); + suffixParts.push.apply(suffixParts, returnTypeParts); return { isVariadic: candidateSignature.hasRestParameter, prefixDisplayParts: prefixParts, suffixDisplayParts: suffixParts, - separatorDisplayParts: separatorParts, + separatorDisplayParts: [punctuationPart(SyntaxKind.CommaToken), spacePart()], parameters: parameterHelpItems, documentation: candidateSignature.getDocumentationComment() }; From c20e4f53eaf944abac5d47c724e60162373f24f3 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Thu, 16 Oct 2014 10:35:34 -0700 Subject: [PATCH 7/8] Produce SignatureHelpParameters from type parameters for generic help --- src/compiler/checker.ts | 9 ++-- src/compiler/types.ts | 5 ++- src/services/signatureHelp.ts | 82 +++++++++++++++++++++++++---------- 3 files changed, 66 insertions(+), 30 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 29ded915826..0c210421232 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1366,14 +1366,14 @@ module ts { } } - function buildDisplayForParametersAndDelimiters(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { + function buildDisplayForParametersAndDelimiters(parameters: Symbol[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { writePunctuation(writer, SyntaxKind.OpenParenToken); - for (var i = 0; i < signature.parameters.length; i++) { + for (var i = 0; i < parameters.length; i++) { if (i > 0) { writePunctuation(writer, SyntaxKind.CommaToken); writeSpace(writer); } - buildParameterDisplay(signature.parameters[i], writer, enclosingDeclaration, flags, typeStack); + buildParameterDisplay(parameters[i], writer, enclosingDeclaration, flags, typeStack); } writePunctuation(writer, SyntaxKind.CloseParenToken); } @@ -1400,7 +1400,7 @@ module ts { buildDisplayForTypeParametersAndDelimiters(signature.typeParameters, writer, enclosingDeclaration, flags, typeStack); } - buildDisplayForParametersAndDelimiters(signature, writer, enclosingDeclaration, flags, typeStack); + buildDisplayForParametersAndDelimiters(signature.parameters, writer, enclosingDeclaration, flags, typeStack); buildReturnTypeDisplay(signature, writer, enclosingDeclaration, flags, typeStack); } @@ -1411,6 +1411,7 @@ module ts { buildTypeDisplay: buildTypeDisplay, buildTypeParameterDisplay: buildTypeParameterDisplay, buildParameterDisplay: buildParameterDisplay, + buildDisplayForParametersAndDelimiters: buildDisplayForParametersAndDelimiters, buildDisplayForTypeParametersAndDelimiters: buildDisplayForTypeParametersAndDelimiters, buildDisplayForTypeArgumentsAndDelimiters: buildDisplayForTypeArgumentsAndDelimiters, buildTypeParameterDisplayFromSymbol: buildTypeParameterDisplayFromSymbol, diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 25532c94f9c..058cc309044 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -675,8 +675,9 @@ module ts { buildParameterDisplay(parameter: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; buildTypeParameterDisplay(tp: TypeParameter, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; buildTypeParameterDisplayFromSymbol(symbol: Symbol, writer: SymbolWriter, enclosingDeclaraiton?: Node, flags?: TypeFormatFlags): void; - buildDisplayForTypeParametersAndDelimiters(typeParameters: TypeParameter[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]): void; - buildReturnTypeDisplay(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]): void; + buildDisplayForParametersAndDelimiters(parameters: Symbol[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; + buildDisplayForTypeParametersAndDelimiters(typeParameters: TypeParameter[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; + buildReturnTypeDisplay(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void; } export interface SymbolWriter { diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 327a61477e7..26041dbd3e0 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -293,33 +293,41 @@ module ts.SignatureHelp { function createSignatureHelpItems(candidates: Signature[], bestSignature: Signature, argumentInfoOrTypeArgumentInfo: ListItemInfo): SignatureHelpItems { var argumentListOrTypeArgumentList = argumentInfoOrTypeArgumentInfo.list; + var parent = argumentListOrTypeArgumentList.parent; + var isTypeParameterHelp = parent.typeArguments && parent.typeArguments.pos === argumentListOrTypeArgumentList.pos; + Debug.assert(isTypeParameterHelp || parent.arguments.pos === argumentListOrTypeArgumentList.pos); + + var callTargetNode = (argumentListOrTypeArgumentList.parent).func; + var callTargetSymbol = typeInfoResolver.getSymbolInfo(callTargetNode); + var callTargetDisplayParts = callTargetSymbol && symbolToDisplayParts(typeInfoResolver, callTargetSymbol, /*enclosingDeclaration*/ undefined, /*meaning*/ undefined); var items: SignatureHelpItem[] = map(candidates, candidateSignature => { - var parameters = candidateSignature.parameters; - var parameterHelpItems: SignatureHelpParameter[] = parameters.length === 0 ? emptyArray : map(parameters, p => { - var displayParts = mapToDisplayParts(writer => - typeInfoResolver.getSymbolDisplayBuilder().buildParameterDisplay(p, writer, argumentListOrTypeArgumentList)); + var signatureHelpParameters: SignatureHelpParameter[]; + var prefixParts: SymbolDisplayPart[] = []; + var suffixParts: SymbolDisplayPart[] = []; - var isOptional = !!(p.valueDeclaration.flags & NodeFlags.QuestionMark); + if (callTargetDisplayParts) { + prefixParts.push.apply(prefixParts, callTargetDisplayParts); + } - return { - name: p.name, - documentation: p.getDocumentationComment(), - displayParts: displayParts, - isOptional: isOptional - }; - }); + if (isTypeParameterHelp) { + prefixParts.push(punctuationPart(SyntaxKind.LessThanToken)); + var typeParameters = candidateSignature.typeParameters; + signatureHelpParameters = typeParameters && typeParameters.length > 0 ? map(typeParameters, createSignatureHelpParameterForTypeParameter) : emptyArray; + suffixParts.push(punctuationPart(SyntaxKind.GreaterThanToken)); + var parameterParts = mapToDisplayParts(writer => + typeInfoResolver.getSymbolDisplayBuilder().buildDisplayForParametersAndDelimiters(candidateSignature.parameters, writer, argumentListOrTypeArgumentList)); + suffixParts.push.apply(suffixParts, parameterParts); + } + else { + var typeParameterParts = mapToDisplayParts(writer => + typeInfoResolver.getSymbolDisplayBuilder().buildDisplayForTypeParametersAndDelimiters(candidateSignature.typeParameters, writer, argumentListOrTypeArgumentList)); + prefixParts.push.apply(prefixParts, typeParameterParts); + prefixParts.push(punctuationPart(SyntaxKind.OpenParenToken)); + var parameters = candidateSignature.parameters; + signatureHelpParameters = parameters.length > 0 ? map(parameters, createSignatureHelpParameterForParameter) : emptyArray; + suffixParts.push(punctuationPart(SyntaxKind.CloseParenToken)); + } - var callTargetNode = (argumentListOrTypeArgumentList.parent).func; - var callTargetSymbol = typeInfoResolver.getSymbolInfo(callTargetNode); - - var prefixParts = callTargetSymbol ? symbolToDisplayParts(typeInfoResolver, callTargetSymbol, /*enclosingDeclaration*/ undefined, /*meaning*/ undefined) : []; - - var typeParameterParts = mapToDisplayParts(writer => - typeInfoResolver.getSymbolDisplayBuilder().buildDisplayForTypeParametersAndDelimiters(candidateSignature.typeParameters, writer, argumentListOrTypeArgumentList)); - prefixParts.push.apply(prefixParts, typeParameterParts); - prefixParts.push(punctuationPart(SyntaxKind.OpenParenToken)); - - var suffixParts = [punctuationPart(SyntaxKind.CloseParenToken)]; var returnTypeParts = mapToDisplayParts(writer => typeInfoResolver.getSymbolDisplayBuilder().buildReturnTypeDisplay(candidateSignature, writer, argumentListOrTypeArgumentList)); suffixParts.push.apply(suffixParts, returnTypeParts); @@ -329,7 +337,7 @@ module ts.SignatureHelp { prefixDisplayParts: prefixParts, suffixDisplayParts: suffixParts, separatorDisplayParts: [punctuationPart(SyntaxKind.CommaToken), spacePart()], - parameters: parameterHelpItems, + parameters: signatureHelpParameters, documentation: candidateSignature.getDocumentationComment() }; }); @@ -375,6 +383,32 @@ module ts.SignatureHelp { argumentIndex: argumentIndex, argumentCount: argumentCount }; + + function createSignatureHelpParameterForParameter(parameter: Symbol): SignatureHelpParameter { + var displayParts = mapToDisplayParts(writer => + typeInfoResolver.getSymbolDisplayBuilder().buildParameterDisplay(parameter, writer, argumentListOrTypeArgumentList)); + + var isOptional = !!(parameter.valueDeclaration.flags & NodeFlags.QuestionMark); + + return { + name: parameter.name, + documentation: parameter.getDocumentationComment(), + displayParts: displayParts, + isOptional: isOptional + }; + } + + function createSignatureHelpParameterForTypeParameter(typeParameter: TypeParameter): SignatureHelpParameter { + var displayParts = mapToDisplayParts(writer => + typeInfoResolver.getSymbolDisplayBuilder().buildTypeParameterDisplay(typeParameter, writer, argumentListOrTypeArgumentList)); + + return { + name: typeParameter.symbol.name, + documentation: emptyArray, + displayParts: displayParts, + isOptional: false + }; + } } } } \ No newline at end of file From 1b8814cdcd86c732da9fb95f0b0f73146c4f1764 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Mon, 20 Oct 2014 11:51:45 -0700 Subject: [PATCH 8/8] Address PR feedback --- src/compiler/checker.ts | 26 ++++++++++++++++---------- src/compiler/parser.ts | 5 +++++ src/services/signatureHelp.ts | 1 + 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 0c210421232..84ce765c014 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -993,9 +993,11 @@ module ts { // This is for caching the result of getSymbolDisplayBuilder. Do not access directly. var _displayBuilder: SymbolDisplayBuilder; function getSymbolDisplayBuilder(): SymbolDisplayBuilder { - // Enclosing declaration is optional when we don't want to get qualified name in the enclosing declaration scope - // Meaning needs to be specified if the enclosing declaration is given - function buildPlainSymbolName(symbol: Symbol, writer: SymbolWriter): void { + /** + * Writes only the name of the symbol out to the writer. Uses the original source text + * for the name of the symbol if it is available to match how the user inputted the name. + */ + function appendSymbolNameOnly(symbol: Symbol, writer: SymbolWriter): void { if (symbol.declarations && symbol.declarations.length > 0) { var declaration = symbol.declarations[0]; if (declaration.name) { @@ -1007,9 +1009,13 @@ module ts { writer.writeSymbol(symbol.name, symbol); } + /** + * Enclosing declaration is optional when we don't want to get qualified name in the enclosing declaration scope + * Meaning needs to be specified if the enclosing declaration is given + */ function buildSymbolDisplay(symbol: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, meaning?: SymbolFlags, flags?: SymbolFormatFlags): void { var parentSymbol: Symbol; - function appendSymbolName(symbol: Symbol): void { + function appendParentTypeArgumentsAndSymbolName(symbol: Symbol): void { if (parentSymbol) { // Write type arguments of instantiated class/interface here if (flags & SymbolFormatFlags.WriteTypeParametersOrArguments) { @@ -1024,7 +1030,7 @@ module ts { writePunctuation(writer, SyntaxKind.DotToken); } parentSymbol = symbol; - buildPlainSymbolName(symbol, writer); + appendSymbolNameOnly(symbol, writer); } // Let the writer know we just wrote out a symbol. The declaration emitter writer uses @@ -1050,7 +1056,7 @@ module ts { if (accessibleSymbolChain) { for (var i = 0, n = accessibleSymbolChain.length; i < n; i++) { - appendSymbolName(accessibleSymbolChain[i]); + appendParentTypeArgumentsAndSymbolName(accessibleSymbolChain[i]); } } else { @@ -1064,7 +1070,7 @@ module ts { return; } - appendSymbolName(symbol); + appendParentTypeArgumentsAndSymbolName(symbol); } } } @@ -1078,7 +1084,7 @@ module ts { return; } - return appendSymbolName(symbol); + return appendParentTypeArgumentsAndSymbolName(symbol); } function buildTypeDisplay(type: Type, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { @@ -1314,7 +1320,7 @@ module ts { } function buildTypeParameterDisplay(tp: TypeParameter, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, typeStack?: Type[]) { - buildPlainSymbolName(tp.symbol, writer); + appendSymbolNameOnly(tp.symbol, writer); var constraint = getConstraintOfTypeParameter(tp); if (constraint) { writeSpace(writer); @@ -1328,7 +1334,7 @@ module ts { if (getDeclarationFlagsFromSymbol(p) & NodeFlags.Rest) { writePunctuation(writer, SyntaxKind.DotDotDotToken); } - buildPlainSymbolName(p, writer); + appendSymbolNameOnly(p, writer); if (p.valueDeclaration.flags & NodeFlags.QuestionMark || (p.valueDeclaration).initializer) { writePunctuation(writer, SyntaxKind.QuestionToken); } diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index f71af45c7de..18e114750dd 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -2344,6 +2344,11 @@ module ts { function parseTypeArguments(): NodeArray { var typeArgumentListStart = scanner.getTokenPos(); var errorCountBeforeTypeParameterList = file.syntacticErrors.length; + // We pass parseSingleTypeArgument instead of parseType as the element parser + // because parseSingleTypeArgument knows how to parse a missing type argument. + // This is useful for signature help. parseType has the disadvantage that when + // it sees a missing type, it changes the LookAheadMode to Error, and the result + // is a broken binary expression, which breaks signature help. var result = parseBracketedList(ParsingContext.TypeArguments, parseSingleTypeArgument, SyntaxKind.LessThanToken, SyntaxKind.GreaterThanToken); if (!result.length && file.syntacticErrors.length === errorCountBeforeTypeParameterList) { grammarErrorAtPos(typeArgumentListStart, scanner.getStartPos() - typeArgumentListStart, Diagnostics.Type_argument_list_cannot_be_empty); diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 26041dbd3e0..86e378abb45 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -261,6 +261,7 @@ module ts.SignatureHelp { function getChildListThatStartsWithOpenerToken(parent: Node, openerToken: Node, sourceFile: SourceFile): Node { var children = parent.getChildren(sourceFile); var indexOfOpenerToken = children.indexOf(openerToken); + Debug.assert(indexOfOpenerToken >= 0 && children.length > indexOfOpenerToken + 1); return children[indexOfOpenerToken + 1]; }