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

Commit 4544f03

Browse files
felixfbeckerfelixfbecker
authored
Fix workspace/xreferences (#288)
* Fix workspace/xreferences * Add test for query with PackageDescriptor
1 parent d8a39a1 commit 4544f03

File tree

2 files changed

+70
-29
lines changed

2 files changed

+70
-29
lines changed

‎src/test/typescript-service-helpers.ts‎

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,9 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
561561
[rootUri + 'a.ts', 'class a { foo() { const i = 1;} }'],
562562
[rootUri + 'foo/b.ts', 'class b { bar: number; baz(): number { return this.bar;}}; function qux() {}'],
563563
[rootUri + 'c.ts', 'import { x } from "dep/dep";'],
564-
[rootUri + 'package.json', '{ "name": "mypkg" }'],
565-
[rootUri + 'node_modules/dep/dep.ts', 'export var x = 1;']
564+
[rootUri + 'package.json', JSON.stringify({ name: 'mypkg' })],
565+
[rootUri + 'node_modules/dep/dep.ts', 'export var x = 1;'],
566+
[rootUri + 'node_modules/dep/package.json', JSON.stringify({ name: 'dep' })]
566567
])));
567568

568569
afterEach(shutdownService);
@@ -865,7 +866,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
865866
line: 0
866867
},
867868
start: {
868-
character: 10,
869+
character: 9,
869870
line: 0
870871
}
871872
},
@@ -892,7 +893,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
892893
line: 0
893894
},
894895
start: {
895-
character: 10,
896+
character: 9,
896897
line: 0
897898
}
898899
},
@@ -905,7 +906,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
905906
assert.deepEqual(result, []);
906907
});
907908
it('should return all references to a symbol from a dependency', async function (this: TestContext & ITestCallbackContext) {
908-
const result: ReferenceInformation[] = await this.service.workspaceXreferences({ query: { name: 'x', containerName: '' } })
909+
const result: ReferenceInformation[] = await this.service.workspaceXreferences({ query: { name: 'x' } })
909910
.reduce<jsonpatch.Operation, ReferenceInformation[]>(jsonpatch.applyReducer, null as any)
910911
.toPromise();
911912
assert.deepEqual(result, [{
@@ -916,7 +917,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
916917
line: 0
917918
},
918919
start: {
919-
character: 9,
920+
character: 8,
920921
line: 0
921922
}
922923
},
@@ -931,6 +932,38 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
931932
}
932933
}]);
933934
});
935+
it('should return all references to a symbol from a dependency with PackageDescriptor query', async function (this: TestContext & ITestCallbackContext) {
936+
const result: ReferenceInformation[] = await this.service.workspaceXreferences({ query: { name: 'x', package: { name: 'dep' } } })
937+
.reduce<jsonpatch.Operation, ReferenceInformation[]>(jsonpatch.applyReducer, null as any)
938+
.toPromise();
939+
assert.deepEqual(result, [{
940+
reference: {
941+
range: {
942+
end: {
943+
character: 10,
944+
line: 0
945+
},
946+
start: {
947+
character: 8,
948+
line: 0
949+
}
950+
},
951+
uri: rootUri + 'c.ts'
952+
},
953+
symbol: {
954+
filePath: 'node_modules/dep/dep.ts',
955+
containerKind: '',
956+
containerName: '"node_modules/dep/dep"',
957+
kind: 'var',
958+
name: 'x',
959+
package: {
960+
name: 'dep',
961+
repoURL: undefined,
962+
version: undefined
963+
}
964+
}
965+
}]);
966+
});
934967
it('should return all references to all symbols if empty SymbolDescriptor query is passed', async function (this: TestContext & ITestCallbackContext) {
935968
const result: ReferenceInformation[] = await this.service.workspaceXreferences({ query: {} })
936969
.reduce<jsonpatch.Operation, ReferenceInformation[]>(jsonpatch.applyReducer, null as any)
@@ -951,7 +984,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
951984
line: 0
952985
},
953986
start: {
954-
character: 6,
987+
character: 5,
955988
line: 0
956989
}
957990
},
@@ -973,7 +1006,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
9731006
line: 0
9741007
},
9751008
start: {
976-
character: 10,
1009+
character: 9,
9771010
line: 0
9781011
}
9791012
},
@@ -995,7 +1028,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
9951028
line: 0
9961029
},
9971030
start: {
998-
character: 24,
1031+
character: 23,
9991032
line: 0
10001033
}
10011034
},
@@ -1010,7 +1043,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
10101043
line: 0
10111044
},
10121045
start: {
1013-
character: 9,
1046+
character: 8,
10141047
line: 0
10151048
}
10161049
},
@@ -1039,7 +1072,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
10391072
line: 0
10401073
},
10411074
start: {
1042-
character: 6,
1075+
character: 5,
10431076
line: 0
10441077
}
10451078
},
@@ -1061,7 +1094,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
10611094
line: 0
10621095
},
10631096
start: {
1064-
character: 10,
1097+
character: 9,
10651098
line: 0
10661099
}
10671100
},
@@ -1083,7 +1116,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
10831116
line: 0
10841117
},
10851118
start: {
1086-
character: 23,
1119+
character: 22,
10871120
line: 0
10881121
}
10891122
},
@@ -1105,7 +1138,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
11051138
line: 0
11061139
},
11071140
start: {
1108-
character: 52,
1141+
character: 51,
11091142
line: 0
11101143
}
11111144
},
@@ -1127,7 +1160,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
11271160
line: 0
11281161
},
11291162
start: {
1130-
character: 68,
1163+
character: 67,
11311164
line: 0
11321165
}
11331166
},

