Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Commit 61edbfa

Browse files
felixfbeckerfelixfbecker
authored
Match SymbolDescriptors with a numeric score (#249)
* Match SymbolDescriptors with a numeric score * Refactor _workspaceSymbolsDefinitelyTyped
1 parent 168c60f commit 61edbfa

File tree

3 files changed

+150
-95
lines changed

3 files changed

+150
-95
lines changed

‎src/test/util-test.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,55 @@
11
import * as assert from 'assert';
2-
import { isGlobalTSFile, isSymbolDescriptorMatch } from '../util';
2+
import { getMatchScore, isGlobalTSFile, isSymbolDescriptorMatch } from '../util';
33

44
describe('util', () => {
5+
describe('getMatchScore()', () => {
6+
it('should return a score of 4 if 4 properties match', () => {
7+
const score = getMatchScore({
8+
containerName: 'ts',
9+
kind: 'interface',
10+
name: 'Program',
11+
package: undefined
12+
}, {
13+
containerKind: 'module',
14+
containerName: 'ts',
15+
kind: 'interface',
16+
name: 'Program',
17+
package: undefined
18+
});
19+
assert.equal(score, 4);
20+
});
21+
it('should return a score of 4 if 4 properties match and 1 does not', () => {
22+
const score = getMatchScore({
23+
containerKind: '',
24+
containerName: 'util',
25+
kind: 'var',
26+
name: 'colors',
27+
package: undefined
28+
}, {
29+
containerKind: '',
30+
containerName: '',
31+
kind: 'var',
32+
name: 'colors',
33+
package: undefined
34+
});
35+
assert.equal(score, 4);
36+
});
37+
it('should return a score of 3 if 3 properties match deeply', () => {
38+
const score = getMatchScore({
39+
name: 'a',
40+
kind: 'class',
41+
package: { name: 'mypkg' },
42+
containerKind: undefined
43+
}, {
44+
kind: 'class',
45+
name: 'a',
46+
containerKind: '',
47+
containerName: '',
48+
package: { name: 'mypkg' }
49+
});
50+
assert.equal(score, 3);
51+
});
52+
});
553
describe('isSymbolDescriptorMatch()', () => {
654
it('should return true for a matching query', () => {
755
const matches = isSymbolDescriptorMatch({

‎src/typescript-service.ts

Lines changed: 84 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
import {
4747
convertStringtoSymbolKind,
4848
defInfoToSymbolDescriptor,
49+
getMatchScore,
4950
isLocalUri,
5051
isSymbolDescriptorMatch,
5152
normalizeUri,
@@ -576,33 +577,52 @@ export class TypeScriptService {
576577
// TODO stream 50 results, then re-query and stream the rest
577578
const limit = Math.min(params.limit || Infinity, 50);
578579

579-
const query = params.query;
580-
const symbolQuery = params.symbol && { ...params.symbol };
581-
582-
if (symbolQuery && symbolQuery.package) {
583-
// Strip all fields except name from PackageDescriptor
584-
symbolQuery.package = { name: symbolQuery.package.name };
585-
}
586-
587580
// Return cached result for empty query, if available
588-
if (!query && !symbolQuery && this.emptyQueryWorkspaceSymbols) {
581+
if (!params.query && !params.symbol && this.emptyQueryWorkspaceSymbols) {
589582
return this.emptyQueryWorkspaceSymbols;
590583
}
591584

592-
// Use special logic for DefinitelyTyped
593-
return this.isDefinitelyTyped
594-
.mergeMap(isDefinitelyTyped => {
585+
/** A sorted array that keeps track of symbol match scores to determine the index to insert the symbol at */
586+
const scores: number[] = [];
587+
588+
let observable = this.isDefinitelyTyped
589+
.mergeMap((isDefinitelyTyped: boolean): Observable<[number, SymbolInformation]> => {
590+
// Use special logic for DefinitelyTyped
591+
// Search only in the correct subdirectory for the given PackageDescriptor
595592
if (isDefinitelyTyped) {
596-
return this._workspaceSymbolDefinitelyTyped({ ...params, limit }, span);
597-
}
593+
// Error if not passed a SymbolDescriptor query with an `@types` PackageDescriptor
594+
if (!params.symbol || !params.symbol.package || !params.symbol.package.name || !params.symbol.package.name.startsWith('@types/')) {
595+
return Observable.throw('workspace/symbol on DefinitelyTyped is only supported with a SymbolDescriptor query with an @types PackageDescriptor');
596+
}
598597

599-
// A workspace/symol request searches all symbols in own code, but not in dependencies
600-
let patches = Observable.from(this.projectManager.ensureOwnFiles(span))
601-
.mergeMap(() =>
602-
symbolQuery && symbolQuery.package
598+
// Fetch all files in the package subdirectory
599+
// All packages are in the types/ subdirectory
600+
const normRootUri = this.rootUri.endsWith('/') ? this.rootUri : this.rootUri + '/';
601+
const packageRootUri = normRootUri + params.symbol.package.name.substr(1) + '/';
602+
603+
return Observable.from(this.updater.ensureStructure(span))
604+
.mergeMap(() => Observable.from<string>(this.inMemoryFileSystem.uris() as any))
605+
.filter(uri => uri.startsWith(packageRootUri))
606+
.mergeMap(uri => this.updater.ensure(uri, span))
607+
.toArray()
608+
.mergeMap(() => {
609+
span.log({ event: 'fetched package files' });
610+
const config = this.projectManager.getParentConfiguration(packageRootUri, 'ts');
611+
if (!config) {
612+
throw new Error(`Could not find tsconfig for ${packageRootUri}`);
613+
}
614+
// Don't match PackageDescriptor on symbols
615+
return this._getSymbolsInConfig(config, params.query || omit(params.symbol!, 'package'), limit, span);
616+
});
617+
}
618+
// Regular workspace symbol search
619+
// Search all symbols in own code, but not in dependencies
620+
return Observable.from(this.projectManager.ensureOwnFiles(span))
621+
.mergeMap<void, ProjectConfiguration>(() =>
622+
params.symbol && params.symbol.package && params.symbol.package.name
603623
// If SymbolDescriptor query with PackageDescriptor, search for package.jsons with matching package name
604624
? Observable.from<string>(this.packageManager.packageJsonUris() as any)
605-
.filter(packageJsonUri => !symbolQuery || !symbolQuery.package || !symbolQuery.package.name || (JSON.parse(this.inMemoryFileSystem.getContent(packageJsonUri)) as PackageJson).name === symbolQuery.package!.name)
625+
.filter(packageJsonUri => (JSON.parse(this.inMemoryFileSystem.getContent(packageJsonUri)) as PackageJson).name === params.symbol!.package!.name)
606626
// Find their parent and child tsconfigs
607627
.mergeMap(packageJsonUri => Observable.merge(
608628
castArray<ProjectConfiguration>(this.projectManager.getParentConfiguration(packageJsonUri) || []),
@@ -613,70 +633,32 @@ export class TypeScriptService {
613633
: this.projectManager.configurations() as any
614634
)
615635
// If PackageDescriptor is given, only search project with the matching package name
616-
.mergeMap<ProjectConfiguration, SymbolInformation>(config => this._collectWorkspaceSymbols(config, query || symbolQuery, limit, span) as any)
617-
.filter(symbol => !symbol.location.uri.includes('/node_modules/'))
618-
// Filter duplicate symbols
619-
// There may be few configurations that contain the same file(s)
620-
// or files from different configurations may refer to the same file(s)
621-
.distinct(symbol => hashObject(symbol, { respectType: false } as any))
622-
.take(limit)
623-
.map(symbol => ({ op: 'add', path: '/-', value: symbol }) as AddPatch)
624-
.startWith({ op: 'add', path: '', value: [] });
625-
626-
if (!query && !symbolQuery) {
627-
patches = this.emptyQueryWorkspaceSymbols = patches.publishReplay().refCount();
636+
.mergeMap(config => this._getSymbolsInConfig(config, params.query || params.symbol, limit, span));
637+
})
638+
// Filter symbols found in dependencies
639+
.filter(([score, symbol]) => !symbol.location.uri.includes('/node_modules/'))
640+
// Filter duplicate symbols
641+
// There may be few configurations that contain the same file(s)
642+
// or files from different configurations may refer to the same file(s)
643+
.distinct(symbol => hashObject(symbol, { respectType: false } as any))
644+
.take(limit)
645+
// Find out at which index to insert the symbol to maintain sorting order by score
646+
.map(([score, symbol]) => {
647+
const index = scores.findIndex(s => s < score);
648+
if (index === -1) {
649+
scores.push(score);
650+
return { op: 'add', path: '/-', value: symbol } as AddPatch;
628651
}
652+
scores.splice(index, 0, score);
653+
return { op: 'add', path: '/' + index, value: symbol } as AddPatch;
654+
})
655+
.startWith({ op: 'add', path: '', value: [] });
629656

630-
return patches;
631-
});
632-
}
633-
634-
/**
635-
* Specialised version of workspaceSymbol for DefinitelyTyped.
636-
* Searches only in the correct subdirectory for the given PackageDescriptor.
637-
* Will error if not passed a SymbolDescriptor query with an `@types` PackageDescriptor
638-
*
639-
* @return Observable of JSON Patches that build a `SymbolInformation[]` result
640-
*/
641-
protected _workspaceSymbolDefinitelyTyped(params: WorkspaceSymbolParams, childOf = new Span()): Observable<OpPatch> {
642-
const span = childOf.tracer().startSpan('Handle workspace/symbol DefinitelyTyped', { childOf });
643-
644-
if (!params.symbol || !params.symbol.package || !params.symbol.package.name || !params.symbol.package.name.startsWith('@types/')) {
645-
return Observable.throw('workspace/symbol on DefinitelyTyped is only supported with a SymbolDescriptor query with an @types PackageDescriptor');
657+
if (!params.query && !params.symbol) {
658+
observable = this.emptyQueryWorkspaceSymbols = observable.publishReplay().refCount();
646659
}
647660

648-
const symbolQuery = { ...params.symbol };
649-
// Don't match PackageDescriptor on symbols
650-
symbolQuery.package = undefined;
651-
652-
// Fetch all files in the package subdirectory
653-
// All packages are in the types/ subdirectory
654-
const normRootUri = this.rootUri.endsWith('/') ? this.rootUri : this.rootUri + '/';
655-
const packageRootUri = normRootUri + params.symbol.package.name.substr(1) + '/';
656-
657-
return Observable.from(this.updater.ensureStructure(span))
658-
.mergeMap(() => Observable.from<string>(this.inMemoryFileSystem.uris() as any))
659-
.filter(uri => uri.startsWith(packageRootUri))
660-
.mergeMap(uri => this.updater.ensure(uri, span))
661-
.toArray()
662-
.mergeMap<any, SymbolInformation>(() => {
663-
span.log({ event: 'fetched package files' });
664-
665-
// Search symbol in configuration
666-
// forcing TypeScript mode
667-
const config = this.projectManager.getConfiguration(uri2path(packageRootUri), 'ts');
668-
return this._collectWorkspaceSymbols(config, params.query || symbolQuery, params.limit, span) as any;
669-
})
670-
.map(symbol => ({ op: 'add', path: '/-', value: symbol }) as AddPatch)
671-
.startWith({ op: 'add', path: '', value: [] })
672-
.catch<OpPatch, never>(err => {
673-
span.setTag('error', true);
674-
span.log({ 'event': 'error', 'error.object': err, 'message': err.message, 'stack': err.stack });
675-
throw err;
676-
})
677-
.finally(() => {
678-
span.finish();
679-
});
661+
return observable;
680662
}
681663

682664
/**
@@ -1128,8 +1110,8 @@ export class TypeScriptService {
11281110
* @param limit An optional limit that is passed to TypeScript
11291111
* @return Observable of SymbolInformations
11301112
*/
1131-
private _collectWorkspaceSymbols(config: ProjectConfiguration, query?: string | Partial<SymbolDescriptor>, limit = Infinity, childOf = new Span()): Observable<SymbolInformation> {
1132-
const span = childOf.tracer().startSpan('Collect workspace symbols', { childOf });
1113+
protected _getSymbolsInConfig(config: ProjectConfiguration, query?: string | Partial<SymbolDescriptor>, limit = Infinity, childOf = new Span()): Observable<[number, SymbolInformation]> {
1114+
const span = childOf.tracer().startSpan('Get symbols in config', { childOf });
11331115
span.addTags({ config: config.configFilePath, query, limit });
11341116

11351117
return (() => {
@@ -1142,35 +1124,40 @@ export class TypeScriptService {
11421124
}
11431125

11441126
if (query) {
1145-
let items: Observable<ts.NavigateToItem>;
1127+
let items: Observable<[number, ts.NavigateToItem]>;
11461128
if (typeof query === 'string') {
11471129
// Query by text query
1148-
items = Observable.from(config.getService().getNavigateToItems(query, limit, undefined, false));
1130+
items = Observable.from(config.getService().getNavigateToItems(query, limit, undefined, false))
1131+
// Same score for all
1132+
.map(item => [1, item]);
11491133
} else {
11501134
const queryWithoutPackage = omit(query, 'package') as SymbolDescriptor;
11511135
// Query by name
11521136
items = Observable.from(config.getService().getNavigateToItems(query.name || '', limit, undefined, false))
1153-
// First filter to match SymbolDescriptor, ignoring PackageDescriptor
1154-
.filter(item => isSymbolDescriptorMatch(queryWithoutPackage, {
1137+
// Get a score how good the symbol matches the SymbolDescriptor (ignoring PackageDescriptor)
1138+
.map((item): [number, ts.NavigateToItem] => [getMatchScore(queryWithoutPackage, {
11551139
kind: item.kind,
11561140
name: item.name,
11571141
containerKind: item.containerKind,
11581142
containerName: item.containerName
1159-
}))
1160-
// if SymbolDescriptor matched, get package.json and match PackageDescriptor name
1143+
}), item])
1144+
// If score === 0, no properties matched
1145+
.filter(([score, symbol]) => score > 0)
1146+
// If SymbolDescriptor matched, get package.json and match PackageDescriptor name
11611147
// TODO get and match full PackageDescriptor (version)
1162-
.mergeMap(item => {
1148+
.mergeMap(([score, item]) => {
11631149
if (!query.package || !query.package.name) {
1164-
return [item];
1150+
return [[score, item]];
11651151
}
11661152
const uri = path2uri('', item.fileName);
11671153
return Observable.from(this.packageManager.getClosestPackageJson(uri, span))
1168-
.mergeMap(packageJson => packageJson && packageJson.name === query.package!.name! ? [item] : []);
1154+
// If PackageDescriptor matches, increase score
1155+
.map((packageJson): [number, ts.NavigateToItem] => packageJson && packageJson.name === query.package!.name! ? [score + 1, item] : [score, item]);
11691156
});
11701157
}
11711158
return Observable.from(items)
11721159
// Map NavigateToItems to SymbolInformations
1173-
.map(item => {
1160+
.map(([score, item]) => {
11741161
const sourceFile = program.getSourceFile(item.fileName);
11751162
if (!sourceFile) {
11761163
throw new Error(`Source file ${item.fileName} does not exist`);
@@ -1189,13 +1176,16 @@ export class TypeScriptService {
11891176
if (item.containerName) {
11901177
symbolInformation.containerName = item.containerName;
11911178
}
1192-
return symbolInformation;
1179+
return [score, symbolInformation] as [number, SymbolInformation];
11931180
})
1194-
.filter(symbolInformation => isLocalUri(symbolInformation.location.uri));
1181+
.filter(([score, symbolInformation]) => isLocalUri(symbolInformation.location.uri));
11951182
} else {
11961183
// An empty query uses a different algorithm to iterate all files and aggregate the symbols per-file to get all symbols
11971184
// TODO make all implementations use this? It has the advantage of being streamable and cancellable
1198-
return Observable.from<SymbolInformation>(this._getNavigationTreeItems(config) as any).take(limit);
1185+
return Observable.from<SymbolInformation>(this._getNavigationTreeItems(config) as any)
1186+
// Same score for all
1187+
.map(symbol => [1, symbol])
1188+
.take(limit);
11991189
}
12001190
} catch (err) {
12011191
return Observable.throw(err);

‎src/util.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,23 @@ export function defInfoToSymbolDescriptor(d: ts.DefinitionInfo): SymbolDescripto
220220
};
221221
}
222222

223+
/**
224+
* Compares two values and returns a numeric score defining of how well they match.
225+
* Every property that matches increases the score by 1.
226+
*/
227+
export function getMatchScore(query: any, value: any): number {
228+
// If query is a scalar value, compare by identity and return 0 or 1
229+
if (typeof query !== 'object' || query === null) {
230+
return +(query === value);
231+
}
232+
// If value is scalar, return no match
233+
if (typeof value !== 'object' && value !== null) {
234+
return 0;
235+
}
236+
// Both values are objects, compare each property and sum the scores
237+
return Object.keys(query).reduce((score, key) => score + getMatchScore(query[key], value[key]), 0);
238+
}
239+
223240
/**
224241
* Returns true if the passed SymbolDescriptor has at least the same properties as the passed partial SymbolDescriptor
225242
*/

0 commit comments

Comments
 (0)