Revert "fix(language-service): only provide template results on reference requests (#41041)" (#41930)

This reverts commit 10aa5641dd.

Issue resolved upstream https://github.com/microsoft/vscode/issues/117095

PR Close #41930
This commit is contained in:
Andrew Scott 2021-05-03 11:08:24 -07:00 committed by Misko Hevery
parent f42796a8f5
commit 3777cf55aa
2 changed files with 62 additions and 61 deletions

View File

@ -366,9 +366,7 @@ export class ReferencesAndRenameBuilder {
entries.set(createLocationKey(entry), entry); entries.set(createLocationKey(entry), entry);
} }
} else { } else {
// TODO(atscott): uncomment when VSCode deduplicates results on their end entries.set(createLocationKey(ref), ref);
// https://github.com/microsoft/vscode/issues/117095
// entries.set(createLocationKey(ref), ref);
} }
} }
return Array.from(entries.values()); return Array.from(entries.values());

View File

@ -43,8 +43,8 @@ describe('find references and rename locations', () => {
it('gets component member references from TS file and external template', () => { it('gets component member references from TS file and external template', () => {
const refs = getReferencesAtPosition(appFile)!; const refs = getReferencesAtPosition(appFile)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.html']); assertFileNames(refs, ['app.html', 'app.ts']);
assertTextSpans(refs, ['myProp']); assertTextSpans(refs, ['myProp']);
}); });
@ -78,8 +78,8 @@ describe('find references and rename locations', () => {
it('gets references', () => { it('gets references', () => {
const refs = getReferencesAtPosition(templateFile)!; const refs = getReferencesAtPosition(templateFile)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.html']); assertFileNames(refs, ['app.html', 'app.ts']);
assertTextSpans(refs, ['myProp']); assertTextSpans(refs, ['myProp']);
}); });
@ -112,7 +112,7 @@ describe('find references and rename locations', () => {
it('gets component member reference in ts file', () => { it('gets component member reference in ts file', () => {
const refs = getReferencesAtPosition(appFile)!; const refs = getReferencesAtPosition(appFile)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts']);
assertTextSpans(refs, ['setTitle']); assertTextSpans(refs, ['setTitle']);
@ -149,7 +149,7 @@ describe('find references and rename locations', () => {
it('gets member reference in ts file', () => { it('gets member reference in ts file', () => {
const refs = getReferencesAtPosition(appFile)!; const refs = getReferencesAtPosition(appFile)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertTextSpans(refs, ['title']); assertTextSpans(refs, ['title']);
}); });
@ -215,9 +215,9 @@ describe('find references and rename locations', () => {
it('gets member reference in ts file', () => { it('gets member reference in ts file', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.html']); assertFileNames(refs, ['app.ts', 'app.html']);
assertTextSpans(refs, ['title']); assertTextSpans(refs, ['title']);
}); });
@ -253,7 +253,7 @@ describe('find references and rename locations', () => {
it('get reference to member in ts file', () => { it('get reference to member in ts file', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts']);
assertTextSpans(refs, ['otherTitle']); assertTextSpans(refs, ['otherTitle']);
@ -290,7 +290,7 @@ describe('find references and rename locations', () => {
it('gets reference to member type definition and initialization in component class', () => { it('gets reference to member type definition and initialization in component class', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
// 3 references: the type definition, the value assignment, and the read in the template // 3 references: the type definition, the value assignment, and the read in the template
expect(refs.length).toBe(1); expect(refs.length).toBe(3);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts']);
// TODO(atscott): investigate if we can make the template keyed read be just the 'name' part. // TODO(atscott): investigate if we can make the template keyed read be just the 'name' part.
@ -298,7 +298,7 @@ describe('find references and rename locations', () => {
// https://sourcegraph.com/github.com/microsoft/TypeScript@d5779c75d3dd19565b60b9e2960b8aac36d4d635/-/blob/src/services/findAllReferences.ts#L508-512 // https://sourcegraph.com/github.com/microsoft/TypeScript@d5779c75d3dd19565b60b9e2960b8aac36d4d635/-/blob/src/services/findAllReferences.ts#L508-512
// One possible solution would be to extend `FullTemplateMapping` to include the matched TCB // One possible solution would be to extend `FullTemplateMapping` to include the matched TCB
// node and then do the same thing that TS does: if the node is a string, adjust the span. // node and then do the same thing that TS does: if the node is a string, adjust the span.
assertTextSpans(refs, ['"name"']); assertTextSpans(refs, ['name', '"name"']);
}); });
it('gets rename locations in component class', () => { it('gets rename locations in component class', () => {
@ -338,9 +338,9 @@ describe('find references and rename locations', () => {
it('get references in ts file', () => { it('get references in ts file', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.html']); assertFileNames(refs, ['app.ts', 'app.html']);
assertTextSpans(refs, ['batman']); assertTextSpans(refs, ['batman']);
}); });
@ -493,8 +493,8 @@ describe('find references and rename locations', () => {
it('should get references', () => { it('should get references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.html']); assertFileNames(refs, ['dir.ts', 'app.html']);
assertTextSpans(refs, ['dirValue']); assertTextSpans(refs, ['dirValue']);
}); });
@ -523,8 +523,8 @@ describe('find references and rename locations', () => {
it('should get references', () => { it('should get references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.html']); assertFileNames(refs, ['dir.ts', 'app.html']);
assertTextSpans(refs, ['dirValue']); assertTextSpans(refs, ['dirValue']);
}); });
@ -553,8 +553,8 @@ describe('find references and rename locations', () => {
it('should get references', () => { it('should get references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.html']); assertFileNames(refs, ['dir.ts', 'app.html']);
assertTextSpans(refs, ['doSomething']); assertTextSpans(refs, ['doSomething']);
}); });
@ -693,8 +693,8 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts', 'example-directive.ts']);
assertTextSpans(refs, ['identifier']); assertTextSpans(refs, ['identifier']);
}); });
@ -727,7 +727,7 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts']);
assertTextSpans(refs, ['name']); assertTextSpans(refs, ['name']);
}); });
@ -777,9 +777,9 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(5);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['index.d.ts', 'prefix-pipe.ts', 'app.ts']);
assertTextSpans(refs, ['prefixPipe']); assertTextSpans(refs, ['transform', 'prefixPipe']);
}); });
it('should find rename locations', () => { it('should find rename locations', () => {
@ -817,7 +817,7 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts']);
assertTextSpans(refs, ['prefix']); assertTextSpans(refs, ['prefix']);
}); });
@ -862,8 +862,8 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toEqual(1); expect(refs.length).toEqual(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['string-model.ts', 'app.ts']);
assertTextSpans(refs, ['model']); assertTextSpans(refs, ['model']);
}); });
@ -947,8 +947,8 @@ describe('find references and rename locations', () => {
it('should work for text attributes', () => { it('should work for text attributes', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toEqual(1); expect(refs.length).toEqual(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['string-model.ts', 'app.ts']);
assertTextSpans(refs, ['model']); assertTextSpans(refs, ['model']);
}); });
@ -988,8 +988,8 @@ describe('find references and rename locations', () => {
it('should work from the TS input declaration', () => { it('should work from the TS input declaration', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toEqual(1); expect(refs.length).toEqual(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts', 'string-model.ts']);
assertTextSpans(refs, ['model']); assertTextSpans(refs, ['model']);
}); });
@ -1041,8 +1041,8 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toEqual(1); expect(refs.length).toEqual(3);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts', 'string-model.ts', 'other-dir.ts']);
assertTextSpans(refs, ['model']); assertTextSpans(refs, ['model']);
}); });
@ -1076,9 +1076,9 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toEqual(1); expect(refs.length).toEqual(2);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['string-model.ts', 'app.ts']);
assertTextSpans(refs, ['alias']); assertTextSpans(refs, ['aliasedModel', 'alias']);
}); });
it('should find rename locations', () => { it('should find rename locations', () => {
@ -1130,7 +1130,7 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toEqual(1); expect(refs.length).toEqual(2);
assertTextSpans(refs, ['modelChange']); assertTextSpans(refs, ['modelChange']);
}); });
@ -1154,8 +1154,8 @@ describe('find references and rename locations', () => {
it('should find references', () => { it('should find references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toEqual(1); expect(refs.length).toEqual(2);
assertTextSpans(refs, ['alias']); assertTextSpans(refs, ['aliasedModelChange', 'alias']);
}); });
it('should find rename locations', () => { it('should find rename locations', () => {
@ -1194,9 +1194,12 @@ describe('find references and rename locations', () => {
file.moveCursorToText('[(mod¦el)]'); file.moveCursorToText('[(mod¦el)]');
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toEqual(2); // Note that this includes the 'model` twice from the template. As with other potential
assertFileNames(refs, ['app.ts']); // duplicates (like if another plugin returns the same span), we expect the LS clients to filter
assertTextSpans(refs, ['model']); // these out themselves.
expect(refs.length).toEqual(4);
assertFileNames(refs, ['dir.ts', 'app.ts']);
assertTextSpans(refs, ['model', 'modelChange']);
}); });
describe('directives', () => { describe('directives', () => {
@ -1232,9 +1235,9 @@ describe('find references and rename locations', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
// 4 references are: class declaration, template usage, app import and use in declarations // 4 references are: class declaration, template usage, app import and use in declarations
// list. // list.
expect(refs.length).toBe(1); expect(refs.length).toBe(4);
assertTextSpans(refs, ['<div dir>']); assertTextSpans(refs, ['<div dir>', 'Dir']);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts', 'dir.ts']);
}); });
it('should find rename locations', () => { it('should find rename locations', () => {
@ -1281,9 +1284,9 @@ describe('find references and rename locations', () => {
it('gets references to all matching directives', () => { it('gets references to all matching directives', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(2); expect(refs.length).toBe(8);
assertTextSpans(refs, ['<div dir>']); assertTextSpans(refs, ['<div dir>', 'Dir', 'Dir2']);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts', 'dir.ts', 'dir2.ts']);
}); });
it('finds rename locations for all matching directives', () => { it('finds rename locations for all matching directives', () => {
@ -1318,9 +1321,9 @@ describe('find references and rename locations', () => {
it('should be able to request references', () => { it('should be able to request references', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(1); expect(refs.length).toBe(6);
assertTextSpans(refs, ['<div *ngFor="let item of items"></div>']); assertTextSpans(refs, ['<div *ngFor="let item of items"></div>', 'NgForOf']);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['index.d.ts', 'app.ts']);
}); });
it('should not support rename if directive is in a dts file', () => { it('should not support rename if directive is in a dts file', () => {
@ -1360,9 +1363,9 @@ describe('find references and rename locations', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
// 4 references are: class declaration, template usage, app import and use in declarations // 4 references are: class declaration, template usage, app import and use in declarations
// list. // list.
expect(refs.length).toBe(1); expect(refs.length).toBe(4);
assertTextSpans(refs, ['<my-comp>']); assertTextSpans(refs, ['<my-comp>', 'MyComp']);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts', 'comp.ts']);
}); });
it('gets rename locations', () => { it('gets rename locations', () => {
@ -1405,9 +1408,9 @@ describe('find references and rename locations', () => {
const refs = getReferencesAtPosition(file)!; const refs = getReferencesAtPosition(file)!;
// 4 references are: class declaration, template usage, app import and use in declarations // 4 references are: class declaration, template usage, app import and use in declarations
// list. // list.
expect(refs.length).toBe(1); expect(refs.length).toBe(4);
assertTextSpans(refs, ['<my-comp>']); assertTextSpans(refs, ['<my-comp>', 'MyComp']);
assertFileNames(refs, ['app.ts']); assertFileNames(refs, ['app.ts', 'comp.ts']);
}); });
it('finds rename locations', () => { it('finds rename locations', () => {