Match SymbolDescriptors with a numeric score

src/test/util-test.ts

@@ -1,7 +1,55 @@
 import * as assert from 'assert';
-import { isGlobalTSFile, isSymbolDescriptorMatch } from '../util';
+import { getMatchScore, isGlobalTSFile, isSymbolDescriptorMatch } from '../util';
 
 describe('util', () => {
+	describe('getMatchScore()', () => {
+		it('should return a score of 4 if 4 properties match', () => {
+			const score = getMatchScore({
+				containerName: 'ts',
+				kind: 'interface',
+				name: 'Program',
+				package: undefined
+			}, {
+				containerKind: 'module',
+				containerName: 'ts',
+				kind: 'interface',
+				name: 'Program',
+				package: undefined
+			});
+			assert.equal(score, 4);
+		});
+		it('should return a score of 4 if 4 properties match and 1 does not', () => {
+			const score = getMatchScore({
+				containerKind: '',
+				containerName: 'util',
+				kind: 'var',
+				name: 'colors',
+				package: undefined
+			}, {
+				containerKind: '',
+				containerName: '',
+				kind: 'var',
+				name: 'colors',
+				package: undefined
+			});
+			assert.equal(score, 4);
+		});
+		it('should return a score of 3 if 3 properties match deeply', () => {
+			const score = getMatchScore({
+				name: 'a',
+				kind: 'class',
+				package: { name: 'mypkg' },
+				containerKind: undefined
+			}, {
+				kind: 'class',
+				name: 'a',
+				containerKind: '',
+				containerName: '',
+				package: { name: 'mypkg' }
+			});
+			assert.equal(score, 3);
+		});
+	});
 	describe('isSymbolDescriptorMatch()', () => {
 		it('should return true for a matching query', () => {
 			const matches = isSymbolDescriptorMatch({

src/typescript-service.ts

@@ -46,6 +46,7 @@ import {
 import {
 	convertStringtoSymbolKind,
 	defInfoToSymbolDescriptor,
+	getMatchScore,
 	isLocalUri,
 	isSymbolDescriptorMatch,
 	normalizeUri,
@@ -576,33 +577,52 @@ export class TypeScriptService {
 		// TODO stream 50 results, then re-query and stream the rest
 		const limit = Math.min(params.limit || Infinity, 50);
 
-		const query = params.query;
-		const symbolQuery = params.symbol && { ...params.symbol };
-
-		if (symbolQuery && symbolQuery.package) {
-			// Strip all fields except name from PackageDescriptor
-			symbolQuery.package = { name: symbolQuery.package.name };
-		}
-
 		// Return cached result for empty query, if available
-		if (!query && !symbolQuery && this.emptyQueryWorkspaceSymbols) {
+		if (!params.query && !params.symbol && this.emptyQueryWorkspaceSymbols) {
 			return this.emptyQueryWorkspaceSymbols;
 		}
 
-		// Use special logic for DefinitelyTyped
-		return this.isDefinitelyTyped
-			.mergeMap(isDefinitelyTyped => {
+		/** A sorted array that keeps track of symbol match scores to determine the index to insert the symbol at */
+		const scores: number[] = [];
+
+		let observable = this.isDefinitelyTyped
+			.mergeMap((isDefinitelyTyped: boolean): Observable<[number, SymbolInformation]> => {
+				// Use special logic for DefinitelyTyped
+				// Search only in the correct subdirectory for the given PackageDescriptor
 				if (isDefinitelyTyped) {
-					return this._workspaceSymbolDefinitelyTyped({ ...params, limit }, span);
-				}
+					// Error if not passed a SymbolDescriptor query with an `@types` PackageDescriptor
+					if (!params.symbol || !params.symbol.package || !params.symbol.package.name || !params.symbol.package.name.startsWith('@types/')) {
+						return Observable.throw('workspace/symbol on DefinitelyTyped is only supported with a SymbolDescriptor query with an @types PackageDescriptor');
+					}
 
-				// A workspace/symol request searches all symbols in own code, but not in dependencies
-				let patches = Observable.from(this.projectManager.ensureOwnFiles(span))
-					.mergeMap(() =>
-						symbolQuery && symbolQuery.package
+					// Fetch all files in the package subdirectory
+					// All packages are in the types/ subdirectory
+					const normRootUri = this.rootUri.endsWith('/') ? this.rootUri : this.rootUri + '/';
+					const packageRootUri = normRootUri + params.symbol.package.name.substr(1) + '/';
+
+					return Observable.from(this.updater.ensureStructure(span))
+						.mergeMap(() => Observable.from<string>(this.inMemoryFileSystem.uris() as any))
+						.filter(uri => uri.startsWith(packageRootUri))
+						.mergeMap(uri => this.updater.ensure(uri, span))
+						.toArray()
+						.mergeMap(() => {
+							span.log({ event: 'fetched package files' });
+							const config = this.projectManager.getParentConfiguration(packageRootUri, 'ts');
+							if (!config) {
+								throw new Error(`Could not find tsconfig for ${packageRootUri}`);
+							}
+							// Don't match PackageDescriptor on symbols
+							return this._getSymbolsInConfig(config, params.query || omit(params.symbol!, 'package'), limit, span);
+						});
+				}
+				// Regular workspace symbol search
+				// Search all symbols in own code, but not in dependencies
+				return Observable.from(this.projectManager.ensureOwnFiles(span))
+					.mergeMap<void, ProjectConfiguration>(() =>
+						params.symbol && params.symbol.package && params.symbol.package.name
 							// If SymbolDescriptor query with PackageDescriptor, search for package.jsons with matching package name
 							? Observable.from<string>(this.packageManager.packageJsonUris() as any)
-								.filter(packageJsonUri => !symbolQuery || !symbolQuery.package || !symbolQuery.package.name || (JSON.parse(this.inMemoryFileSystem.getContent(packageJsonUri)) as PackageJson).name === symbolQuery.package!.name)
+								.filter(packageJsonUri => (JSON.parse(this.inMemoryFileSystem.getContent(packageJsonUri)) as PackageJson).name === params.symbol!.package!.name)
 								// Find their parent and child tsconfigs
 								.mergeMap(packageJsonUri => Observable.merge(
 									castArray<ProjectConfiguration>(this.projectManager.getParentConfiguration(packageJsonUri) || []),
@@ -613,70 +633,32 @@ export class TypeScriptService {
 							: this.projectManager.configurations() as any
 					)
 					// If PackageDescriptor is given, only search project with the matching package name
-					.mergeMap<ProjectConfiguration, SymbolInformation>(config => this._collectWorkspaceSymbols(config, query || symbolQuery, limit, span) as any)
-					.filter(symbol => !symbol.location.uri.includes('/node_modules/'))
-					// Filter duplicate symbols
-					// There may be few configurations that contain the same file(s)
-					// or files from different configurations may refer to the same file(s)
-					.distinct(symbol => hashObject(symbol, { respectType: false } as any))
-					.take(limit)
-					.map(symbol => ({ op: 'add', path: '/-', value: symbol }) as AddPatch)
-					.startWith({ op: 'add', path: '', value: [] });
-
-				if (!query && !symbolQuery) {
-					patches = this.emptyQueryWorkspaceSymbols = patches.publishReplay().refCount();
+					.mergeMap(config => this._getSymbolsInConfig(config, params.query || params.symbol, limit, span));
+			})
+			// Filter symbols found in dependencies
+			.filter(([score, symbol]) => !symbol.location.uri.includes('/node_modules/'))
+			// Filter duplicate symbols
+			// There may be few configurations that contain the same file(s)
+			// or files from different configurations may refer to the same file(s)
+			.distinct(symbol => hashObject(symbol, { respectType: false } as any))
+			.take(limit)
+			// Find out at which index to insert the symbol to maintain sorting order by score
+			.map(([score, symbol]) => {
+				const index = scores.findIndex(s => s < score);
+				if (index === -1) {
+					scores.push(score);
+					return { op: 'add', path: '/-', value: symbol } as AddPatch;
 				}
+				scores.splice(index, 0, score);
+				return { op: 'add', path: '/' + index, value: symbol } as AddPatch;
+			})
+			.startWith({ op: 'add', path: '', value: [] });
 
-				return patches;
-			});
-	}
-
-	/**
-	 * Specialised version of workspaceSymbol for DefinitelyTyped.
-	 * Searches only in the correct subdirectory for the given PackageDescriptor.
-	 * Will error if not passed a SymbolDescriptor query with an `@types` PackageDescriptor
-	 *
-	 * @return Observable of JSON Patches that build a `SymbolInformation[]` result
-	 */
-	protected _workspaceSymbolDefinitelyTyped(params: WorkspaceSymbolParams, childOf = new Span()): Observable<OpPatch> {
-		const span = childOf.tracer().startSpan('Handle workspace/symbol DefinitelyTyped', { childOf });
-
-		if (!params.symbol || !params.symbol.package || !params.symbol.package.name || !params.symbol.package.name.startsWith('@types/')) {
-			return Observable.throw('workspace/symbol on DefinitelyTyped is only supported with a SymbolDescriptor query with an @types PackageDescriptor');
+		if (!params.query && !params.symbol) {
+			observable = this.emptyQueryWorkspaceSymbols = observable.publishReplay().refCount();
 		}
 
-		const symbolQuery = { ...params.symbol };
-		// Don't match PackageDescriptor on symbols
-		symbolQuery.package = undefined;
-
-		// Fetch all files in the package subdirectory
-		// All packages are in the types/ subdirectory
-		const normRootUri = this.rootUri.endsWith('/') ? this.rootUri : this.rootUri + '/';
-		const packageRootUri = normRootUri + params.symbol.package.name.substr(1) + '/';
-
-		return Observable.from(this.updater.ensureStructure(span))
-			.mergeMap(() => Observable.from<string>(this.inMemoryFileSystem.uris() as any))
-			.filter(uri => uri.startsWith(packageRootUri))
-			.mergeMap(uri => this.updater.ensure(uri, span))
-			.toArray()
-			.mergeMap<any, SymbolInformation>(() => {
-				span.log({ event: 'fetched package files' });
-
-				// Search symbol in configuration
-				// forcing TypeScript mode
-				const config = this.projectManager.getConfiguration(uri2path(packageRootUri), 'ts');
-				return this._collectWorkspaceSymbols(config, params.query || symbolQuery, params.limit, span) as any;
-			})
-			.map(symbol => ({ op: 'add', path: '/-', value: symbol }) as AddPatch)
-			.startWith({ op: 'add', path: '', value: [] })
-			.catch<OpPatch, never>(err => {
-				span.setTag('error', true);
-				span.log({ 'event': 'error', 'error.object': err, 'message': err.message, 'stack': err.stack });
-				throw err;
-			})
-			.finally(() => {
-				span.finish();
-			});
+		return observable;
 	}
 
 	/**
@@ -1128,8 +1110,8 @@ export class TypeScriptService {
 	 * @param limit An optional limit that is passed to TypeScript
 	 * @return Observable of SymbolInformations
 	 */
-	private _collectWorkspaceSymbols(config: ProjectConfiguration, query?: string | Partial<SymbolDescriptor>, limit = Infinity, childOf = new Span()): Observable<SymbolInformation> {
-		const span = childOf.tracer().startSpan('Collect workspace symbols', { childOf });
+	protected _getSymbolsInConfig(config: ProjectConfiguration, query?: string | Partial<SymbolDescriptor>, limit = Infinity, childOf = new Span()): Observable<[number, SymbolInformation]> {
+		const span = childOf.tracer().startSpan('Get symbols in config', { childOf });
 		span.addTags({ config: config.configFilePath, query, limit });
 
 		return (() => {
@@ -1142,35 +1124,40 @@ export class TypeScriptService {
 				}
 
 				if (query) {
-					let items: Observable<ts.NavigateToItem>;
+					let items: Observable<[number, ts.NavigateToItem]>;
 					if (typeof query === 'string') {
 						// Query by text query
-						items = Observable.from(config.getService().getNavigateToItems(query, limit, undefined, false));
+						items = Observable.from(config.getService().getNavigateToItems(query, limit, undefined, false))
+							// Same score for all
+							.map(item => [1, item]);
 					} else {
 						const queryWithoutPackage = omit(query, 'package') as SymbolDescriptor;
 						// Query by name
 						items = Observable.from(config.getService().getNavigateToItems(query.name || '', limit, undefined, false))
-							// First filter to match SymbolDescriptor, ignoring PackageDescriptor
-							.filter(item => isSymbolDescriptorMatch(queryWithoutPackage, {
+							// Get a score how good the symbol matches the SymbolDescriptor (ignoring PackageDescriptor)
+							.map((item): [number, ts.NavigateToItem] => [getMatchScore(queryWithoutPackage, {
 								kind: item.kind,
 								name: item.name,
 								containerKind: item.containerKind,
 								containerName: item.containerName
-							}))
-							// if SymbolDescriptor matched, get package.json and match PackageDescriptor name
+							}), item])
+							// If score === 0, no properties matched
+							.filter(([score, symbol]) => score > 0)
+							// If SymbolDescriptor matched, get package.json and match PackageDescriptor name
 							// TODO get and match full PackageDescriptor (version)
-							.mergeMap(item => {
+							.mergeMap(([score, item]) => {
 								if (!query.package || !query.package.name) {
-									return [item];
+									return [[score, item]];
 								}
 								const uri = path2uri('', item.fileName);
 								return Observable.from(this.packageManager.getClosestPackageJson(uri, span))
-									.mergeMap(packageJson => packageJson && packageJson.name === query.package!.name! ? [item] : []);
+									// If PackageDescriptor matches, increase score
+									.map((packageJson): [number, ts.NavigateToItem] => packageJson && packageJson.name === query.package!.name! ? [score + 1, item] : [score, item]);
 							});
 					}
 					return Observable.from(items)
 						// Map NavigateToItems to SymbolInformations
-						.map(item => {
+						.map(([score, item]) => {
 							const sourceFile = program.getSourceFile(item.fileName);
 							if (!sourceFile) {
 								throw new Error(`Source file ${item.fileName} does not exist`);
@@ -1189,13 +1176,16 @@ export class TypeScriptService {
 							if (item.containerName) {
 								symbolInformation.containerName = item.containerName;
 							}
-							return symbolInformation;
+							return [score, symbolInformation] as [number, SymbolInformation];
 						})
-						.filter(symbolInformation => isLocalUri(symbolInformation.location.uri));
+						.filter(([score, symbolInformation]) => isLocalUri(symbolInformation.location.uri));
 				} else {
 					// An empty query uses a different algorithm to iterate all files and aggregate the symbols per-file to get all symbols
 					// TODO make all implementations use this? It has the advantage of being streamable and cancellable
-					return Observable.from<SymbolInformation>(this._getNavigationTreeItems(config) as any).take(limit);
+					return Observable.from<SymbolInformation>(this._getNavigationTreeItems(config) as any)
+						// Same score for all
+						.map(symbol => [1, symbol])
+						.take(limit);
 				}
 			} catch (err) {
 				return Observable.throw(err);

src/util.ts

@@ -220,6 +220,23 @@ export function defInfoToSymbolDescriptor(d: ts.DefinitionInfo): SymbolDescripto
 	};
 }
 
+/**
+ * Compares two values and returns a numeric score defining of how well they match.
+ * Every property that matches increases the score by 1.
+ */
+export function getMatchScore(query: any, value: any): number {
+	// If query is a scalar value, compare by identity and return 0 or 1
+	if (typeof query !== 'object' || query === null) {
+		return +(query === value);
+	}
+	// If value is scalar, return no match
+	if (typeof value !== 'object' && value !== null) {
+		return 0;
+	}
+	// Both values are objects, compare each property and sum the scores
+	return Object.keys(query).reduce((score, key) => score + getMatchScore(query[key], value[key]), 0);
+}
+
 /**
  * Returns true if the passed SymbolDescriptor has at least the same properties as the passed partial SymbolDescriptor
  */