Skip to content

Commit 2124fcf

Browse files
author
Andy Hanson
committed
goToDefinition: Use the name of a declaration (if possible) when creating DefinitionInfo.
1 parent c4a80b2 commit 2124fcf

61 files changed

Lines changed: 206 additions & 209 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/compiler/utilities.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4318,6 +4318,10 @@ namespace ts {
43184318
return createTextSpan(start, end - start);
43194319
}
43204320

4321+
export function createTextSpanFromNode(node: Node, sourceFile?: SourceFile): TextSpan {
4322+
return createTextSpanFromBounds(node.getStart(sourceFile), node.getEnd());
4323+
}
4324+
43214325
export function textChangeRangeNewSpan(range: TextChangeRange) {
43224326
return createTextSpan(range.span.start, range.newLength);
43234327
}

‎src/harness/fourslash.ts‎

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ namespace FourSlash {
4747
/**
4848
* Inserted in source files by surrounding desired text
4949
* in a range with `[|` and `|]`. For example,
50-
*
50+
*
5151
* [|text in range|]
52-
*
52+
*
5353
* is a range with `text in range` "selected".
5454
*/
5555
ranges: Range[];
@@ -540,53 +540,66 @@ namespace FourSlash {
540540
}
541541

542542
public verifyGoToDefinitionIs(endMarker: string | string[]) {
543-
this.verifyGoToDefinitionWorker(endMarker instanceof Array ? endMarker : [endMarker]);
543+
this.verifyGoToXWorker(endMarker instanceof Array ? endMarker : [endMarker], () => this.getGoToDefinition());
544544
}
545545

546546
public verifyGoToDefinition(arg0: any, endMarkerNames?: string | string[]) {
547+
this.verifyGoToX(arg0, endMarkerNames, () => this.getGoToDefinition());
548+
}
549+
550+
private getGoToDefinition(): ts.DefinitionInfo[] {
551+
return this.languageService.getDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition)
552+
}
553+
554+
public verifyGoToType(arg0: any, endMarkerNames?: string | string[]) {
555+
this.verifyGoToX(arg0, endMarkerNames, () =>
556+
this.languageService.getTypeDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition));
557+
}
558+
559+
private verifyGoToX(arg0: any, endMarkerNames: string | string[] | undefined, getDefs: () => ts.DefinitionInfo[] | undefined) {
547560
if (endMarkerNames) {
548-
this.verifyGoToDefinitionPlain(arg0, endMarkerNames);
561+
this.verifyGoToXPlain(arg0, endMarkerNames, getDefs);
549562
}
550563
else if (arg0 instanceof Array) {
551564
const pairs: [string | string[], string | string[]][] = arg0;
552565
for (const [start, end] of pairs) {
553-
this.verifyGoToDefinitionPlain(start, end);
566+
this.verifyGoToXPlain(start, end, getDefs);
554567
}
555568
}
556569
else {
557570
const obj: { [startMarkerName: string]: string | string[] } = arg0;
558571
for (const startMarkerName in obj) {
559572
if (ts.hasProperty(obj, startMarkerName)) {
560-
this.verifyGoToDefinitionPlain(startMarkerName, obj[startMarkerName]);
573+
this.verifyGoToXPlain(startMarkerName, obj[startMarkerName], getDefs);
561574
}
562575
}
563576
}
564577
}
565578

