goToDefinition: Use the name of a declaration (if possible) when creating DefinitionInfo

src/harness/fourslash.ts

@@ -47,9 +47,9 @@ namespace FourSlash {
         /**
          * Inserted in source files by surrounding desired text
          * in a range with `[|` and `|]`. For example,
-         * 
+         *
          * [|text in range|]
-         * 
+         *
          * is a range with `text in range` "selected".
          */
         ranges: Range[];
@@ -540,53 +540,66 @@ namespace FourSlash {
         }
 
         public verifyGoToDefinitionIs(endMarker: string | string[]) {
-            this.verifyGoToDefinitionWorker(endMarker instanceof Array ? endMarker : [endMarker]);
+            this.verifyGoToXWorker(endMarker instanceof Array ? endMarker : [endMarker], () => this.getGoToDefinition());
         }
 
         public verifyGoToDefinition(arg0: any, endMarkerNames?: string | string[]) {
+            this.verifyGoToX(arg0, endMarkerNames, () => this.getGoToDefinition());
+        }
+
+        private getGoToDefinition(): ts.DefinitionInfo[] {
+            return this.languageService.getDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition)
+        }
+
+        public verifyGoToType(arg0: any, endMarkerNames?: string | string[]) {
+            this.verifyGoToX(arg0, endMarkerNames, () =>
+                this.languageService.getTypeDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition));
+        }
+
+        private verifyGoToX(arg0: any, endMarkerNames: string | string[] | undefined, getDefs: () => ts.DefinitionInfo[] | undefined) {
             if (endMarkerNames) {
-                this.verifyGoToDefinitionPlain(arg0, endMarkerNames);
+                this.verifyGoToXPlain(arg0, endMarkerNames, getDefs);
             }
             else if (arg0 instanceof Array) {
                 const pairs: [string | string[], string | string[]][] = arg0;
                 for (const [start, end] of pairs) {
-                    this.verifyGoToDefinitionPlain(start, end);
+                    this.verifyGoToXPlain(start, end, getDefs);
                 }
             }
             else {
                 const obj: { [startMarkerName: string]: string | string[] } = arg0;
                 for (const startMarkerName in obj) {
                     if (ts.hasProperty(obj, startMarkerName)) {
-                        this.verifyGoToDefinitionPlain(startMarkerName, obj[startMarkerName]);
+                        this.verifyGoToXPlain(startMarkerName, obj[startMarkerName], getDefs);
                     }
                 }
             }
         }
 
