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

VSCode only de-duplicates references results for "go to references" requests
but does not de-duplicate them for "find all references" requests. The
result is that users see duplicate references for results in TypeScript
files - one from the built-in TS extension and one from us.
While this is an issue in VSCode (see https://github.com/microsoft/vscode/issues/117095)
this commit provides a quick workaround on our end until it can be addressed there.

This commit should be reverted when microsoft/vscode/issues/117095 is resolved.

fixes https://github.com/angular/vscode-ng-language-service/issues/1124

PR Close #41041

packages/language-service/ivy/references.ts

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

packages/language-service/ivy/test/references_spec.ts

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