From 4514f8fde52f1d58722e961339ceec43cf3e853d Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 29 Aug 2016 10:14:59 -0700 Subject: [PATCH 1/5] Make goto-definition go to a signature declaration if possible --- src/harness/fourslash.ts | 6 +- src/services/services.ts | 58 +++++++++++++++---- .../goToDefinitionConstructorOverloads.ts | 4 +- .../goToDefinitionFunctionOverloads.ts | 4 +- .../goToDefinitionMethodOverloads.ts | 22 +++---- tests/cases/fourslash/goToDefinition_super.ts | 33 +++++++++++ 6 files changed, 98 insertions(+), 29 deletions(-) create mode 100644 tests/cases/fourslash/goToDefinition_super.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 63731417f48..408c52ef5f5 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1546,7 +1546,7 @@ namespace FourSlash { public goToDefinition(definitionIndex: number) { const definitions = this.languageService.getDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition); if (!definitions || !definitions.length) { - this.raiseError("goToDefinition failed - expected to at least one definition location but got 0"); + this.raiseError("goToDefinition failed - expected to find at least one definition location but got 0"); } if (definitionIndex >= definitions.length) { @@ -1561,7 +1561,7 @@ namespace FourSlash { public goToTypeDefinition(definitionIndex: number) { const definitions = this.languageService.getTypeDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition); if (!definitions || !definitions.length) { - this.raiseError("goToTypeDefinition failed - expected to at least one definition location but got 0"); + this.raiseError("goToTypeDefinition failed - expected to find at least one definition location but got 0"); } if (definitionIndex >= definitions.length) { @@ -1582,7 +1582,7 @@ namespace FourSlash { this.raiseError(`goToDefinition - expected to 0 definition locations but got ${definitions.length}`); } else if (!foundDefinitions && !negative) { - this.raiseError("goToDefinition - expected to at least one definition location but got 0"); + this.raiseError("goToDefinition - expected to find at least one definition location but got 0"); } } diff --git a/src/services/services.ts b/src/services/services.ts index a55f2cf61b0..29cbcfd59d5 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2788,18 +2788,37 @@ namespace ts { return node && node.parent && node.parent.kind === SyntaxKind.PropertyAccessExpression && (node.parent).name === node; } + function climbPastPropertyAccess(node: Node) { + return isRightSideOfPropertyAccess(node) ? node.parent : node; + } + function isCallExpressionTarget(node: Node): boolean { - if (isRightSideOfPropertyAccess(node)) { - node = node.parent; - } - return node && node.parent && node.parent.kind === SyntaxKind.CallExpression && (node.parent).expression === node; + return !!getCallOrNewExpressionWorker(node, SyntaxKind.CallExpression); } function isNewExpressionTarget(node: Node): boolean { - if (isRightSideOfPropertyAccess(node)) { - node = node.parent; + return !!getCallOrNewExpressionWorker(node, SyntaxKind.NewExpression); + } + + function getCallOrNewExpressionTargetingNode(node: Node): CallExpression | NewExpression | undefined { + return getCallOrNewExpressionWorker(node, SyntaxKind.CallExpression) || getCallOrNewExpressionWorker(node, SyntaxKind.NewExpression); + } + + function tryGetCalledDeclaration(typeChecker: TypeChecker, node: Node): SignatureDeclaration | undefined { + const callOrNewExpression = getCallOrNewExpressionTargetingNode(node); + if (callOrNewExpression) { + const signature = typeChecker.getResolvedSignature(callOrNewExpression); + return signature.declaration; } - return node && node.parent && node.parent.kind === SyntaxKind.NewExpression && (node.parent).expression === node; + } + + function getCallOrNewExpressionWorker(node: Node, kind: SyntaxKind): Node | undefined { + const target = climbPastPropertyAccess(node); + return target && + target.parent && + target.parent.kind === kind && + (target.parent).expression === target && + target.parent; } function isNameOfModuleDeclaration(node: Node) { @@ -5068,14 +5087,25 @@ namespace ts { }; } + function getSymbolInfo(typeChecker: TypeChecker, symbol: Symbol, node: Node) { + return { + symbolName: typeChecker.symbolToString(symbol), // Do not get scoped name, just the name of the symbol + symbolKind: getSymbolKind(symbol, node), + containerName: symbol.parent ? typeChecker.symbolToString(symbol.parent, node) : "" + }; + } + + function getDefinitionFromSignatureDeclaration(decl: SignatureDeclaration): DefinitionInfo { + const typeChecker = program.getTypeChecker(); + const { symbolName, symbolKind, containerName } = getSymbolInfo(typeChecker, decl.symbol, decl); + return createDefinitionInfo(decl, symbolKind, symbolName, containerName); + } + function getDefinitionFromSymbol(symbol: Symbol, node: Node): DefinitionInfo[] { const typeChecker = program.getTypeChecker(); const result: DefinitionInfo[] = []; const declarations = symbol.getDeclarations(); - const symbolName = typeChecker.symbolToString(symbol); // Do not get scoped name, just the name of the symbol - const symbolKind = getSymbolKind(symbol, node); - const containerSymbol = symbol.parent; - const containerName = containerSymbol ? typeChecker.symbolToString(containerSymbol, node) : ""; + const { symbolName, symbolKind, containerName } = getSymbolInfo(typeChecker, symbol, node); if (!tryAddConstructSignature(symbol, node, symbolKind, symbolName, containerName, result) && !tryAddCallSignature(symbol, node, symbolKind, symbolName, containerName, result)) { @@ -5201,6 +5231,12 @@ namespace ts { } const typeChecker = program.getTypeChecker(); + + const calledDeclaration = tryGetCalledDeclaration(typeChecker, node); + if (calledDeclaration) { + return [getDefinitionFromSignatureDeclaration(calledDeclaration)]; + } + let symbol = typeChecker.getSymbolAtLocation(node); // Could not find a symbol e.g. node is string or number keyword, diff --git a/tests/cases/fourslash/goToDefinitionConstructorOverloads.ts b/tests/cases/fourslash/goToDefinitionConstructorOverloads.ts index a1fda8886db..d52243a6465 100644 --- a/tests/cases/fourslash/goToDefinitionConstructorOverloads.ts +++ b/tests/cases/fourslash/goToDefinitionConstructorOverloads.ts @@ -11,11 +11,11 @@ goTo.marker('constructorOverloadReference1'); goTo.definition(); -verify.caretAtMarker('constructorDefinition'); +verify.caretAtMarker('constructorOverload1'); goTo.marker('constructorOverloadReference2'); goTo.definition(); -verify.caretAtMarker('constructorDefinition'); +verify.caretAtMarker('constructorOverload2'); goTo.marker('constructorOverload1'); goTo.definition(); diff --git a/tests/cases/fourslash/goToDefinitionFunctionOverloads.ts b/tests/cases/fourslash/goToDefinitionFunctionOverloads.ts index 488d788073a..fb690607f1a 100644 --- a/tests/cases/fourslash/goToDefinitionFunctionOverloads.ts +++ b/tests/cases/fourslash/goToDefinitionFunctionOverloads.ts @@ -9,11 +9,11 @@ goTo.marker('functionOverloadReference1'); goTo.definition(); -verify.caretAtMarker('functionOverloadDefinition'); +verify.caretAtMarker('functionOverload1'); goTo.marker('functionOverloadReference2'); goTo.definition(); -verify.caretAtMarker('functionOverloadDefinition'); +verify.caretAtMarker('functionOverload2'); goTo.marker('functionOverload'); goTo.definition(); diff --git a/tests/cases/fourslash/goToDefinitionMethodOverloads.ts b/tests/cases/fourslash/goToDefinitionMethodOverloads.ts index 6d74881c2af..7b4bd0630d9 100644 --- a/tests/cases/fourslash/goToDefinitionMethodOverloads.ts +++ b/tests/cases/fourslash/goToDefinitionMethodOverloads.ts @@ -1,11 +1,11 @@ /// ////class MethodOverload { -//// static me/*staticMethodOverload1*/thod(); -//// static me/*staticMethodOverload2*/thod(foo: string); -/////*staticMethodDefinition*/static method(foo?: any) { } -//// public met/*instanceMethodOverload1*/hod(): any; -//// public met/*instanceMethodOverload2*/hod(foo: string); +//// /*staticMethodOverload1*/static /*staticMethodOverload1Name*/method(); +//// /*staticMethodOverload2*/static method(foo: string); +//// /*staticMethodDefinition*/static method(foo?: any) { } +//// /*instanceMethodOverload1*/public /*instanceMethodOverload1Name*/method(): any; +//// /*instanceMethodOverload2*/public method(foo: string); /////*instanceMethodDefinition*/public method(foo?: any) { return "foo" } ////} @@ -20,25 +20,25 @@ goTo.marker('staticMethodReference1'); goTo.definition(); -verify.caretAtMarker('staticMethodDefinition'); +verify.caretAtMarker('staticMethodOverload1'); goTo.marker('staticMethodReference2'); goTo.definition(); -verify.caretAtMarker('staticMethodDefinition'); +verify.caretAtMarker('staticMethodOverload2'); goTo.marker('instanceMethodReference1'); goTo.definition(); -verify.caretAtMarker('instanceMethodDefinition'); +verify.caretAtMarker('instanceMethodOverload1'); goTo.marker('instanceMethodReference2'); goTo.definition(); -verify.caretAtMarker('instanceMethodDefinition'); +verify.caretAtMarker('instanceMethodOverload2'); -goTo.marker('staticMethodOverload1'); +goTo.marker('staticMethodOverload1Name'); goTo.definition(); verify.caretAtMarker('staticMethodDefinition'); -goTo.marker('instanceMethodOverload1'); +goTo.marker('instanceMethodOverload1Name'); goTo.definition(); verify.caretAtMarker('instanceMethodDefinition'); diff --git a/tests/cases/fourslash/goToDefinition_super.ts b/tests/cases/fourslash/goToDefinition_super.ts new file mode 100644 index 00000000000..576d3535af0 --- /dev/null +++ b/tests/cases/fourslash/goToDefinition_super.ts @@ -0,0 +1,33 @@ +/// + +////class A { +//// /*ctr*/constructor() {} +//// x() {} +////} +/////*B*/class B extends A {} +////class C extends B { +//// constructor() { +//// /*super*/super(); +//// } +//// method() { +//// /*superExpression*/super.x(); +//// } +////} +////class D { +//// constructor() { +//// /*superBroken*/super(); +//// } +////} + +// Super in call position goes to constructor. +goTo.marker("super"); +goTo.definition(); +verify.caretAtMarker("ctr"); + +// Super in any other position goes to the superclass. +goTo.marker("superExpression"); +goTo.definition(); +verify.caretAtMarker("B"); + +goTo.marker("superBroken"); +verify.definitionCountIs(0); From 59774733d6d70ec0a249993310a78f448c3ae026 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 30 Aug 2016 06:39:13 -0700 Subject: [PATCH 2/5] Support decorators and templates --- src/compiler/utilities.ts | 12 +++++++ src/services/services.ts | 35 ++++++++----------- .../goToDeclarationDecoratorOverloads.ts | 22 ++++++++++++ .../goToDefinitionFunctionOverloads.ts | 9 +++-- .../goToDefinitionTaggedTemplateOverloads.ts | 16 +++++++++ 5 files changed, 71 insertions(+), 23 deletions(-) create mode 100644 tests/cases/fourslash/goToDeclarationDecoratorOverloads.ts create mode 100644 tests/cases/fourslash/goToDefinitionTaggedTemplateOverloads.ts diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 0c026fc6b6d..af156c8f5f9 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1035,6 +1035,18 @@ namespace ts { return undefined; } + export function isCallLikeExpression(node: Node): node is CallLikeExpression { + switch (node.kind) { + case SyntaxKind.CallExpression: + case SyntaxKind.NewExpression: + case SyntaxKind.TaggedTemplateExpression: + case SyntaxKind.Decorator: + return true; + default: + return false; + } + } + export function getInvokedExpression(node: CallLikeExpression): Expression { if (node.kind === SyntaxKind.TaggedTemplateExpression) { return (node).tag; diff --git a/src/services/services.ts b/src/services/services.ts index 29cbcfd59d5..437289e376c 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2793,32 +2793,25 @@ namespace ts { } function isCallExpressionTarget(node: Node): boolean { - return !!getCallOrNewExpressionWorker(node, SyntaxKind.CallExpression); + node = climbPastPropertyAccess(node); + return node && node.parent && node.parent.kind === SyntaxKind.CallExpression && (node.parent).expression === node; } function isNewExpressionTarget(node: Node): boolean { - return !!getCallOrNewExpressionWorker(node, SyntaxKind.NewExpression); + node = climbPastPropertyAccess(node); + return node && node.parent && node.parent.kind === SyntaxKind.NewExpression && (node.parent).expression === node; } - function getCallOrNewExpressionTargetingNode(node: Node): CallExpression | NewExpression | undefined { - return getCallOrNewExpressionWorker(node, SyntaxKind.CallExpression) || getCallOrNewExpressionWorker(node, SyntaxKind.NewExpression); - } - - function tryGetCalledDeclaration(typeChecker: TypeChecker, node: Node): SignatureDeclaration | undefined { - const callOrNewExpression = getCallOrNewExpressionTargetingNode(node); - if (callOrNewExpression) { - const signature = typeChecker.getResolvedSignature(callOrNewExpression); - return signature.declaration; - } - } - - function getCallOrNewExpressionWorker(node: Node, kind: SyntaxKind): Node | undefined { + /** Returns a CallLikeExpression where `node` is the target being invoked. */ + function getAncestorCallLikeExpression(node: Node): CallLikeExpression | undefined { const target = climbPastPropertyAccess(node); - return target && - target.parent && - target.parent.kind === kind && - (target.parent).expression === target && - target.parent; + const callLike = target.parent; + return isCallLikeExpression(callLike) && getInvokedExpression(callLike) === target && callLike; + } + + function tryGetSignatureDeclaration(typeChecker: TypeChecker, node: Node): SignatureDeclaration | undefined { + const callLike = getAncestorCallLikeExpression(node); + return callLike && typeChecker.getResolvedSignature(callLike).declaration; } function isNameOfModuleDeclaration(node: Node) { @@ -5232,7 +5225,7 @@ namespace ts { const typeChecker = program.getTypeChecker(); - const calledDeclaration = tryGetCalledDeclaration(typeChecker, node); + const calledDeclaration = tryGetSignatureDeclaration(typeChecker, node); if (calledDeclaration) { return [getDefinitionFromSignatureDeclaration(calledDeclaration)]; } diff --git a/tests/cases/fourslash/goToDeclarationDecoratorOverloads.ts b/tests/cases/fourslash/goToDeclarationDecoratorOverloads.ts new file mode 100644 index 00000000000..ce012d53aba --- /dev/null +++ b/tests/cases/fourslash/goToDeclarationDecoratorOverloads.ts @@ -0,0 +1,22 @@ +// @Target: ES6 +// @experimentaldecorators: true + +////async function f() {} +//// +/////*defDecString*/function dec(target: any, propertyKey: string): void; +/////*defDecSymbol*/function dec(target: any, propertyKey: symbol): void; +////function dec(target: any, propertyKey: string | symbol) {} +//// +////declare const s: symbol; +////class C { +//// @/*useDecString*/dec f() {} +//// @/*useDecSymbol*/dec [s]() {} +////} + +goTo.marker("useDecString"); +goTo.definition(); +verify.caretAtMarker("defDecString"); + +goTo.marker("useDecSymbol"); +goTo.definition(); +verify.caretAtMarker("defDecSymbol"); diff --git a/tests/cases/fourslash/goToDefinitionFunctionOverloads.ts b/tests/cases/fourslash/goToDefinitionFunctionOverloads.ts index fb690607f1a..4d1c09efd7a 100644 --- a/tests/cases/fourslash/goToDefinitionFunctionOverloads.ts +++ b/tests/cases/fourslash/goToDefinitionFunctionOverloads.ts @@ -1,11 +1,12 @@ /// -/////*functionOverload1*/function /*functionOverload*/functionOverload(); +/////*functionOverload1*/function /*functionOverload*/functionOverload(value: number); /////*functionOverload2*/function functionOverload(value: string); /////*functionOverloadDefinition*/function functionOverload() {} //// -/////*functionOverloadReference1*/functionOverload(); +/////*functionOverloadReference1*/functionOverload(123); /////*functionOverloadReference2*/functionOverload("123"); +/////*brokenOverload*/functionOverload({}); goTo.marker('functionOverloadReference1'); goTo.definition(); @@ -15,6 +16,10 @@ goTo.marker('functionOverloadReference2'); goTo.definition(); verify.caretAtMarker('functionOverload2'); +goTo.marker('brokenOverload'); +goTo.definition(); +verify.caretAtMarker('functionOverload1'); + goTo.marker('functionOverload'); goTo.definition(); verify.caretAtMarker('functionOverloadDefinition'); diff --git a/tests/cases/fourslash/goToDefinitionTaggedTemplateOverloads.ts b/tests/cases/fourslash/goToDefinitionTaggedTemplateOverloads.ts new file mode 100644 index 00000000000..54a2039b0a1 --- /dev/null +++ b/tests/cases/fourslash/goToDefinitionTaggedTemplateOverloads.ts @@ -0,0 +1,16 @@ +/// + +/////*defFNumber*/function f(strs: TemplateStringsArray, x: number): void; +/////*defFBool*/function f(strs: TemplateStringsArray, x: boolean): void; +////function f(strs: TemplateStringsArray, x: number | boolean) {} +//// +/////*useFNumber*/f`${0}`; +/////*useFBool*/f`${false}`; + +goTo.marker("useFNumber"); +goTo.definition(); +verify.caretAtMarker("defFNumber"); + +goTo.marker("useFBool"); +goTo.definition(); +verify.caretAtMarker("defFBool"); From efc7e9db6f3c7e74d6aa974888a084dd7f6bc42b Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 30 Aug 2016 06:51:52 -0700 Subject: [PATCH 3/5] Climb past multiple property accesses if necessary --- src/services/services.ts | 6 +++++- ...initionOverloadsInMultiplePropertyAccesses.ts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/goToDefinitionOverloadsInMultiplePropertyAccesses.ts diff --git a/src/services/services.ts b/src/services/services.ts index 437289e376c..084d372da1a 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2792,6 +2792,10 @@ namespace ts { return isRightSideOfPropertyAccess(node) ? node.parent : node; } + function climbPastManyPropertyAccesses(node: Node): Node { + return isRightSideOfPropertyAccess(node) ? climbPastManyPropertyAccesses(node.parent) : node; + } + function isCallExpressionTarget(node: Node): boolean { node = climbPastPropertyAccess(node); return node && node.parent && node.parent.kind === SyntaxKind.CallExpression && (node.parent).expression === node; @@ -2804,7 +2808,7 @@ namespace ts { /** Returns a CallLikeExpression where `node` is the target being invoked. */ function getAncestorCallLikeExpression(node: Node): CallLikeExpression | undefined { - const target = climbPastPropertyAccess(node); + const target = climbPastManyPropertyAccesses(node); const callLike = target.parent; return isCallLikeExpression(callLike) && getInvokedExpression(callLike) === target && callLike; } diff --git a/tests/cases/fourslash/goToDefinitionOverloadsInMultiplePropertyAccesses.ts b/tests/cases/fourslash/goToDefinitionOverloadsInMultiplePropertyAccesses.ts new file mode 100644 index 00000000000..de15c633e5f --- /dev/null +++ b/tests/cases/fourslash/goToDefinitionOverloadsInMultiplePropertyAccesses.ts @@ -0,0 +1,16 @@ +/// + +// Test that we can climb past more than one property access to reach a call expression. + +////namespace A { +//// export namespace B { +//// export function f(value: number): void; +//// /*1*/export function f(value: string): void; +//// export function f(value: number | string) {} +//// } +////} +////A.B./*2*/f(""); + +goTo.marker("2"); +goTo.definition(); +verify.caretAtMarker("1"); From e8e7ec6c626505ef77b77edbccef786cce918f73 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 1 Sep 2016 07:23:43 -0700 Subject: [PATCH 4/5] Remember to check for existence of `target.parent` --- src/services/services.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/services.ts b/src/services/services.ts index 084d372da1a..e7fb0d9bcc5 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2810,7 +2810,7 @@ namespace ts { function getAncestorCallLikeExpression(node: Node): CallLikeExpression | undefined { const target = climbPastManyPropertyAccesses(node); const callLike = target.parent; - return isCallLikeExpression(callLike) && getInvokedExpression(callLike) === target && callLike; + return callLike && isCallLikeExpression(callLike) && getInvokedExpression(callLike) === target && callLike; } function tryGetSignatureDeclaration(typeChecker: TypeChecker, node: Node): SignatureDeclaration | undefined { From 3eadbf6c962427a7761252bcbad353a23e3ba543 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 1 Sep 2016 12:44:54 -0700 Subject: [PATCH 5/5] Rename function --- src/services/services.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index e7fb0d9bcc5..83a21926599 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5092,7 +5092,7 @@ namespace ts { }; } - function getDefinitionFromSignatureDeclaration(decl: SignatureDeclaration): DefinitionInfo { + function createDefinitionFromSignatureDeclaration(decl: SignatureDeclaration): DefinitionInfo { const typeChecker = program.getTypeChecker(); const { symbolName, symbolKind, containerName } = getSymbolInfo(typeChecker, decl.symbol, decl); return createDefinitionInfo(decl, symbolKind, symbolName, containerName); @@ -5231,7 +5231,7 @@ namespace ts { const calledDeclaration = tryGetSignatureDeclaration(typeChecker, node); if (calledDeclaration) { - return [getDefinitionFromSignatureDeclaration(calledDeclaration)]; + return [createDefinitionFromSignatureDeclaration(calledDeclaration)]; } let symbol = typeChecker.getSymbolAtLocation(node);