Skip to content

Commit 4b67b4a

Browse files
andrewbranchandrewbranch
authored
Support go-to-definition for imports of arbitrary files (#42539)
* Support go-to-definition for imports of scripts and arbitrary files * Support go-to-definition for non-existent files * Add missing file property * Use `isExternalModuleNameRelative` instead of `!pathIsBareSpecifier` * Add partial semantic test * Combine with symbol search for non-source-file file references * Fix and accept API baselines * Fix useless or * A definition is unverified if the file path was a guess, even if a source file has that path
1 parent aa67b16 commit 4b67b4a

15 files changed

+164
-27
lines changed

‎src/harness/client.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,9 @@ namespace ts.server {
526526
private decodeSpan(span: protocol.TextSpan & { file: string }): TextSpan;
527527
private decodeSpan(span: protocol.TextSpan, fileName: string, lineMap?: number[]): TextSpan;
528528
private decodeSpan(span: protocol.TextSpan & { file: string }, fileName?: string, lineMap?: number[]): TextSpan {
529+
if (span.start.line === 1 && span.start.offset === 1 && span.end.line === 1 && span.end.offset === 1) {
530+
return { start: 0, length: 0 };
531+
}
529532
fileName = fileName || span.file;
530533
lineMap = lineMap || this.getLineMap(fileName);
531534
return createTextSpanFromBounds(

‎src/harness/fourslashImpl.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ namespace FourSlash {
688688
this.verifyGoToXWorker(toArray(endMarker), () => this.getGoToDefinition());
689689
}
690690

691-
public verifyGoToDefinition(arg0: any, endMarkerNames?: ArrayOrSingle<string>) {
691+
public verifyGoToDefinition(arg0: any, endMarkerNames?: ArrayOrSingle<string> | { file: string }) {
692692
this.verifyGoToX(arg0, endMarkerNames, () => this.getGoToDefinitionAndBoundSpan());
693693
}
694694

@@ -705,7 +705,7 @@ namespace FourSlash {
705705
this.languageService.getTypeDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition));
706706
}
707707

708-
private verifyGoToX(arg0: any, endMarkerNames: ArrayOrSingle<string> | undefined, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
708+
private verifyGoToX(arg0: any, endMarkerNames: ArrayOrSingle<string> | { file: string } | undefined, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
709709
if (endMarkerNames) {
710710
this.verifyGoToXPlain(arg0, endMarkerNames, getDefs);
711711
}
@@ -725,7 +725,7 @@ namespace FourSlash {
725725
}
726726
}
727727

728-
private verifyGoToXPlain(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string>, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
728+
private verifyGoToXPlain(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string> | { file: string }, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
729729
for (const start of toArray(startMarkerNames)) {
730730
this.verifyGoToXSingle(start, endMarkerNames, getDefs);
731731
}
@@ -737,12 +737,12 @@ namespace FourSlash {
737737
}
738738
}
739739

740-
private verifyGoToXSingle(startMarkerName: string, endMarkerNames: ArrayOrSingle<string>, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
740+
private verifyGoToXSingle(startMarkerName: string, endMarkerNames: ArrayOrSingle<string> | { file: string }, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
741741
this.goToMarker(startMarkerName);
742742
this.verifyGoToXWorker(toArray(endMarkerNames), getDefs, startMarkerName);
743743
}
744744

745-
private verifyGoToXWorker(endMarkers: readonly string[], getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined, startMarkerName?: string) {
745+
private verifyGoToXWorker(endMarkers: readonly (string | { file: string })[], getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined, startMarkerName?: string) {
746746
const defs = getDefs();
747747
let definitions: readonly ts.DefinitionInfo[];
748748
let testName: string;
@@ -762,21 +762,22 @@ namespace FourSlash {
762762
this.raiseError(`${testName} failed - expected to find ${endMarkers.length} definitions but got ${definitions.length}`);
763763
}
764764

765-
ts.zipWith(endMarkers, definitions, (endMarker, definition, i) => {
766-
const marker = this.getMarkerByName(endMarker);
767-
if (ts.comparePaths(marker.fileName, definition.fileName, /*ignoreCase*/ true) !== ts.Comparison.EqualTo || marker.position !== definition.textSpan.start) {
768-
const filesToDisplay = ts.deduplicate([marker.fileName, definition.fileName], ts.equateValues);
769-
const markers = [{ text: "EXPECTED", fileName: marker.fileName, position: marker.position }, { text: "ACTUAL", fileName: definition.fileName, position: definition.textSpan.start }];
765+
ts.zipWith(endMarkers, definitions, (endMarkerOrFileResult, definition, i) => {
766+
const expectedFileName = typeof endMarkerOrFileResult === "string" ? this.getMarkerByName(endMarkerOrFileResult).fileName : endMarkerOrFileResult.file;
767+
const expectedPosition = typeof endMarkerOrFileResult === "string" ? this.getMarkerByName(endMarkerOrFileResult).position : 0;
768+
if (ts.comparePaths(expectedFileName, definition.fileName, /*ignoreCase*/ true) !== ts.Comparison.EqualTo || expectedPosition !== definition.textSpan.start) {
769+
const filesToDisplay = ts.deduplicate([expectedFileName, definition.fileName], ts.equateValues);
770+
const markers = [{ text: "EXPECTED", fileName: expectedFileName, position: expectedPosition }, { text: "ACTUAL", fileName: definition.fileName, position: definition.textSpan.start }];
770771
const text = filesToDisplay.map(fileName => {
771772
const markersToRender = markers.filter(m => m.fileName === fileName).sort((a, b) => b.position - a.position);
772-
let fileContent = this.getFileContent(fileName);
773+
let fileContent = this.tryGetFileContent(fileName) || "";
773774
for (const marker of markersToRender) {
774775
fileContent = fileContent.slice(0, marker.position) + `\x1b[1;4m/*${marker.text}*/\x1b[0;31m` + fileContent.slice(marker.position);
775776
}
776777
return `// @Filename: ${fileName}\n${fileContent}`;
777778
}).join("\n\n");
778779

779-
this.raiseError(`${testName} failed for definition ${endMarker} (${i}): expected ${marker.fileName} at ${marker.position}, got ${definition.fileName} at ${definition.textSpan.start}\n\n${text}\n`);
780+
this.raiseError(`${testName} failed for definition ${endMarkerOrFileResult} (${i}): expected ${expectedFileName} at ${expectedPosition}, got ${definition.fileName} at ${definition.textSpan.start}\n\n${text}\n`);
780781
}
781782
});
782783
}

‎src/server/protocol.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -987,8 +987,15 @@ namespace ts.server.protocol {
987987
export interface FileSpanWithContext extends FileSpan, TextSpanWithContext {
988988
}
989989

990+
export interface DefinitionInfo extends FileSpanWithContext {
991+
/**
992+
* When true, the file may or may not exist.
993+
*/
994+
unverified?: boolean;
995+
}
996+
990997
export interface DefinitionInfoAndBoundSpan {
991-
definitions: readonly FileSpanWithContext[];
998+
definitions: readonly DefinitionInfo[];
992999
textSpan: TextSpan;
9931000
}
9941001

‎src/services/findAllReferences.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ namespace ts.FindAllReferences {
624624
}
625625
if (isSourceFile(node)) {
626626
const resolvedRef = GoToDefinition.getReferenceAtPosition(node, position, program);
627-
if (!resolvedRef) {
627+
if (!resolvedRef?.file) {
628628
return undefined;
629629
}
630630
const moduleSymbol = program.getTypeChecker().getMergedSymbol(resolvedRef.file.symbol);
@@ -656,7 +656,7 @@ namespace ts.FindAllReferences {
656656
if (!symbol) {
657657
// String literal might be a property (and thus have a symbol), so do this here rather than in getReferencedSymbolsSpecial.
658658
if (!options.implementations && isStringLiteralLike(node)) {
659-
if (isRequireCall(node.parent, /*requireStringLiteralLikeArgument*/ true) || isExternalModuleReference(node.parent) || isImportDeclaration(node.parent) || isImportCall(node.parent)) {
659+
if (isModuleSpecifierLike(node)) {
660660
const fileIncludeReasons = program.getFileIncludeReasons();
661661
const referencedFileName = node.getSourceFile().resolvedModules?.get(node.text)?.resolvedFileName;
662662
const referencedFile = referencedFileName ? program.getSourceFile(referencedFileName) : undefined;

‎src/services/goToDefinition.ts

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
namespace ts.GoToDefinition {
33
export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile, position: number): readonly DefinitionInfo[] | undefined {
44
const resolvedRef = getReferenceAtPosition(sourceFile, position, program);
5-
if (resolvedRef) {
6-
return [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.file.fileName)];
5+
const fileReferenceDefinition = resolvedRef && [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName, resolvedRef.unverified)] || emptyArray;
6+
if (resolvedRef?.file) {
7+
// If `file` is missing, do a symbol-based lookup as well
8+
return fileReferenceDefinition;
79
}
810

911
const node = getTouchingPropertyName(sourceFile, position);
@@ -25,7 +27,7 @@ namespace ts.GoToDefinition {
2527
// Could not find a symbol e.g. node is string or number keyword,
2628
// or the symbol was an internal symbol and does not have a declaration e.g. undefined symbol
2729
if (!symbol) {
28-
return getDefinitionInfoForIndexSignatures(node, typeChecker);
30+
return concatenate(fileReferenceDefinition, getDefinitionInfoForIndexSignatures(node, typeChecker));
2931
}
3032

3133
const calledDeclaration = tryGetSignatureDeclaration(typeChecker, node);
@@ -76,7 +78,7 @@ namespace ts.GoToDefinition {
7678
});
7779
}
7880

79-
return getDefinitionFromObjectLiteralElement(typeChecker, node) || getDefinitionFromSymbol(typeChecker, symbol, node);
81+
return concatenate(fileReferenceDefinition, getDefinitionFromObjectLiteralElement(typeChecker, node) || getDefinitionFromSymbol(typeChecker, symbol, node));
8082
}
8183

8284
/**
@@ -111,24 +113,42 @@ namespace ts.GoToDefinition {
111113
}
112114
}
113115

114-
export function getReferenceAtPosition(sourceFile: SourceFile, position: number, program: Program): { reference: FileReference, file: SourceFile } | undefined {
116+
export function getReferenceAtPosition(sourceFile: SourceFile, position: number, program: Program): { reference: FileReference, fileName: string, unverified: boolean, file?: SourceFile } | undefined {
115117
const referencePath = findReferenceInPosition(sourceFile.referencedFiles, position);
116118
if (referencePath) {
117119
const file = program.getSourceFileFromReference(sourceFile, referencePath);
118-
return file && { reference: referencePath, file };
120+
return file && { reference: referencePath, fileName: file.fileName, file, unverified: false };
119121
}
120122

121123
const typeReferenceDirective = findReferenceInPosition(sourceFile.typeReferenceDirectives, position);
122124
if (typeReferenceDirective) {
123125
const reference = program.getResolvedTypeReferenceDirectives().get(typeReferenceDirective.fileName);
124126
const file = reference && program.getSourceFile(reference.resolvedFileName!); // TODO:GH#18217
125-
return file && { reference: typeReferenceDirective, file };
127+
return file && { reference: typeReferenceDirective, fileName: file.fileName, file, unverified: false };
126128
}
127129

128130
const libReferenceDirective = findReferenceInPosition(sourceFile.libReferenceDirectives, position);
129131
if (libReferenceDirective) {
130132
const file = program.getLibFileFromReference(libReferenceDirective);
131-
return file && { reference: libReferenceDirective, file };
133+
return file && { reference: libReferenceDirective, fileName: file.fileName, file, unverified: false };
134+
}
135+
136+
if (sourceFile.resolvedModules?.size) {
137+
const node = getTokenAtPosition(sourceFile, position);
138+
if (isModuleSpecifierLike(node) && isExternalModuleNameRelative(node.text) && sourceFile.resolvedModules.has(node.text)) {
139+
const verifiedFileName = sourceFile.resolvedModules.get(node.text)?.resolvedFileName;
140+
const fileName = verifiedFileName || resolvePath(getDirectoryPath(sourceFile.fileName), node.text);
141+
return {
142+
file: program.getSourceFile(fileName),
143+
fileName,
144+
reference: {
145+
pos: node.getStart(),
146+
end: node.getEnd(),
147+
fileName: node.text
148+
},
149+
unverified: !!verifiedFileName,
150+
};
151+
}
132152
}
133153

134154
return undefined;
@@ -318,14 +338,15 @@ namespace ts.GoToDefinition {
318338
return find(refs, ref => textRangeContainsPositionInclusive(ref, pos));
319339
}
320340

321-
function getDefinitionInfoForFileReference(name: string, targetFileName: string): DefinitionInfo {
341+
function getDefinitionInfoForFileReference(name: string, targetFileName: string, unverified: boolean): DefinitionInfo {
322342
return {
323343
fileName: targetFileName,
324344
textSpan: createTextSpanFromBounds(0, 0),
325345
kind: ScriptElementKind.scriptElement,
326346
name,
327347
containerName: undefined!,
328348
containerKind: undefined!, // TODO: GH#18217
349+
unverified,
329350
};
330351
}
331352

‎src/services/services.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2492,6 +2492,17 @@ namespace ts {
24922492
return refactor.getEditsForRefactor(getRefactorContext(file, positionOrRange, preferences, formatOptions), refactorName, actionName);
24932493
}
24942494

2495+
function toLineColumnOffset(fileName: string, position: number): LineAndCharacter {
2496+
// Go to Definition supports returning a zero-length span at position 0 for
2497+
// non-existent files. We need to special-case the conversion of position 0
2498+
// to avoid a crash trying to get the text for that file, since this function
2499+
// otherwise assumes that 'fileName' is the name of a file that exists.
2500+
if (position === 0) {
2501+
return { line: 0, character: 0 };
2502+
}
2503+
return sourceMapper.toLineColumnOffset(fileName, position);
2504+
}
2505+
24952506
function prepareCallHierarchy(fileName: string, position: number): CallHierarchyItem | CallHierarchyItem[] | undefined {
24962507
synchronizeHostData();
24972508
const declarations = CallHierarchy.resolveCallHierarchyDeclaration(program, getTouchingPropertyName(getValidSourceFile(fileName), position));
@@ -2567,7 +2578,7 @@ namespace ts {
25672578
getAutoImportProvider,
25682579
getApplicableRefactors,
25692580
getEditsForRefactor,
2570-
toLineColumnOffset: sourceMapper.toLineColumnOffset,
2581+
toLineColumnOffset,
25712582
getSourceMapper: () => sourceMapper,
25722583
clearSourceMapperCache: () => sourceMapper.clearCache(),
25732584
prepareCallHierarchy,

‎src/services/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,7 @@ namespace ts {
991991
name: string;
992992
containerKind: ScriptElementKind;
993993
containerName: string;
994+
unverified?: boolean;
994995
/* @internal */ isLocal?: boolean;
995996
}
996997

‎src/services/utilities.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,6 +1906,14 @@ namespace ts {
19061906
});
19071907
}
19081908

1909+
export function isModuleSpecifierLike(node: Node): node is StringLiteralLike {
1910+
return isStringLiteralLike(node) && (
1911+
isExternalModuleReference(node.parent) ||
1912+
isImportDeclaration(node.parent) ||
1913+
isRequireCall(node.parent, /*requireStringLiteralLikeArgument*/ false) && node.parent.arguments[0] === node ||
1914+
isImportCall(node.parent) && node.parent.arguments[0] === node);
1915+
}
1916+
19091917
export type ObjectBindingElementWithoutPropertyName = BindingElement & { name: Identifier };
19101918

19111919
export function isObjectBindingElementWithoutPropertyName(bindingElement: Node): bindingElement is ObjectBindingElementWithoutPropertyName {

‎src/testRunner/unittests/tsserver/partialSemanticServer.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
namespace ts.projectSystem {
2-
describe("unittests:: tsserver:: Semantic operations on PartialSemantic server", () => {
2+
describe("unittests:: tsserver:: Semantic operations on partialSemanticServer", () => {
33
function setup() {
44
const file1: File = {
55
path: `${tscWatch.projectRoot}/a.ts`,
@@ -204,5 +204,20 @@ function fooB() { }`
204204
assert.isUndefined(project.getPackageJsonAutoImportProvider());
205205
assert.deepEqual(project.getPackageJsonsForAutoImport(), emptyArray);
206206
});
207+
208+
it("should support go-to-definition on module specifiers", () => {
209+
const { session, file1, file2 } = setup();
210+
openFilesForSession([file1], session);
211+
const response = session.executeCommandSeq<protocol.DefinitionAndBoundSpanRequest>({
212+
command: protocol.CommandTypes.DefinitionAndBoundSpan,
213+
arguments: protocolFileLocationFromSubstring(file1, `"./b"`)
214+
}).response as protocol.DefinitionInfoAndBoundSpan;
215+
assert.isDefined(response);
216+
assert.deepEqual(response.definitions, [{
217+
file: file2.path,
218+
start: { line: 1, offset: 1 },
219+
end: { line: 1, offset: 1 }
220+
}]);
221+
});
207222
});
208223
}

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5965,6 +5965,7 @@ declare namespace ts {
59655965
name: string;
59665966
containerKind: ScriptElementKind;
59675967
containerName: string;
5968+
unverified?: boolean;
59685969
}
59695970
interface DefinitionInfoAndBoundSpan {
59705971
definitions?: readonly DefinitionInfo[];
@@ -7286,8 +7287,14 @@ declare namespace ts.server.protocol {
72867287
}
72877288
interface FileSpanWithContext extends FileSpan, TextSpanWithContext {
72887289
}
7290+
interface DefinitionInfo extends FileSpanWithContext {
7291+
/**
7292+
* When true, the file may or may not exist.
7293+
*/
7294+
unverified?: boolean;
7295+
}
72897296
interface DefinitionInfoAndBoundSpan {
7290-
definitions: readonly FileSpanWithContext[];
7297+
definitions: readonly DefinitionInfo[];
72917298
textSpan: TextSpan;
72927299
}
72937300
/**

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5965,6 +5965,7 @@ declare namespace ts {
59655965
name: string;
59665966
containerKind: ScriptElementKind;
59675967
containerName: string;
5968+
unverified?: boolean;
59685969
}
59695970
interface DefinitionInfoAndBoundSpan {
59705971
definitions?: readonly DefinitionInfo[];

‎tests/cases/fourslash/fourslash.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ declare namespace FourSlashInterface {
286286
* `verify.goToDefinition(["a", "aa"], "b");` verifies that markers "a" and "aa" have the same definition "b".
287287
* `verify.goToDefinition("a", ["b", "bb"]);` verifies that "a" has multiple definitions available.
288288
*/
289+
goToDefinition(startMarkerNames: ArrayOrSingle<string>, fileResult: { file: string }): void;
289290
goToDefinition(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string>): void;
290291
goToDefinition(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string>, range: Range): void;
291292
/** Performs `goToDefinition` for each pair. */

‎tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @esModuleInterop: true
4+
5+
// @Filename: index.css
6+
//// /*2a*/html { font-size: 16px; }
7+
8+
// @Filename: types.ts
9+
//// declare module /*2b*/"*.css" {
10+
//// const styles: any;
11+
//// export = styles;
12+
//// }
13+
14+
// @Filename: index.ts
15+
//// import styles from [|/*1*/"./index.css"|];
16+
17+
verify.goToDefinition("1", ["2a", "2b"]);

‎tests/cases/fourslash/goToDefinitionScriptImport.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @filename: scriptThing.ts
4+
//// /*1d*/console.log("woooo side effects")
5+
6+
// @filename: stylez.css
7+
//// /*2d*/div {
8+
//// color: magenta;
9+
//// }
10+
11+
// @filename: moduleThing.ts
12+
13+
// not a module, but we should let you jump to it.
14+
//// import [|/*1*/"./scriptThing"|];
15+
16+
// not JS/TS, but if we can, you should be able to jump to it.
17+
//// import [|/*2*/"./stylez.css"|];
18+
19+
verify.goToDefinition("1", "1d");
20+
verify.goToDefinition("2", "2d");

0 commit comments

Comments
 (0)