566-
private verifyGoToDefinitionPlain(startMarkerNames: string | string[], endMarkerNames: string | string[]) {
579+
private verifyGoToXPlain(startMarkerNames: string | string[], endMarkerNames: string | string[], getDefs: () => ts.DefinitionInfo[] | undefined) {
567580
if (startMarkerNames instanceof Array) {
568581
for (const start of startMarkerNames) {
569-
this.verifyGoToDefinitionSingle(start, endMarkerNames);
582+
this.verifyGoToXSingle(start, endMarkerNames, getDefs);
570583
}
571584
}
572585
else {
573-
this.verifyGoToDefinitionSingle(startMarkerNames, endMarkerNames);
586+
this.verifyGoToXSingle(startMarkerNames, endMarkerNames, getDefs);
574587
}
575588
}
576589

577590
public verifyGoToDefinitionForMarkers(markerNames: string[]) {
578591
for (const markerName of markerNames) {
579-
this.verifyGoToDefinitionSingle(`${markerName}Reference`, `${markerName}Definition`);
592+
this.verifyGoToXSingle(`${markerName}Reference`, `${markerName}Definition`, () => this.getGoToDefinition());
580593
}
581594
}
582595

583-
private verifyGoToDefinitionSingle(startMarkerName: string, endMarkerNames: string | string[]) {
596+
private verifyGoToXSingle(startMarkerName: string, endMarkerNames: string | string[], getDefs: () => ts.DefinitionInfo[] | undefined) {
584597
this.goToMarker(startMarkerName);
585-
this.verifyGoToDefinitionWorker(endMarkerNames instanceof Array ? endMarkerNames : [endMarkerNames]);
598+
this.verifyGoToXWorker(endMarkerNames instanceof Array ? endMarkerNames : [endMarkerNames], getDefs);
586599
}
587600

588-
private verifyGoToDefinitionWorker(endMarkers: string[]) {
589-
const definitions = this.languageService.getDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition) || [];
601+
private verifyGoToXWorker(endMarkers: string[], getDefs: () => ts.DefinitionInfo[] | undefined) {
602+
const definitions = getDefs() || [];
590603

591604
if (endMarkers.length !== definitions.length) {
592605
this.raiseError(`goToDefinitions failed - expected to find ${endMarkers.length} definitions but got ${definitions.length}`);
@@ -1987,7 +2000,7 @@ namespace FourSlash {
19872000
* Compares expected text to the text that would be in the sole range
19882001
* (ie: [|...|]) in the file after applying the codefix sole codefix
19892002
* in the source file.
1990-
*
2003+
*
19912004
* Because codefixes are only applied on the working file, it is unsafe
19922005
* to apply this more than once (consider a refactoring across files).
19932006
*/
@@ -3042,10 +3055,6 @@ namespace FourSlashInterface {
30423055
this.state.goToEOF();
30433056
}
30443057

3045-
public type(definitionIndex = 0) {
3046-
this.state.goToTypeDefinition(definitionIndex);
3047-
}
3048-
30493058
public implementation() {
30503059
this.state.goToImplementation();
30513060
}
@@ -3213,6 +3222,13 @@ namespace FourSlashInterface {
32133222
this.state.verifyGoToDefinition(arg0, endMarkerName);
32143223
}
32153224

3225+
public goToType(startMarkerName: string | string[], endMarkerName: string | string[]): void;
3226+
public goToType(startsAndEnds: [string | string[], string | string[]][]): void;
3227+
public goToType(startsAndEnds: { [startMarkerName: string]: string | string[] }): void;
3228+
public goToType(arg0: any, endMarkerName?: string | string[]) {
3229+
this.state.verifyGoToType(arg0, endMarkerName);
3230+
}
3231+
32163232
public goToDefinitionForMarkers(...markerNames: string[]) {
32173233
this.state.verifyGoToDefinitionForMarkers(markerNames);
32183234
}

‎src/services/findAllReferences.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ namespace ts.FindAllReferences {
325325
fileName: targetLabel.getSourceFile().fileName,
326326
kind: ScriptElementKind.label,
327327
name: labelName,
328-
textSpan: createTextSpanFromBounds(targetLabel.getStart(), targetLabel.getEnd()),
328+
textSpan: createTextSpanFromNode(targetLabel, sourceFile),
329329
displayParts: [displayPart(labelName, SymbolDisplayPartKind.text)]
330330
};
331331

@@ -871,7 +871,7 @@ namespace ts.FindAllReferences {
871871
fileName: node.getSourceFile().fileName,
872872
kind: ScriptElementKind.variableElement,
873873
name: "this",
874-
textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()),
874+
textSpan: createTextSpanFromNode(node),
875875
displayParts
876876
},
877877
references: references
@@ -942,7 +942,7 @@ namespace ts.FindAllReferences {
942942
fileName: node.getSourceFile().fileName,
943943
kind: ScriptElementKind.variableElement,
944944
name: type.text,
945-
textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()),
945+
textSpan: createTextSpanFromNode(node),
946946
displayParts: [displayPart(getTextOfNode(node), SymbolDisplayPartKind.stringLiteral)]
947947
},
948948
references: references

‎src/services/goToDefinition.ts‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,8 @@ namespace ts.GoToDefinition {
1515
const typeReferenceDirective = findReferenceInPosition(sourceFile.typeReferenceDirectives, position);
1616
if (typeReferenceDirective) {
1717
const referenceFile = program.getResolvedTypeReferenceDirectives()[typeReferenceDirective.fileName];
18-
if (referenceFile && referenceFile.resolvedFileName) {
19-
return [getDefinitionInfoForFileReference(typeReferenceDirective.fileName, referenceFile.resolvedFileName)];
20-
}
21-
return undefined;
18+
return referenceFile && referenceFile.resolvedFileName &&
19+
[getDefinitionInfoForFileReference(typeReferenceDirective.fileName, referenceFile.resolvedFileName)];
2220
}
2321

2422
const node = getTouchingPropertyName(sourceFile, position);
@@ -30,7 +28,7 @@ namespace ts.GoToDefinition {
3028
if (isJumpStatementTarget(node)) {
3129
const labelName = (<Identifier>node).text;
3230
const label = getTargetLabel((<BreakOrContinueStatement>node.parent), (<Identifier>node).text);
33-
return label ? [createDefinitionInfo(label, ScriptElementKind.label, labelName, /*containerName*/ undefined)] : undefined;
31+
return label ? [createDefinitionInfoFromName(label, ScriptElementKind.label, labelName, /*containerName*/ undefined)] : undefined;
3432
}
3533

3634
const typeChecker = program.getTypeChecker();
@@ -171,33 +169,38 @@ namespace ts.GoToDefinition {
171169

172170
function tryAddSignature(signatureDeclarations: Declaration[], selectConstructors: boolean, symbolKind: string, symbolName: string, containerName: string, result: DefinitionInfo[]) {
173171
const declarations: Declaration[] = [];
174-
let definition: Declaration;
172+
let definition: Declaration | undefined;
175173

176-
forEach(signatureDeclarations, d => {
177-
if ((selectConstructors && d.kind === SyntaxKind.Constructor) ||
178-
(!selectConstructors && (d.kind === SyntaxKind.FunctionDeclaration || d.kind === SyntaxKind.MethodDeclaration || d.kind === SyntaxKind.MethodSignature))) {
174+
for (const d of signatureDeclarations) {
175+
if (selectConstructors ? d.kind === SyntaxKind.Constructor : isSignatureDeclaration(d)) {
179176
declarations.push(d);
180177
if ((<FunctionLikeDeclaration>d).body) definition = d;
181178
}
182-
});
183-
184-
if (definition) {
185-
result.push(createDefinitionInfo(definition, symbolKind, symbolName, containerName));
186-
return true;
187179
}
188-
else if (declarations.length) {
189-
result.push(createDefinitionInfo(lastOrUndefined(declarations), symbolKind, symbolName, containerName));
180+
181+
if (declarations.length) {
182+
result.push(createDefinitionInfo(definition || lastOrUndefined(declarations), symbolKind, symbolName, containerName));
190183
return true;
191184
}
192-
193185
return false;
194186
}
195187
}
196188

197-
function createDefinitionInfo(node: Node, symbolKind: string, symbolName: string, containerName: string): DefinitionInfo {
189+
function isSignatureDeclaration(node: Node): boolean {
190+
return node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.MethodSignature
191+
}
192+
193+
/** Creates a DefinitionInfo from a Declaration, using the declaration's name if possible. */
194+
function createDefinitionInfo(node: Declaration, symbolKind: string, symbolName: string, containerName: string): DefinitionInfo {
195+
return createDefinitionInfoFromName(node.name || node, symbolKind, symbolName, containerName);
196+
}
197+
198+
/** Creates a DefinitionInfo directly from the name of a declaration. */
199+
function createDefinitionInfoFromName(name: Node, symbolKind: string, symbolName: string, containerName: string): DefinitionInfo {
200+
const sourceFile = name.getSourceFile();
198201
return {
199-
fileName: node.getSourceFile().fileName,
200-
textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()),
202+
fileName: sourceFile.fileName,
203+
textSpan: createTextSpanFromNode(name, sourceFile),
201204
kind: symbolKind,
202205
name: symbolName,
203206
containerKind: undefined,

‎src/services/navigateTo.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ namespace ts.NavigateTo {
199199
matchKind: PatternMatchKind[rawItem.matchKind],
200200
isCaseSensitive: rawItem.isCaseSensitive,
201201
fileName: rawItem.fileName,
202-
textSpan: createTextSpanFromBounds(declaration.getStart(), declaration.getEnd()),
202+
textSpan: createTextSpanFromNode(declaration),
203203
// TODO(jfreeman): What should be the containerName when the container has a computed name?
204204
containerName: container && container.name ? (<Identifier>container.name).text : "",
205205
containerKind: container && container.name ? getNodeKind(container) : ""

‎src/services/navigationBar.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ namespace ts.NavigationBar {
591591
function getNodeSpan(node: Node): TextSpan {
592592
return node.kind === SyntaxKind.SourceFile
593593
? createTextSpanFromBounds(node.getFullStart(), node.getEnd())
594-
: createTextSpanFromBounds(node.getStart(curSourceFile), node.getEnd());
594+
: createTextSpanFromNode(node, curSourceFile);
595595
}
596596

597597
function getFunctionOrClassName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string {
@@ -626,15 +626,15 @@ namespace ts.NavigationBar {
626626

627627
/**
628628
* Matches all whitespace characters in a string. Eg:
629-
*
629+
*
630630
* "app.
631-
*
631+
*
632632
* onactivated"
633-
*
633+
*
634634
* matches because of the newline, whereas
635-
*
635+
*
636636
* "app.onactivated"
637-
*
637+
*
638638
* does not match.
639639
*/
640640
const whiteSpaceRegex = /\s+/g;

‎src/services/outliningElementsCollector.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace ts.OutliningElementsCollector {
88
if (hintSpanNode && startElement && endElement) {
99
const span: OutliningSpan = {
1010
textSpan: createTextSpanFromBounds(startElement.pos, endElement.end),
11-
hintSpan: createTextSpanFromBounds(hintSpanNode.getStart(), hintSpanNode.end),
11+
hintSpan: createTextSpanFromNode(hintSpanNode, sourceFile),
1212
bannerText: collapseText,
1313
autoCollapse: autoCollapse
1414
};
@@ -135,7 +135,7 @@ namespace ts.OutliningElementsCollector {
135135

136136
// Block was a standalone block. In this case we want to only collapse
137137
// the span of the block, independent of any parent span.
138-
const span = createTextSpanFromBounds(n.getStart(), n.end);
138+
const span = createTextSpanFromNode(n);
139139
elements.push({
140140
textSpan: span,
141141
hintSpan: span,

‎tests/cases/fourslash/ambientShorthandGotoDefinition.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
/// <reference path='fourslash.ts' />
22

33
// @Filename: declarations.d.ts
4-
/////*module*/declare module "jquery"
4+
////declare module /*module*/"jquery"
55

66
// @Filename: user.ts
77
///////<reference path="declarations.d.ts"/>
88
////import /*importFoo*/foo, {bar} from "jquery";
9-
////import /*importBaz*/* as /*idBaz*/baz from "jquery";
10-
/////*importBang*/import /*idBang*/bang = require("jquery");
9+
////import * as /*importBaz*/baz from "jquery";
10+
////import /*importBang*/bang = require("jquery");
1111
////foo/*useFoo*/(bar/*useBar*/, baz/*useBaz*/, bang/*useBang*/);
1212

1313
verify.quickInfoAt("useFoo", "import foo");
@@ -22,11 +22,11 @@ verify.goToDefinition("useBar", "module");
2222
verify.quickInfoAt("useBaz", "import baz");
2323
verify.goToDefinition({
2424
useBaz: "importBaz",
25-
idBaz: "module"
25+
importBaz: "module"
2626
});
2727

2828
verify.quickInfoAt("useBang", "import bang = require(\"jquery\")");
2929
verify.goToDefinition({
3030
useBang: "importBang",
31-
idBang: "module"
31+
importBang: "module"
3232
});

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ declare namespace FourSlashInterface {
110110
marker(name?: string): void;
111111
bof(): void;
112112
eof(): void;
113-
type(definitionIndex?: number): void;
114113
implementation(): void;
115114
position(position: number, fileIndex?: number): any;
116115
position(position: number, fileName?: string): any;
@@ -165,6 +164,8 @@ declare namespace FourSlashInterface {
165164
goToDefinition(startsAndEnds: { [startMarkerName: string]: string | string[] }): void;
166165
/** Verifies goToDefinition for each `${markerName}Reference` -> `${markerName}Definition` */
167166
goToDefinitionForMarkers(...markerNames: string[]): void;
167+
goToType(startsAndEnds: { [startMarkerName: string]: string | string[] }): void;
168+
goToType(startMarkerNames: string | string[], endMarkerNames: string | string[]): void;
168169
verifyGetEmitOutputForCurrentFile(expected: string): void;
169170
verifyGetEmitOutputContentsForCurrentFile(expected: ts.OutputFile[]): void;
170171
/**

‎tests/cases/fourslash/goToDefinitionAlias.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
/// <reference path='fourslash.ts' />
22

33
// @Filename: b.ts
4-
/////*alias1Definition*/import alias1 = require("fileb");
4+
////import /*alias1Definition*/alias1 = require("fileb");
55
////module Module {
6-
//// /*alias2Definition*/export import alias2 = alias1;
6+
//// export import /*alias2Definition*/alias2 = alias1;
77
////}
88
////
99
////// Type position

0 commit comments

Comments
 (0)