‎src/typescript-service.ts‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
InitializeResult,
4545
PackageDescriptor,
4646
PackageInformation,
47+
ReferenceInformation,
4748
SymbolDescriptor,
4849
SymbolLocationInformation,
4950
WorkspaceReferenceParams,
@@ -53,7 +54,6 @@ import {
5354
defInfoToSymbolDescriptor,
5455
getMatchingPropertyCount,
5556
getPropertyCount,
56-
isSymbolDescriptorMatch,
5757
JSONPTR,
5858
normalizeUri,
5959
observableFromIterable,
@@ -761,14 +761,21 @@ export class TypeScriptService {
761761
*/
762762
workspaceXreferences(params: WorkspaceReferenceParams, span = new Span()): Observable<jsonpatch.Operation> {
763763
const queryWithoutPackage = omit(params.query, 'package');
764-
return Observable.from(this.projectManager.ensureAllFiles(span))
765-
.mergeMap<void, ProjectConfiguration>(() => {
764+
const minScore = Math.min(4.75, getPropertyCount(queryWithoutPackage));
765+
return this.isDefinitelyTyped
766+
.mergeMap(isDefinitelyTyped => {
767+
if (isDefinitelyTyped) {
768+
throw new Error('workspace/xreferences not supported in DefinitelyTyped');
769+
}
770+
return this.projectManager.ensureAllFiles(span);
771+
})
772+
.mergeMap(() => {
766773
// if we were hinted that we should only search a specific package, find it and only search the owning tsconfig.json
767774
if (params.hints && params.hints.dependeePackageName) {
768775
return observableFromIterable(this.packageManager.packageJsonUris())
769776
.filter(uri => (JSON.parse(this.inMemoryFileSystem.getContent(uri)) as PackageJson).name === params.hints!.dependeePackageName)
770777
.take(1)
771-
.mergeMap<string, ProjectConfiguration>(uri => {
778+
.mergeMap(uri => {
772779
const config = this.projectManager.getParentConfiguration(uri);
773780
if (!config) {
774781
return observableFromIterable(this.projectManager.configurations());
@@ -796,35 +803,36 @@ export class TypeScriptService {
796803
.filter((node): node is ts.Identifier => node.kind === ts.SyntaxKind.Identifier)
797804
.mergeMap(node => {
798805
try {
799-
// Get DefinitionInformations at the node
806+
// Find definition for node
800807
return Observable.from(config.getService().getDefinitionAtPosition(source.fileName, node.pos + 1) || [])
801808
.mergeMap(definition => {
802809
const symbol = defInfoToSymbolDescriptor(definition, this.root);
803810
// Check if SymbolDescriptor without PackageDescriptor matches
804-
if (!isSymbolDescriptorMatch(queryWithoutPackage, symbol)) {
811+
const score = getMatchingPropertyCount(queryWithoutPackage, symbol);
812+
if (score < minScore || (params.query.package && !definition.fileName.includes(params.query.package.name))) {
805813
return [];
806814
}
815+
span.log({ event: 'match', score });
807816
// If no PackageDescriptor query, return match
808817
if (!params.query.package || !params.query.package) {
809818
return [symbol];
810819
}
811820
// If SymbolDescriptor matched and the query contains a PackageDescriptor, get package.json and match PackageDescriptor name
812-
// TODO match full PackageDescriptor (version)
821+
// TODO match full PackageDescriptor (version) and fill out the symbol.package field
813822
const uri = path2uri(definition.fileName);
814-
return this._getPackageDescriptor(uri)
815-
.mergeMap(packageDescriptor => {
823+
return Observable.from(this._getPackageDescriptor(uri, span))
824+
.filter(packageDescriptor => !!(packageDescriptor && packageDescriptor.name === params.query.package!.name!))
825+
.map(packageDescriptor => {
816826
symbol.package = packageDescriptor;
817-
return packageDescriptor && packageDescriptor.name === params.query.package!.name!
818-
? [symbol]
819-
: [];
827+
return symbol;
820828
});
821829
})
822-
.map(symbol => ({
830+
.map((symbol: SymbolDescriptor): ReferenceInformation => ({
823831
symbol,
824832
reference: {
825833
uri: locationUri(source.fileName),
826834
range: {
827-
start: ts.getLineAndCharacterOfPosition(source, node.pos + 1),
835+
start: ts.getLineAndCharacterOfPosition(source, node.pos),
828836
end: ts.getLineAndCharacterOfPosition(source, node.end)
829837
}
830838
}

0 commit comments

Comments
 (0)