From 5a039afd9e01066bd6d3f12b161d0399dbb0486c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 28 Jan 2021 12:47:23 -0800 Subject: [PATCH 1/9] Support go-to-definition for imports of scripts and arbitrary files --- src/services/findAllReferences.ts | 4 +-- src/services/goToDefinition.ts | 27 +++++++++++++++---- src/services/utilities.ts | 8 ++++++ .../fourslash/goToDefinitionScriptImport.ts | 20 ++++++++++++++ 4 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/goToDefinitionScriptImport.ts diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 12a97433615ca..5df5669986fe4 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 faaf1077354a9..74a1c953a3ff4 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -3,7 +3,7 @@ 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)]; + return [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName)]; } const node = getTouchingPropertyName(sourceFile, position); @@ -108,24 +108,41 @@ 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, 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 }; } 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 }; } 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 }; + } + + if (sourceFile.resolvedModules?.size) { + const node = getTokenAtPosition(sourceFile, position); + if (isModuleSpecifierLike(node) && !pathIsBareSpecifier(node.text) && sourceFile.resolvedModules.has(node.text)) { + const fileName = sourceFile.resolvedModules.get(node.text)?.resolvedFileName + || resolvePath(getDirectoryPath(sourceFile.fileName), node.text); + return { + file: program.getSourceFile(fileName), + fileName, + reference: { + pos: node.getStart(), + end: node.getEnd(), + fileName: node.text + } + }; + } } return undefined; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 8344c3e60ff87..ec5e2d9f574db 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/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"); From 8a9cc947c0fae47769899e77769a8622efdcbbbf Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 28 Jan 2021 12:47:38 -0800 Subject: [PATCH 2/9] Support go-to-definition for non-existent files --- src/harness/client.ts | 3 +++ src/harness/fourslashImpl.ts | 25 ++++++++++--------- src/services/services.ts | 13 +++++++++- tests/cases/fourslash/fourslash.ts | 1 + .../goToDefinitionScriptImportServer.ts | 24 ++++++++++++++++++ 5 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 tests/cases/fourslash/server/goToDefinitionScriptImportServer.ts diff --git a/src/harness/client.ts b/src/harness/client.ts index 4bdb316b6b47d..769aedd8efd9c 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -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( diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 15d0be1a143ba..0f87da1ec02a5 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/services/services.ts b/src/services/services.ts index 3b4ce09683446..6d2d80d2027cd 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/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 7228fee24b118..8fe3fac1270a3 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -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, 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/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" }); From 5f256f175af05398497140bf36778fd4c49259c8 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 28 Jan 2021 14:29:35 -0800 Subject: [PATCH 3/9] Add missing file property --- src/services/goToDefinition.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index 74a1c953a3ff4..798354f336b56 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -112,20 +112,20 @@ namespace ts.GoToDefinition { const referencePath = findReferenceInPosition(sourceFile.referencedFiles, position); if (referencePath) { const file = program.getSourceFileFromReference(sourceFile, referencePath); - return file && { reference: referencePath, fileName: file.fileName }; + return file && { reference: referencePath, fileName: file.fileName, file }; } 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, fileName: file.fileName }; + return file && { reference: typeReferenceDirective, fileName: file.fileName, file }; } const libReferenceDirective = findReferenceInPosition(sourceFile.libReferenceDirectives, position); if (libReferenceDirective) { const file = program.getLibFileFromReference(libReferenceDirective); - return file && { reference: libReferenceDirective, fileName: file.fileName }; + return file && { reference: libReferenceDirective, fileName: file.fileName, file }; } if (sourceFile.resolvedModules?.size) { From 248ed3728e7a4a035e91bc5ec67c4fcc50df5326 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 28 Jan 2021 16:02:36 -0800 Subject: [PATCH 4/9] Use `isExternalModuleNameRelative` instead of `!pathIsBareSpecifier` --- src/services/goToDefinition.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index 798354f336b56..77201729aa4d7 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -130,7 +130,7 @@ namespace ts.GoToDefinition { if (sourceFile.resolvedModules?.size) { const node = getTokenAtPosition(sourceFile, position); - if (isModuleSpecifierLike(node) && !pathIsBareSpecifier(node.text) && sourceFile.resolvedModules.has(node.text)) { + if (isModuleSpecifierLike(node) && isExternalModuleNameRelative(node.text) && sourceFile.resolvedModules.has(node.text)) { const fileName = sourceFile.resolvedModules.get(node.text)?.resolvedFileName || resolvePath(getDirectoryPath(sourceFile.fileName), node.text); return { From f6d3b18f1c9857cc611928e39f1468a81617f99c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 1 Feb 2021 11:47:16 -0800 Subject: [PATCH 5/9] Add partial semantic test --- .../unittests/tsserver/partialSemanticServer.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/testRunner/unittests/tsserver/partialSemanticServer.ts b/src/testRunner/unittests/tsserver/partialSemanticServer.ts index 2c8b8ed14b068..579b89440ada7 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`, @@ -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({ + 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 } + }]); + }); }); } From a5195f53b29fd2a35c2adcafab5183d707c4dcfb Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 16 Feb 2021 14:29:50 -0800 Subject: [PATCH 6/9] Combine with symbol search for non-source-file file references --- src/server/protocol.ts | 9 ++++++++- src/services/goToDefinition.ts | 13 +++++++++---- src/services/types.ts | 1 + .../goToDefinitionCSSPatternAmbientModule.ts | 17 +++++++++++++++++ 4 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 237ce1fd5a05f..f87cc4b5347fc 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -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; } diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index 77201729aa4d7..c3f6b6544fa7b 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -2,8 +2,9 @@ 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.fileName)]; + if (resolvedRef?.file) { + // If `file` is missing, do a symbol-based lookup as well + return [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName, /*unverified*/ false)]; } const node = getTouchingPropertyName(sourceFile, position); @@ -93,7 +94,10 @@ namespace ts.GoToDefinition { } } - return getDefinitionFromSymbol(typeChecker, symbol, node); + return flatten([ + resolvedRef && getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName, /*unverified*/ true), + getDefinitionFromSymbol(typeChecker, symbol, node) + ]); } /** @@ -332,7 +336,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), @@ -340,6 +344,7 @@ namespace ts.GoToDefinition { name, containerName: undefined!, containerKind: undefined!, // TODO: GH#18217 + unverified, }; } diff --git a/src/services/types.ts b/src/services/types.ts index dd97197a92416..bb266186e5132 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -989,6 +989,7 @@ namespace ts { name: string; containerKind: ScriptElementKind; containerName: string; + unverified?: boolean; /* @internal */ isLocal?: boolean; } diff --git a/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts b/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts new file mode 100644 index 0000000000000..63715824ea078 --- /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" { +//// declare const styles: any; +//// export = styles; +//// } + +// @Filename: index.ts +//// import styles from [|/*1*/"./index.css"|]; + +verify.goToDefinition("1", ["2a", "2b"]); From 54d8a72b90141815cab2b126a0aa71c29e52acda Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 16 Feb 2021 14:54:01 -0800 Subject: [PATCH 7/9] Fix and accept API baselines --- src/services/goToDefinition.ts | 10 ++++------ tests/baselines/reference/api/tsserverlibrary.d.ts | 9 ++++++++- tests/baselines/reference/api/typescript.d.ts | 1 + .../fourslash/goToDefinitionCSSPatternAmbientModule.ts | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index c3f6b6544fa7b..dad75ad6576c7 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -2,9 +2,10 @@ namespace ts.GoToDefinition { export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile, position: number): readonly DefinitionInfo[] | undefined { const resolvedRef = getReferenceAtPosition(sourceFile, position, program); + const fileReferenceDefinition = resolvedRef && [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName, !!resolvedRef.file)] || emptyArray; if (resolvedRef?.file) { // If `file` is missing, do a symbol-based lookup as well - return [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName, /*unverified*/ false)]; + return fileReferenceDefinition; } const node = getTouchingPropertyName(sourceFile, position); @@ -26,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 fileReferenceDefinition || getDefinitionInfoForIndexSignatures(node, typeChecker); } const calledDeclaration = tryGetSignatureDeclaration(typeChecker, node); @@ -94,10 +95,7 @@ namespace ts.GoToDefinition { } } - return flatten([ - resolvedRef && getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName, /*unverified*/ true), - getDefinitionFromSymbol(typeChecker, symbol, node) - ]); + return concatenate(fileReferenceDefinition, getDefinitionFromSymbol(typeChecker, symbol, node)); } /** diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 1598efef3279c..4e50ad7bd5d11 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -5948,6 +5948,7 @@ declare namespace ts { name: string; containerKind: ScriptElementKind; containerName: string; + unverified?: boolean; } interface DefinitionInfoAndBoundSpan { definitions?: readonly DefinitionInfo[]; @@ -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; } /** diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index f8f86d5b88fb2..75cc65cbf070c 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -5948,6 +5948,7 @@ declare namespace ts { name: string; containerKind: ScriptElementKind; containerName: string; + unverified?: boolean; } interface DefinitionInfoAndBoundSpan { definitions?: readonly DefinitionInfo[]; diff --git a/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts b/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts index 63715824ea078..9d595e81eb444 100644 --- a/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts +++ b/tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts @@ -7,7 +7,7 @@ // @Filename: types.ts //// declare module /*2b*/"*.css" { -//// declare const styles: any; +//// const styles: any; //// export = styles; //// } From 48e0d269a93f8977cd85a5e4bca8e6c74bc69aa4 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 16 Feb 2021 15:06:02 -0800 Subject: [PATCH 8/9] Fix useless or --- src/services/goToDefinition.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index dad75ad6576c7..da1296177af15 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -27,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 fileReferenceDefinition || getDefinitionInfoForIndexSignatures(node, typeChecker); + return concatenate(fileReferenceDefinition, getDefinitionInfoForIndexSignatures(node, typeChecker)); } const calledDeclaration = tryGetSignatureDeclaration(typeChecker, node); From c2aa82cc4d1ab9795f1622c3f936a247b83e3810 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 26 Feb 2021 15:20:41 -0800 Subject: [PATCH 9/9] A definition is unverified if the file path was a guess, even if a source file has that path --- src/services/goToDefinition.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index da1296177af15..2d55d257ed8fc 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -2,7 +2,7 @@ namespace ts.GoToDefinition { export function getDefinitionAtPosition(program: Program, sourceFile: SourceFile, position: number): readonly DefinitionInfo[] | undefined { const resolvedRef = getReferenceAtPosition(sourceFile, position, program); - const fileReferenceDefinition = resolvedRef && [getDefinitionInfoForFileReference(resolvedRef.reference.fileName, resolvedRef.fileName, !!resolvedRef.file)] || emptyArray; + 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; @@ -110,31 +110,31 @@ namespace ts.GoToDefinition { || (!isCallLikeExpression(calledDeclaration.parent) && s === calledDeclaration.parent.symbol); } - export function getReferenceAtPosition(sourceFile: SourceFile, position: number, program: Program): { reference: FileReference, fileName: string, 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, fileName: file.fileName, 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, fileName: file.fileName, 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, fileName: file.fileName, 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 fileName = sourceFile.resolvedModules.get(node.text)?.resolvedFileName - || resolvePath(getDirectoryPath(sourceFile.fileName), 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, @@ -142,7 +142,8 @@ namespace ts.GoToDefinition { pos: node.getStart(), end: node.getEnd(), fileName: node.text - } + }, + unverified: !!verifiedFileName, }; } }