Fix workspace/xreferences (#296)

- Move SymbolDescriptor path normalization to function
- Move `defInfoToSymbolDescriptor` to symbols module
- Strip node_modules from SymbolDescriptor paths

Author: felixfbecker

src/symbols.ts

@@ -3,7 +3,33 @@ import * as ts from 'typescript';
 import { SymbolInformation, SymbolKind } from 'vscode-languageserver-types';
 import { isTypeScriptLibrary } from './memfs';
 import { SymbolDescriptor } from './request-type';
-import { path2uri } from './util';
+import { path2uri, toUnixPath } from './util';
+
+/**
+ * Returns a SymbolDescriptor for a ts.DefinitionInfo
+ */
+export function definitionInfoToSymbolDescriptor(info: ts.DefinitionInfo, rootPath: string): SymbolDescriptor {
+	const rootUnixPath = toUnixPath(rootPath);
+	const symbolDescriptor: SymbolDescriptor = {
+		kind: info.kind || '',
+		name: info.name || '',
+		containerKind: info.containerKind || '',
+		containerName: info.containerName || '',
+		filePath: info.fileName
+	};
+	// If the symbol is an external module representing a file, set name to the file path
+	if (info.kind === ts.ScriptElementKind.moduleElement && info.name && /[\\\/]/.test(info.name)) {
+		symbolDescriptor.name = '"' + info.fileName.replace(/(?:\.d)?\.tsx?$/, '') + '"';
+	}
+	// If the symbol itself is not a module and there is no containerKind
+	// then the container is an external module named by the file name (without file extension)
+	if (info.kind !== ts.ScriptElementKind.moduleElement && !info.containerKind && !info.containerName) {
+		symbolDescriptor.containerName = '"' + info.fileName.replace(/(?:\.d)?\.tsx?$/, '') + '"';
+		symbolDescriptor.containerKind = ts.ScriptElementKind.moduleElement;
+	}
+	normalizeSymbolDescriptorPaths(symbolDescriptor, rootUnixPath);
+	return symbolDescriptor;
+}
 
 /**
  * Transforms definition's file name to URI. If definition belongs to the in-memory TypeScript library,
@@ -136,10 +162,7 @@ export function navigationTreeToSymbolDescriptor(tree: ts.NavigationTree, parent
 		}
 		symbolDescriptor.containerKind = ts.ScriptElementKind.moduleElement;
 	}
-	// Make all paths that may occur in module names relative to the workspace rootPath
-	symbolDescriptor.name = symbolDescriptor.name.replace(rootPath, '');
-	symbolDescriptor.containerName = symbolDescriptor.containerName.replace(rootPath, '');
-	symbolDescriptor.filePath = symbolDescriptor.filePath.replace(rootPath, '');
+	normalizeSymbolDescriptorPaths(symbolDescriptor, rootPath);
 	return symbolDescriptor;
 }
 
@@ -167,3 +190,18 @@ export function navigationTreeIsSymbol(tree: ts.NavigationTree): boolean {
 	}
 	return true;
 }
+
+/**
+ * Makes paths relative to the passed rootPath and strips `node_modules` out of paths
+ */
+function normalizeSymbolDescriptorPaths(symbol: SymbolDescriptor, rootPath: string) {
+	for (const key of ['name', 'containerName', 'filePath'] as ['name', 'containerName', 'filePath']) {
+		// Make all paths that may occur in module names relative to the workspace rootPath
+		symbol[key] = symbol[key].replace(rootPath, '');
+		// Remove node_modules part from a module name
+		// The SymbolDescriptor will be used in the defining repo, where the symbol file path will never contain node_modules
+		// It may contain the package name though if the repo is a monorepo with multiple packages
+		const regExp = /[^"]*node_modules\//;
+		symbol[key] = symbol[key].replace(regExp, '');
+	}
+}

src/test/typescript-service-helpers.ts

@@ -925,9 +925,9 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
 						uri: rootUri + 'c.ts'
 					},
 					symbol: {
-						filePath: 'node_modules/dep/dep.ts',
+						filePath: 'dep/dep.ts',
 						containerKind: '',
-						containerName: '"node_modules/dep/dep"',
+						containerName: '"dep/dep"',
 						kind: 'var',
 						name: 'x'
 					}
@@ -952,9 +952,9 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
 						uri: rootUri + 'c.ts'
 					},
 					symbol: {
-						filePath: 'node_modules/dep/dep.ts',
+						filePath: 'dep/dep.ts',
 						containerKind: '',
-						containerName: '"node_modules/dep/dep"',
+						containerName: '"dep/dep"',
 						kind: 'var',
 						name: 'x',
 						package: {
@@ -1051,9 +1051,9 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
 							uri: rootUri + 'c.ts'
 						},
 						symbol: {
-							filePath: 'node_modules/dep/dep.ts',
+							filePath: 'dep/dep.ts',
 							containerKind: '',
-							containerName: '"node_modules/dep/dep"',
+							containerName: '"dep/dep"',
 							kind: 'var',
 							name: 'x'
 						}

src/typescript-service.ts

@@ -51,7 +51,6 @@ import {
 	WorkspaceSymbolParams
 } from './request-type';
 import {
-	defInfoToSymbolDescriptor,
 	getMatchingPropertyCount,
 	getPropertyCount,
 	JSONPTR,
@@ -66,6 +65,7 @@ import { castArray, merge, omit } from 'lodash';
 import * as url from 'url';
 import { extractDefinitelyTypedPackageName, extractNodeModulesPackageName, PackageJson, PackageManager } from './packages';
 import {
+	definitionInfoToSymbolDescriptor,
 	locationUri,
 	navigateToItemToSymbolInformation,
 	navigationTreeIsSymbol,
@@ -408,7 +408,7 @@ export class TypeScriptService {
 								if (!sourceFile) {
 									throw new Error(`Expected source file ${definition.fileName} to exist in configuration`);
 								}
-								const symbol = defInfoToSymbolDescriptor(definition, this.root);
+								const symbol = definitionInfoToSymbolDescriptor(definition, this.root);
 								if (packageDescriptor) {
 									symbol.package = packageDescriptor;
 								}
@@ -802,7 +802,7 @@ export class TypeScriptService {
 									// Find definition for node
 									return Observable.from(config.getService().getDefinitionAtPosition(source.fileName, node.pos + 1) || [])
 										.mergeMap(definition => {
-											const symbol = defInfoToSymbolDescriptor(definition, this.root);
+											const symbol = definitionInfoToSymbolDescriptor(definition, this.root);
 											// Check if SymbolDescriptor without PackageDescriptor matches
 											const score = getMatchingPropertyCount(queryWithoutPackage, symbol);
 											if (score < minScore || (params.query.package && !definition.fileName.includes(params.query.package.name))) {

src/util.ts

@@ -147,35 +147,6 @@ export function isDeclarationFile(filename: string): boolean {
 	return filename.endsWith('.d.ts');
 }
 
-/**
- * Converts a ts.DefinitionInfo to a SymbolDescriptor
- */
-export function defInfoToSymbolDescriptor(info: ts.DefinitionInfo, rootPath: string): SymbolDescriptor {
-	const rootUnixPath = toUnixPath(rootPath);
-	const symbolDescriptor: SymbolDescriptor = {
-		kind: info.kind || '',
-		name: info.name || '',
-		containerKind: info.containerKind || '',
-		containerName: info.containerName || '',
-		filePath: info.fileName
-	};
-	// If the symbol is an external module representing a file, set name to the file path
-	if (info.kind === ts.ScriptElementKind.moduleElement && info.name && /[\\\/]/.test(info.name)) {
-		symbolDescriptor.name = '"' + info.fileName.replace(/(?:\.d)?\.tsx?$/, '') + '"';
-	}
-	// If the symbol itself is not a module and there is no containerKind
-	// then the container is an external module named by the file name (without file extension)
-	if (info.kind !== ts.ScriptElementKind.moduleElement && !info.containerKind && !info.containerName) {
-		symbolDescriptor.containerName = '"' + info.fileName.replace(/(?:\.d)?\.tsx?$/, '') + '"';
-		symbolDescriptor.containerKind = ts.ScriptElementKind.moduleElement;
-	}
-	// Make paths relative to root paths
-	symbolDescriptor.containerName = symbolDescriptor.containerName.replace(rootUnixPath, '');
-	symbolDescriptor.name = symbolDescriptor.name.replace(rootUnixPath, '');
-	symbolDescriptor.filePath = symbolDescriptor.filePath.replace(rootUnixPath, '');
-	return symbolDescriptor;
-}
-
 /**
  * Compares two values and returns a numeric score between 0 and 1 defining of how well they match.
  * E.g. if 2 of 4 properties in the query match, will return 2