Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support go-to-definition for imports of arbitrary files #42539

Merged
merged 10 commits into from
Mar 1, 2021
3 changes: 3 additions & 0 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,9 @@ namespace ts.server {
private decodeSpan(span: protocol.TextSpan & { file: string }): TextSpan;
private decodeSpan(span: protocol.TextSpan, fileName: string, lineMap?: number[]): TextSpan;
private decodeSpan(span: protocol.TextSpan & { file: string }, fileName?: string, lineMap?: number[]): TextSpan {
if (span.start.line === 1 && span.start.offset === 1 && span.end.line === 1 && span.end.offset === 1) {
return { start: 0, length: 0 };
}
fileName = fileName || span.file;
lineMap = lineMap || this.getLineMap(fileName);
return createTextSpanFromBounds(
Expand Down
25 changes: 13 additions & 12 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ namespace FourSlash {
this.verifyGoToXWorker(toArray(endMarker), () => this.getGoToDefinition());
}

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

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

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

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

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

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

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

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`);
this.raiseError(`${testName} failed for definition ${endMarkerOrFileResult} (${i}): expected ${expectedFileName} at ${expectedPosition}, got ${definition.fileName} at ${definition.textSpan.start}\n\n${text}\n`);
}
});
}
Expand Down
9 changes: 8 additions & 1 deletion src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -977,8 +977,15 @@ namespace ts.server.protocol {
export interface FileSpanWithContext extends FileSpan, TextSpanWithContext {
}

export interface DefinitionInfo extends FileSpanWithContext {
/**
* When true, the file may or may not exist.
*/
unverified?: boolean;
}

export interface DefinitionInfoAndBoundSpan {
definitions: readonly FileSpanWithContext[];
definitions: readonly DefinitionInfo[];
textSpan: TextSpan;
}

Expand Down
4 changes: 2 additions & 2 deletions src/services/findAllReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ namespace ts.FindAllReferences {
}
if (isSourceFile(node)) {
const resolvedRef = GoToDefinition.getReferenceAtPosition(node, position, program);
if (!resolvedRef) {
if (!resolvedRef?.file) {
return undefined;
}
const moduleSymbol = program.getTypeChecker().getMergedSymbol(resolvedRef.file.symbol);
Expand Down Expand Up @@ -656,7 +656,7 @@ namespace ts.FindAllReferences {
if (!symbol) {
// String literal might be a property (and thus have a symbol), so do this here rather than in getReferencedSymbolsSpecial.
if (!options.implementations && isStringLiteralLike(node)) {
if (isRequireCall(node.parent, /*requireStringLiteralLikeArgument*/ true) || isExternalModuleReference(node.parent) || isImportDeclaration(node.parent) || isImportCall(node.parent)) {
if (isModuleSpecifierLike(node)) {
const fileIncludeReasons = program.getFileIncludeReasons();
const referencedFileName = node.getSourceFile().resolvedModules?.get(node.text)?.resolvedFileName;
const referencedFile = referencedFileName ? program.getSourceFile(referencedFileName) : undefined;
Expand Down
39 changes: 30 additions & 9 deletions src/services/goToDefinition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
namespace ts.GoToDefinition {
export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile, position: number): readonly DefinitionInfo[] | undefined {
const resolvedRef = getReferenceAtPosition(sourceFile, position, program);
if (resolvedRef) {
return [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.file.fileName)];
const fileReferenceDefinition = resolvedRef && [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName, resolvedRef.unverified)] || emptyArray;
if (resolvedRef?.file) {
// If `file` is missing, do a symbol-based lookup as well
return fileReferenceDefinition;
}

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

const calledDeclaration = tryGetSignatureDeclaration(typeChecker, node);
Expand Down Expand Up @@ -93,7 +95,7 @@ namespace ts.GoToDefinition {
}
}

return getDefinitionFromSymbol(typeChecker, symbol, node);
return concatenate(fileReferenceDefinition, getDefinitionFromSymbol(typeChecker, symbol, node));
}

/**
Expand All @@ -108,24 +110,42 @@ namespace ts.GoToDefinition {
|| (!isCallLikeExpression(calledDeclaration.parent) && s === calledDeclaration.parent.symbol);
}

export function getReferenceAtPosition(sourceFile: SourceFile, position: number, program: Program): { reference: FileReference, file: SourceFile } | undefined {
export function getReferenceAtPosition(sourceFile: SourceFile, position: number, program: Program): { reference: FileReference, fileName: string, unverified: boolean, file?: SourceFile } | undefined {
const referencePath = findReferenceInPosition(sourceFile.referencedFiles, position);
if (referencePath) {
const file = program.getSourceFileFromReference(sourceFile, referencePath);
return file && { reference: referencePath, file };
return file && { reference: referencePath, fileName: file.fileName, file, unverified: false };
}

const typeReferenceDirective = findReferenceInPosition(sourceFile.typeReferenceDirectives, position);
if (typeReferenceDirective) {
const reference = program.getResolvedTypeReferenceDirectives().get(typeReferenceDirective.fileName);
const file = reference && program.getSourceFile(reference.resolvedFileName!); // TODO:GH#18217
return file && { reference: typeReferenceDirective, file };
return file && { reference: typeReferenceDirective, fileName: file.fileName, file, unverified: false };
}

const libReferenceDirective = findReferenceInPosition(sourceFile.libReferenceDirectives, position);
if (libReferenceDirective) {
const file = program.getLibFileFromReference(libReferenceDirective);
return file && { reference: libReferenceDirective, file };
return file && { reference: libReferenceDirective, fileName: file.fileName, file, unverified: false };
}

if (sourceFile.resolvedModules?.size) {
const node = getTokenAtPosition(sourceFile, position);
if (isModuleSpecifierLike(node) && isExternalModuleNameRelative(node.text) && sourceFile.resolvedModules.has(node.text)) {
const verifiedFileName = sourceFile.resolvedModules.get(node.text)?.resolvedFileName;
const fileName = verifiedFileName || resolvePath(getDirectoryPath(sourceFile.fileName), node.text);
return {
file: program.getSourceFile(fileName),
fileName,
reference: {
pos: node.getStart(),
end: node.getEnd(),
fileName: node.text
},
unverified: !!verifiedFileName,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewbranch just realised that we are not passing this on in the through server even though we added this property to protocol.
tsserver37220.log

};
}
}

return undefined;
Expand Down Expand Up @@ -315,14 +335,15 @@ namespace ts.GoToDefinition {
return find(refs, ref => textRangeContainsPositionInclusive(ref, pos));
}

function getDefinitionInfoForFileReference(name: string, targetFileName: string): DefinitionInfo {
function getDefinitionInfoForFileReference(name: string, targetFileName: string, unverified: boolean): DefinitionInfo {
return {
fileName: targetFileName,
textSpan: createTextSpanFromBounds(0, 0),
kind: ScriptElementKind.scriptElement,
name,
containerName: undefined!,
containerKind: undefined!, // TODO: GH#18217
unverified,
};
}

Expand Down
13 changes: 12 additions & 1 deletion src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2492,6 +2492,17 @@ namespace ts {
return refactor.getEditsForRefactor(getRefactorContext(file, positionOrRange, preferences, formatOptions), refactorName, actionName);
}

function toLineColumnOffset(fileName: string, position: number): LineAndCharacter {
// Go to Definition supports returning a zero-length span at position 0 for
// non-existent files. We need to special-case the conversion of position 0
// to avoid a crash trying to get the text for that file, since this function
// otherwise assumes that 'fileName' is the name of a file that exists.
if (position === 0) {
return { line: 0, character: 0 };
}
return sourceMapper.toLineColumnOffset(fileName, position);
}

function prepareCallHierarchy(fileName: string, position: number): CallHierarchyItem | CallHierarchyItem[] | undefined {
synchronizeHostData();
const declarations = CallHierarchy.resolveCallHierarchyDeclaration(program, getTouchingPropertyName(getValidSourceFile(fileName), position));
Expand Down Expand Up @@ -2567,7 +2578,7 @@ namespace ts {
getAutoImportProvider,
getApplicableRefactors,
getEditsForRefactor,
toLineColumnOffset: sourceMapper.toLineColumnOffset,
toLineColumnOffset,
getSourceMapper: () => sourceMapper,
clearSourceMapperCache: () => sourceMapper.clearCache(),
prepareCallHierarchy,
Expand Down
1 change: 1 addition & 0 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,7 @@ namespace ts {
name: string;
containerKind: ScriptElementKind;
containerName: string;
unverified?: boolean;
/* @internal */ isLocal?: boolean;
}

Expand Down
8 changes: 8 additions & 0 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1906,6 +1906,14 @@ namespace ts {
});
}

export function isModuleSpecifierLike(node: Node): node is StringLiteralLike {
return isStringLiteralLike(node) && (
isExternalModuleReference(node.parent) ||
isImportDeclaration(node.parent) ||
isRequireCall(node.parent, /*requireStringLiteralLikeArgument*/ false) && node.parent.arguments[0] === node ||
isImportCall(node.parent) && node.parent.arguments[0] === node);
}

export type ObjectBindingElementWithoutPropertyName = BindingElement & { name: Identifier };

export function isObjectBindingElementWithoutPropertyName(bindingElement: Node): bindingElement is ObjectBindingElementWithoutPropertyName {
Expand Down
17 changes: 16 additions & 1 deletion src/testRunner/unittests/tsserver/partialSemanticServer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
namespace ts.projectSystem {
describe("unittests:: tsserver:: Semantic operations on PartialSemantic server", () => {
describe("unittests:: tsserver:: Semantic operations on partialSemanticServer", () => {
function setup() {
const file1: File = {
path: `${tscWatch.projectRoot}/a.ts`,
Expand Down Expand Up @@ -203,5 +203,20 @@ function fooB() { }`
assert.isUndefined(project.getPackageJsonAutoImportProvider());
assert.deepEqual(project.getPackageJsonsForAutoImport(), emptyArray);
});

it("should support go-to-definition on module specifiers", () => {
const { session, file1, file2 } = setup();
openFilesForSession([file1], session);
const response = session.executeCommandSeq<protocol.DefinitionAndBoundSpanRequest>({
command: protocol.CommandTypes.DefinitionAndBoundSpan,
arguments: protocolFileLocationFromSubstring(file1, `"./b"`)
}).response as protocol.DefinitionInfoAndBoundSpan;
assert.isDefined(response);
assert.deepEqual(response.definitions, [{
file: file2.path,
start: { line: 1, offset: 1 },
end: { line: 1, offset: 1 }
}]);
});
});
}
9 changes: 8 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5948,6 +5948,7 @@ declare namespace ts {
name: string;
containerKind: ScriptElementKind;
containerName: string;
unverified?: boolean;
}
interface DefinitionInfoAndBoundSpan {
definitions?: readonly DefinitionInfo[];
Expand Down Expand Up @@ -7208,8 +7209,14 @@ declare namespace ts.server.protocol {
}
interface FileSpanWithContext extends FileSpan, TextSpanWithContext {
}
interface DefinitionInfo extends FileSpanWithContext {
/**
* When true, the file may or may not exist.
*/
unverified?: boolean;
}
interface DefinitionInfoAndBoundSpan {
definitions: readonly FileSpanWithContext[];
definitions: readonly DefinitionInfo[];
textSpan: TextSpan;
}
/**
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5948,6 +5948,7 @@ declare namespace ts {
name: string;
containerKind: ScriptElementKind;
containerName: string;
unverified?: boolean;
}
interface DefinitionInfoAndBoundSpan {
definitions?: readonly DefinitionInfo[];
Expand Down
1 change: 1 addition & 0 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ declare namespace FourSlashInterface {
* `verify.goToDefinition(["a", "aa"], "b");` verifies that markers "a" and "aa" have the same definition "b".
* `verify.goToDefinition("a", ["b", "bb"]);` verifies that "a" has multiple definitions available.
*/
goToDefinition(startMarkerNames: ArrayOrSingle<string>, fileResult: { file: string }): void;
goToDefinition(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string>): void;
goToDefinition(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string>, range: Range): void;
/** Performs `goToDefinition` for each pair. */
Expand Down
17 changes: 17 additions & 0 deletions tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/// <reference path="fourslash.ts" />

// @esModuleInterop: true

// @Filename: index.css
//// /*2a*/html { font-size: 16px; }

// @Filename: types.ts
//// declare module /*2b*/"*.css" {
//// const styles: any;
//// export = styles;
//// }

// @Filename: index.ts
//// import styles from [|/*1*/"./index.css"|];

verify.goToDefinition("1", ["2a", "2b"]);
20 changes: 20 additions & 0 deletions tests/cases/fourslash/goToDefinitionScriptImport.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <reference path="fourslash.ts" />

// @filename: scriptThing.ts
//// /*1d*/console.log("woooo side effects")

// @filename: stylez.css
//// /*2d*/div {
//// color: magenta;
//// }

// @filename: moduleThing.ts

// not a module, but we should let you jump to it.
//// import [|/*1*/"./scriptThing"|];

// not JS/TS, but if we can, you should be able to jump to it.
//// import [|/*2*/"./stylez.css"|];

verify.goToDefinition("1", "1d");
verify.goToDefinition("2", "2d");
Loading