-        private verifyGoToDefinitionPlain(startMarkerNames: string | string[], endMarkerNames: string | string[]) {
+        private verifyGoToXPlain(startMarkerNames: string | string[], endMarkerNames: string | string[], getDefs: () => ts.DefinitionInfo[] | undefined) {
             if (startMarkerNames instanceof Array) {
                 for (const start of startMarkerNames) {
-                    this.verifyGoToDefinitionSingle(start, endMarkerNames);
+                    this.verifyGoToXSingle(start, endMarkerNames, getDefs);
                 }
             }
             else {
-                this.verifyGoToDefinitionSingle(startMarkerNames, endMarkerNames);
+                this.verifyGoToXSingle(startMarkerNames, endMarkerNames, getDefs);
             }
         }
 
         public verifyGoToDefinitionForMarkers(markerNames: string[]) {
             for (const markerName of markerNames) {
-                this.verifyGoToDefinitionSingle(`${markerName}Reference`, `${markerName}Definition`);
+                this.verifyGoToXSingle(`${markerName}Reference`, `${markerName}Definition`, () => this.getGoToDefinition());
             }
         }
 
-        private verifyGoToDefinitionSingle(startMarkerName: string, endMarkerNames: string | string[]) {
+        private verifyGoToXSingle(startMarkerName: string, endMarkerNames: string | string[], getDefs: () => ts.DefinitionInfo[] | undefined) {
             this.goToMarker(startMarkerName);
-            this.verifyGoToDefinitionWorker(endMarkerNames instanceof Array ? endMarkerNames : [endMarkerNames]);
+            this.verifyGoToXWorker(endMarkerNames instanceof Array ? endMarkerNames : [endMarkerNames], getDefs);
         }
 
-        private verifyGoToDefinitionWorker(endMarkers: string[]) {
-            const definitions = this.languageService.getDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition) || [];
+        private verifyGoToXWorker(endMarkers: string[], getDefs: () => ts.DefinitionInfo[] | undefined) {
+            const definitions = getDefs() || [];
 
             if (endMarkers.length !== definitions.length) {
                 this.raiseError(`goToDefinitions failed - expected to find ${endMarkers.length} definitions but got ${definitions.length}`);
@@ -1987,7 +2000,7 @@ namespace FourSlash {
          * Compares expected text to the text that would be in the sole range
          * (ie: [|...|]) in the file after applying the codefix sole codefix
          * in the source file.
-         * 
+         *
          * Because codefixes are only applied on the working file, it is unsafe
          * to apply this more than once (consider a refactoring across files).
          */
@@ -3042,10 +3055,6 @@ namespace FourSlashInterface {
             this.state.goToEOF();
         }
 
-        public type(definitionIndex = 0) {
-            this.state.goToTypeDefinition(definitionIndex);
-        }
-
         public implementation() {
             this.state.goToImplementation();
         }
@@ -3213,6 +3222,13 @@ namespace FourSlashInterface {
             this.state.verifyGoToDefinition(arg0, endMarkerName);
         }
 
+        public goToType(startMarkerName: string | string[], endMarkerName: string | string[]): void;
+        public goToType(startsAndEnds: [string | string[], string | string[]][]): void;
+        public goToType(startsAndEnds: { [startMarkerName: string]: string | string[] }): void;
+        public goToType(arg0: any, endMarkerName?: string | string[]) {
+            this.state.verifyGoToType(arg0, endMarkerName);
+        }
+
         public goToDefinitionForMarkers(...markerNames: string[]) {
             this.state.verifyGoToDefinitionForMarkers(markerNames);
         }

src/services/findAllReferences.ts

@@ -325,7 +325,7 @@ namespace ts.FindAllReferences {
                 fileName: targetLabel.getSourceFile().fileName,
                 kind: ScriptElementKind.label,
                 name: labelName,
-                textSpan: createTextSpanFromBounds(targetLabel.getStart(), targetLabel.getEnd()),
+                textSpan: createTextSpanFromNode(targetLabel, sourceFile),
                 displayParts: [displayPart(labelName, SymbolDisplayPartKind.text)]
             };
 
@@ -871,7 +871,7 @@ namespace ts.FindAllReferences {
                     fileName: node.getSourceFile().fileName,
                     kind: ScriptElementKind.variableElement,
                     name: "this",
-                    textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()),
+                    textSpan: createTextSpanFromNode(node),
                     displayParts
                 },
                 references: references
@@ -942,7 +942,7 @@ namespace ts.FindAllReferences {
                     fileName: node.getSourceFile().fileName,
                     kind: ScriptElementKind.variableElement,
                     name: type.text,
-                    textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()),
+                    textSpan: createTextSpanFromNode(node),
                     displayParts: [displayPart(getTextOfNode(node), SymbolDisplayPartKind.stringLiteral)]
                 },
                 references: references

src/services/goToDefinition.ts

@@ -15,10 +15,8 @@ namespace ts.GoToDefinition {
         const typeReferenceDirective = findReferenceInPosition(sourceFile.typeReferenceDirectives, position);
         if (typeReferenceDirective) {
             const referenceFile = program.getResolvedTypeReferenceDirectives()[typeReferenceDirective.fileName];
-            if (referenceFile && referenceFile.resolvedFileName) {
-                return [getDefinitionInfoForFileReference(typeReferenceDirective.fileName, referenceFile.resolvedFileName)];
-            }
-            return undefined;
+            return referenceFile && referenceFile.resolvedFileName &&
+                [getDefinitionInfoForFileReference(typeReferenceDirective.fileName, referenceFile.resolvedFileName)];
         }
 
         const node = getTouchingPropertyName(sourceFile, position);
@@ -30,7 +28,7 @@ namespace ts.GoToDefinition {
         if (isJumpStatementTarget(node)) {
             const labelName = (<Identifier>node).text;
             const label = getTargetLabel((<BreakOrContinueStatement>node.parent), (<Identifier>node).text);
-            return label ? [createDefinitionInfo(label, ScriptElementKind.label, labelName, /*containerName*/ undefined)] : undefined;
+            return label ? [createDefinitionInfoFromName(label, ScriptElementKind.label, labelName, /*containerName*/ undefined)] : undefined;
         }
 
         const typeChecker = program.getTypeChecker();
@@ -171,33 +169,38 @@ namespace ts.GoToDefinition {
 
         function tryAddSignature(signatureDeclarations: Declaration[], selectConstructors: boolean, symbolKind: string, symbolName: string, containerName: string, result: DefinitionInfo[]) {
             const declarations: Declaration[] = [];
-            let definition: Declaration;
+            let definition: Declaration | undefined;
 
-            forEach(signatureDeclarations, d => {
-                if ((selectConstructors && d.kind === SyntaxKind.Constructor) ||
-                    (!selectConstructors && (d.kind === SyntaxKind.FunctionDeclaration || d.kind === SyntaxKind.MethodDeclaration || d.kind === SyntaxKind.MethodSignature))) {
+            for (const d of signatureDeclarations) {
+                if (selectConstructors ? d.kind === SyntaxKind.Constructor : isSignatureDeclaration(d)) {
                     declarations.push(d);
                     if ((<FunctionLikeDeclaration>d).body) definition = d;
                 }
-            });
-
-            if (definition) {
-                result.push(createDefinitionInfo(definition, symbolKind, symbolName, containerName));
-                return true;
             }
-            else if (declarations.length) {
-                result.push(createDefinitionInfo(lastOrUndefined(declarations), symbolKind, symbolName, containerName));
+
+            if (declarations.length) {
+                result.push(createDefinitionInfo(definition || lastOrUndefined(declarations), symbolKind, symbolName, containerName));
                 return true;
             }
-
             return false;
         }
     }
 
-    function createDefinitionInfo(node: Node, symbolKind: string, symbolName: string, containerName: string): DefinitionInfo {
+    function isSignatureDeclaration(node: Node): boolean {
+        return node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.MethodSignature
+    }
+
+    /** Creates a DefinitionInfo from a Declaration, using the declaration's name if possible. */
+    function createDefinitionInfo(node: Declaration, symbolKind: string, symbolName: string, containerName: string): DefinitionInfo {
+        return createDefinitionInfoFromName(node.name || node, symbolKind, symbolName, containerName);
+    }
+
+    /** Creates a DefinitionInfo directly from the name of a declaration. */
+    function createDefinitionInfoFromName(name: Node, symbolKind: string, symbolName: string, containerName: string): DefinitionInfo {
+        const sourceFile = name.getSourceFile();
         return {
-            fileName: node.getSourceFile().fileName,
-            textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()),
+            fileName: sourceFile.fileName,
+            textSpan: createTextSpanFromNode(name, sourceFile),
             kind: symbolKind,
             name: symbolName,
             containerKind: undefined,

src/services/navigateTo.ts

@@ -199,7 +199,7 @@ namespace ts.NavigateTo {
                 matchKind: PatternMatchKind[rawItem.matchKind],
                 isCaseSensitive: rawItem.isCaseSensitive,
                 fileName: rawItem.fileName,
-                textSpan: createTextSpanFromBounds(declaration.getStart(), declaration.getEnd()),
+                textSpan: createTextSpanFromNode(declaration),
                 // TODO(jfreeman): What should be the containerName when the container has a computed name?
                 containerName: container && container.name ? (<Identifier>container.name).text : "",
                 containerKind: container && container.name ? getNodeKind(container) : ""

src/services/navigationBar.ts

@@ -591,7 +591,7 @@ namespace ts.NavigationBar {
     function getNodeSpan(node: Node): TextSpan {
         return node.kind === SyntaxKind.SourceFile
             ? createTextSpanFromBounds(node.getFullStart(), node.getEnd())
-            : createTextSpanFromBounds(node.getStart(curSourceFile), node.getEnd());
+            : createTextSpanFromNode(node, curSourceFile);
     }
 
     function getFunctionOrClassName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string {
@@ -626,15 +626,15 @@ namespace ts.NavigationBar {
 
     /**
      * Matches all whitespace characters in a string. Eg:
-     * 
+     *
      * "app.
-     * 
+     *
      * onactivated"
-     * 
+     *
      * matches because of the newline, whereas
-     * 
+     *
      * "app.onactivated"
-     * 
+     *
      * does not match.
      */
     const whiteSpaceRegex = /\s+/g;

src/services/outliningElementsCollector.ts

@@ -8,7 +8,7 @@ namespace ts.OutliningElementsCollector {
             if (hintSpanNode && startElement && endElement) {
                 const span: OutliningSpan = {
                     textSpan: createTextSpanFromBounds(startElement.pos, endElement.end),
-                    hintSpan: createTextSpanFromBounds(hintSpanNode.getStart(), hintSpanNode.end),
+                    hintSpan: createTextSpanFromNode(hintSpanNode, sourceFile),
                     bannerText: collapseText,
                     autoCollapse: autoCollapse
                 };
@@ -135,7 +135,7 @@ namespace ts.OutliningElementsCollector {
 
                         // Block was a standalone block.  In this case we want to only collapse
                         // the span of the block, independent of any parent span.
-                        const span = createTextSpanFromBounds(n.getStart(), n.end);
+                        const span = createTextSpanFromNode(n);
                         elements.push({
                             textSpan: span,
                             hintSpan: span,

src/services/utilities.ts

@@ -1112,6 +1112,10 @@ namespace ts {
             return !tripleSlashDirectivePrefixRegex.test(commentText);
         }
     }
+
+    export function createTextSpanFromNode(node: Node, sourceFile?: SourceFile): TextSpan {
+        return createTextSpanFromBounds(node.getStart(sourceFile), node.getEnd());
+    }
 }
 
 // Display-part writer helpers

tests/cases/fourslash/ambientShorthandGotoDefinition.ts

@@ -1,13 +1,13 @@
 /// <reference path='fourslash.ts' />
 
 // @Filename: declarations.d.ts
-/////*module*/declare module "jquery"
+////declare module /*module*/"jquery"
 
 // @Filename: user.ts
 ///////<reference path="declarations.d.ts"/>
 ////import /*importFoo*/foo, {bar} from "jquery";
-////import /*importBaz*/* as /*idBaz*/baz from "jquery";
-/////*importBang*/import /*idBang*/bang = require("jquery");
+////import * as /*importBaz*/baz from "jquery";
+////import /*importBang*/bang = require("jquery");
 ////foo/*useFoo*/(bar/*useBar*/, baz/*useBaz*/, bang/*useBang*/);
 
 verify.quickInfoAt("useFoo", "import foo");
@@ -22,11 +22,11 @@ verify.goToDefinition("useBar", "module");
 verify.quickInfoAt("useBaz", "import baz");
 verify.goToDefinition({
     useBaz: "importBaz",
-    idBaz: "module"
+    importBaz: "module"
 });
 
 verify.quickInfoAt("useBang", "import bang = require(\"jquery\")");
 verify.goToDefinition({
     useBang: "importBang",
-    idBang: "module"
+    importBang: "module"
 });

tests/cases/fourslash/fourslash.ts

@@ -110,7 +110,6 @@ declare namespace FourSlashInterface {
         marker(name?: string): void;
         bof(): void;
         eof(): void;
-        type(definitionIndex?: number): void;
         implementation(): void;
         position(position: number, fileIndex?: number): any;
         position(position: number, fileName?: string): any;
@@ -165,6 +164,8 @@ declare namespace FourSlashInterface {
         goToDefinition(startsAndEnds: { [startMarkerName: string]: string | string[] }): void;
         /** Verifies goToDefinition for each `${markerName}Reference` -> `${markerName}Definition` */
         goToDefinitionForMarkers(...markerNames: string[]): void;
+        goToType(startsAndEnds: { [startMarkerName: string]: string | string[] }): void;
+        goToType(startMarkerNames: string | string[], endMarkerNames: string | string[]): void;
         verifyGetEmitOutputForCurrentFile(expected: string): void;
         verifyGetEmitOutputContentsForCurrentFile(expected: ts.OutputFile[]): void;
         /**

tests/cases/fourslash/goToDefinitionAlias.ts

@@ -1,9 +1,9 @@
 /// <reference path='fourslash.ts' />
 
 // @Filename: b.ts
-/////*alias1Definition*/import alias1 = require("fileb");
+////import /*alias1Definition*/alias1 = require("fileb");
 ////module Module {
-////    /*alias2Definition*/export import alias2 = alias1;
+////    export import /*alias2Definition*/alias2 = alias1;
 ////}
 ////
 ////// Type position

tests/cases/fourslash/goToDefinitionAmbiants.ts

@@ -1,11 +1,11 @@
 /// <reference path='fourslash.ts' />
 
 ////declare var /*ambientVariableDefinition*/ambientVar;
-/////*ambientFunctionDefinition*/declare function ambientFunction();
+////declare function /*ambientFunctionDefinition*/ambientFunction();
 ////declare class ambientClass {
 ////    /*constructorDefinition*/constructor();
-////    /*staticMethodDefinition*/static method();
-////    /*instanceMethodDefinition*/public method();
+////    static /*staticMethodDefinition*/method();
+////    public /*instanceMethodDefinition*/method();
 ////}
 ////
 /////*ambientVariableReference*/ambientVar = 1;

tests/cases/fourslash/goToDefinitionDecorator.ts

@@ -9,10 +9,10 @@
 
 
 // @Filename: a.ts
-/////*decoratorDefinition*/function decorator(target) {
+////function /*decoratorDefinition*/decorator(target) {
 ////    return target;
 ////}
-/////*decoratorFactoryDefinition*/function decoratorFactory(...args) {
+////function /*decoratorFactoryDefinition*/decoratorFactory(...args) {
 ////    return target => target;
 ////}
 

tests/cases/fourslash/goToDeclarationDecoratorOverloads.ts → tests/cases/fourslash/goToDefinitionDecoratorOverloads.ts

@@ -3,8 +3,8 @@
 
 ////async function f() {}
 ////
-/////*defDecString*/function dec(target: any, propertyKey: string): void;
-/////*defDecSymbol*/function dec(target: any, propertyKey: symbol): void;
+////function /*defDecString*/dec(target: any, propertyKey: string): void;
+////function /*defDecSymbol*/dec(target: any, propertyKey: symbol): void;
 ////function dec(target: any, propertyKey: string | symbol) {}
 ////
 ////declare const s: symbol;