diff --git a/src/harness/client.ts b/src/harness/client.ts index 4acfca3e0eb91..e98db818aecb5 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -526,6 +526,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( diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 88237aea5a25d..73d83a1fb3510 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -688,7 +688,7 @@ namespace FourSlash { this.verifyGoToXWorker(toArray(endMarker), () => this.getGoToDefinition()); } - public verifyGoToDefinition(arg0: any, endMarkerNames?: ArrayOrSingle) { + public verifyGoToDefinition(arg0: any, endMarkerNames?: ArrayOrSingle | { file: string }) { this.verifyGoToX(arg0, endMarkerNames, () => this.getGoToDefinitionAndBoundSpan()); } @@ -705,7 +705,7 @@ namespace FourSlash { this.languageService.getTypeDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition)); } - private verifyGoToX(arg0: any, endMarkerNames: ArrayOrSingle | undefined, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) { + private verifyGoToX(arg0: any, endMarkerNames: ArrayOrSingle | { file: string } | undefined, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) { if (endMarkerNames) { this.verifyGoToXPlain(arg0, endMarkerNames, getDefs); } @@ -725,7 +725,7 @@ namespace FourSlash { } } - private verifyGoToXPlain(startMarkerNames: ArrayOrSingle, endMarkerNames: ArrayOrSingle, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) { + private verifyGoToXPlain(startMarkerNames: ArrayOrSingle, endMarkerNames: ArrayOrSingle | { file: string }, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) { for (const start of toArray(startMarkerNames)) { this.verifyGoToXSingle(start, endMarkerNames, getDefs); } @@ -737,12 +737,12 @@ namespace FourSlash { } } - private verifyGoToXSingle(startMarkerName: string, endMarkerNames: ArrayOrSingle, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) { + private verifyGoToXSingle(startMarkerName: string, endMarkerNames: ArrayOrSingle | { 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; @@ -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`); } }); } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 776f0ed60398f..60ec24f0f6bf0 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -987,8 +987,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; } diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index c4f6c8dea756a..851d2600fc7eb 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -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); @@ -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; diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index 532fbbe9ba120..b77b8ed0a4ceb 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -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); @@ -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); @@ -76,7 +78,7 @@ namespace ts.GoToDefinition { }); } - return getDefinitionFromObjectLiteralElement(typeChecker, node) || getDefinitionFromSymbol(typeChecker, symbol, node); + return concatenate(fileReferenceDefinition, getDefinitionFromObjectLiteralElement(typeChecker, node) || getDefinitionFromSymbol(typeChecker, symbol, node)); } /** @@ -111,24 +113,42 @@ namespace ts.GoToDefinition { } } - 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, + }; + } } return undefined; @@ -318,7 +338,7 @@ 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), @@ -326,6 +346,7 @@ namespace ts.GoToDefinition { name, containerName: undefined!, containerKind: undefined!, // TODO: GH#18217 + unverified, }; } diff --git a/src/services/services.ts b/src/services/services.ts index 884e890dccb20..2d482fe5ae9e6 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -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)); @@ -2567,7 +2578,7 @@ namespace ts { getAutoImportProvider, getApplicableRefactors, getEditsForRefactor, - toLineColumnOffset: sourceMapper.toLineColumnOffset, + toLineColumnOffset, getSourceMapper: () => sourceMapper, clearSourceMapperCache: () => sourceMapper.clearCache(), prepareCallHierarchy, diff --git a/src/services/types.ts b/src/services/types.ts index c09d0d7d59da3..9542ad56600fc 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -991,6 +991,7 @@ namespace ts { name: string; containerKind: ScriptElementKind; containerName: string; + unverified?: boolean; /* @internal */ isLocal?: boolean; } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index d57f267ea977c..74315895927ed 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -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 { diff --git a/src/testRunner/unittests/tsserver/partialSemanticServer.ts b/src/testRunner/unittests/tsserver/partialSemanticServer.ts index 5ef5cb6ccecc4..d7af24e3167bd 100644 --- a/src/testRunner/unittests/tsserver/partialSemanticServer.ts +++ b/src/testRunner/unittests/tsserver/partialSemanticServer.ts @@ -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`, @@ -204,5 +204,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({ + 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 } + }]); + }); }); } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 157fab06969ba..48420cb2e3d90 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -5965,6 +5965,7 @@ declare namespace ts { name: string; containerKind: ScriptElementKind; containerName: string; + unverified?: boolean; } interface DefinitionInfoAndBoundSpan { definitions?: readonly DefinitionInfo[]; @@ -7286,8 +7287,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; } /** diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 25e9f24c6224d..6847f968ab894 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -5965,6 +5965,7 @@ declare namespace ts { name: string; containerKind: ScriptElementKind; containerName: string; + unverified?: boolean; } interface DefinitionInfoAndBoundSpan { definitions?: readonly DefinitionInfo[]; diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index bf81a295f023f..845c0674a9830 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -286,6 +286,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, fileResult: { file: string }): void; goToDefinition(startMarkerNames: ArrayOrSingle, endMarkerNames: ArrayOrSingle): void; goToDefinition(startMarkerNames: ArrayOrSingle, endMarkerNames: ArrayOrSingle, range: Range): void; /** Performs `goToDefinition` for each pair. */ diff --git a/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts b/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts new file mode 100644 index 0000000000000..9d595e81eb444 --- /dev/null +++ b/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts @@ -0,0 +1,17 @@ +/// + +// @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"]); diff --git a/tests/cases/fourslash/goToDefinitionScriptImport.ts b/tests/cases/fourslash/goToDefinitionScriptImport.ts new file mode 100644 index 0000000000000..d576f5454ad30 --- /dev/null +++ b/tests/cases/fourslash/goToDefinitionScriptImport.ts @@ -0,0 +1,20 @@ +/// + +// @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"); diff --git a/tests/cases/fourslash/server/goToDefinitionScriptImportServer.ts b/tests/cases/fourslash/server/goToDefinitionScriptImportServer.ts new file mode 100644 index 0000000000000..82e1e6ef5cea4 --- /dev/null +++ b/tests/cases/fourslash/server/goToDefinitionScriptImportServer.ts @@ -0,0 +1,24 @@ +/// + +// @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"|]; + +// does not exist, but should return a response to it anyway so an editor can create it. +//// import [|/*3*/"./foo.txt"|]; + +verify.goToDefinition("1", "1d"); +verify.goToDefinition("2", "2d"); +verify.goToDefinition("3", { file: "/foo.txt" });