From 8a1c98c5e8fe74c741bcfec2cd4b4d6fbf530d60 Mon Sep 17 00:00:00 2001
From: Andrew Scott <atscott@google.com>
Date: Mon, 9 Nov 2020 13:35:48 -0800
Subject: [PATCH] refactor(compiler-cli): add keySpan to reference nodes
 (#39616)

Similar to #39613, #39609, and #38898, we should store the `keySpan` for
Reference nodes so that we can accurately map from a template node to a
span in the original file. This is most notably an issue at the moment
for directive references `#ref="exportAs"`. The current behavior for the
language service when requesting information for the reference
is that it will return a text span that results in
highlighting the entire source when it should only highlight "ref" (test
added for this case as well).

PR Close #39616
---
 packages/compiler/src/render3/r3_ast.ts       |  2 +-
 .../src/render3/r3_template_transform.ts      |  7 +++---
 .../test/render3/r3_ast_spans_spec.ts         | 22 ++++++++++---------
 .../ivy/test/definitions_spec.ts              |  4 ++--
 .../ivy/test/quick_info_spec.ts               | 14 +++++++++---
 5 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts
index 68153667b12..dc121bbc054 100644
--- a/packages/compiler/src/render3/r3_ast.ts
+++ b/packages/compiler/src/render3/r3_ast.ts
@@ -139,7 +139,7 @@ export class Variable implements Node {
 export class Reference implements Node {
   constructor(
       public name: string, public value: string, public sourceSpan: ParseSourceSpan,
-      public valueSpan?: ParseSourceSpan) {}
+      readonly keySpan: ParseSourceSpan, public valueSpan?: ParseSourceSpan) {}
   visit<Result>(visitor: Visitor<Result>): Result {
     return visitor.visitReference(this);
   }
diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts
index 3019bfb3b3b..6a831f4a484 100644
--- a/packages/compiler/src/render3/r3_template_transform.ts
+++ b/packages/compiler/src/render3/r3_template_transform.ts
@@ -357,7 +357,8 @@ class HtmlAstToIvyAst implements html.Visitor {
 
       } else if (bindParts[KW_REF_IDX]) {
         const identifier = bindParts[IDENT_KW_IDX];
-        this.parseReference(identifier, value, srcSpan, attribute.valueSpan, references);
+        const keySpan = createKeySpan(srcSpan, bindParts[KW_REF_IDX], identifier);
+        this.parseReference(identifier, value, srcSpan, keySpan, attribute.valueSpan, references);
       } else if (bindParts[KW_ON_IDX]) {
         const events: ParsedEvent[] = [];
         const identifier = bindParts[IDENT_KW_IDX];
@@ -451,7 +452,7 @@ class HtmlAstToIvyAst implements html.Visitor {
   }
 
   private parseReference(
-      identifier: string, value: string, sourceSpan: ParseSourceSpan,
+      identifier: string, value: string, sourceSpan: ParseSourceSpan, keySpan: ParseSourceSpan,
       valueSpan: ParseSourceSpan|undefined, references: t.Reference[]) {
     if (identifier.indexOf('-') > -1) {
       this.reportError(`"-" is not allowed in reference names`, sourceSpan);
@@ -459,7 +460,7 @@ class HtmlAstToIvyAst implements html.Visitor {
       this.reportError(`Reference does not have a name`, sourceSpan);
     }
 
-    references.push(new t.Reference(identifier, value, sourceSpan, valueSpan));
+    references.push(new t.Reference(identifier, value, sourceSpan, keySpan, valueSpan));
   }
 
   private parseAssignmentEvent(
diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts
index 3b1ef79825c..f073904a0e4 100644
--- a/packages/compiler/test/render3/r3_ast_spans_spec.ts
+++ b/packages/compiler/test/render3/r3_ast_spans_spec.ts
@@ -59,8 +59,10 @@ class R3AstSourceSpans implements t.Visitor<void> {
   }
 
   visitReference(reference: t.Reference) {
-    this.result.push(
-        ['Reference', humanizeSpan(reference.sourceSpan), humanizeSpan(reference.valueSpan)]);
+    this.result.push([
+      'Reference', humanizeSpan(reference.sourceSpan), humanizeSpan(reference.keySpan),
+      humanizeSpan(reference.valueSpan)
+    ]);
   }
 
   visitTextAttribute(attribute: t.TextAttribute) {
@@ -211,7 +213,7 @@ describe('R3 AST source spans', () => {
     it('is correct for reference via #...', () => {
       expectFromHtml('<ng-template #a></ng-template>').toEqual([
         ['Template', '<ng-template #a></ng-template>', '<ng-template #a>', '</ng-template>'],
-        ['Reference', '#a', '<empty>'],
+        ['Reference', '#a', 'a', '<empty>'],
       ]);
     });
 
@@ -220,14 +222,14 @@ describe('R3 AST source spans', () => {
         [
           'Template', '<ng-template #a="b"></ng-template>', '<ng-template #a="b">', '</ng-template>'
         ],
-        ['Reference', '#a="b"', 'b'],
+        ['Reference', '#a="b"', 'a', 'b'],
       ]);
     });
 
     it('is correct for reference via ref-...', () => {
       expectFromHtml('<ng-template ref-a></ng-template>').toEqual([
         ['Template', '<ng-template ref-a></ng-template>', '<ng-template ref-a>', '</ng-template>'],
-        ['Reference', 'ref-a', '<empty>'],
+        ['Reference', 'ref-a', 'a', '<empty>'],
       ]);
     });
 
@@ -237,7 +239,7 @@ describe('R3 AST source spans', () => {
           'Template', '<ng-template data-ref-a></ng-template>', '<ng-template data-ref-a>',
           '</ng-template>'
         ],
-        ['Reference', 'data-ref-a', '<empty>'],
+        ['Reference', 'data-ref-a', 'a', '<empty>'],
       ]);
     });
 
@@ -405,28 +407,28 @@ describe('R3 AST source spans', () => {
     it('is correct for references via #...', () => {
       expectFromHtml('<div #a></div>').toEqual([
         ['Element', '<div #a></div>', '<div #a>', '</div>'],
-        ['Reference', '#a', '<empty>'],
+        ['Reference', '#a', 'a', '<empty>'],
       ]);
     });
 
     it('is correct for references with name', () => {
       expectFromHtml('<div #a="b"></div>').toEqual([
         ['Element', '<div #a="b"></div>', '<div #a="b">', '</div>'],
-        ['Reference', '#a="b"', 'b'],
+        ['Reference', '#a="b"', 'a', 'b'],
       ]);
     });
 
     it('is correct for references via ref-', () => {
       expectFromHtml('<div ref-a></div>').toEqual([
         ['Element', '<div ref-a></div>', '<div ref-a>', '</div>'],
-        ['Reference', 'ref-a', '<empty>'],
+        ['Reference', 'ref-a', 'a', '<empty>'],
       ]);
     });
 
     it('is correct for references via data-ref-', () => {
       expectFromHtml('<div ref-a></div>').toEqual([
         ['Element', '<div ref-a></div>', '<div ref-a>', '</div>'],
-        ['Reference', 'ref-a', '<empty>'],
+        ['Reference', 'ref-a', 'a', '<empty>'],
       ]);
     });
   });
diff --git a/packages/language-service/ivy/test/definitions_spec.ts b/packages/language-service/ivy/test/definitions_spec.ts
index 117f5725938..b9d0e0c88bc 100644
--- a/packages/language-service/ivy/test/definitions_spec.ts
+++ b/packages/language-service/ivy/test/definitions_spec.ts
@@ -235,7 +235,7 @@ describe('definitions', () => {
     it('should work for element reference declarations', () => {
       const definitions = getDefinitionsAndAssertBoundSpan({
         templateOverride: `<div #cha¦rt></div>{{chart}}`,
-        expectedSpanText: '#chart',
+        expectedSpanText: 'chart',
       });
       // We're already at the definition, so nothing is returned
       expect(definitions).toEqual([]);
@@ -249,7 +249,7 @@ describe('definitions', () => {
       expect(definitions!.length).toEqual(1);
 
       const [varDef] = definitions;
-      expect(varDef.textSpan).toEqual('#chart');
+      expect(varDef.textSpan).toEqual('chart');
     });
   });
 
diff --git a/packages/language-service/ivy/test/quick_info_spec.ts b/packages/language-service/ivy/test/quick_info_spec.ts
index b8a6b5b6d61..8d41c1463a5 100644
--- a/packages/language-service/ivy/test/quick_info_spec.ts
+++ b/packages/language-service/ivy/test/quick_info_spec.ts
@@ -196,7 +196,7 @@ describe('quick info', () => {
     it('should work for element reference declarations', () => {
       const {documentation} = expectQuickInfo({
         templateOverride: `<div #¦chart></div>`,
-        expectedSpanText: '#chart',
+        expectedSpanText: 'chart',
         expectedDisplayString: '(reference) chart: HTMLDivElement'
       });
       expect(toText(documentation))
@@ -205,15 +205,23 @@ describe('quick info', () => {
               'interface it also has available to it by inheritance) for manipulating <div> elements.');
     });
 
+    it('should work for directive references', () => {
+      expectQuickInfo({
+        templateOverride: `<div string-model #dir¦Ref="stringModel"></div>`,
+        expectedSpanText: 'dirRef',
+        expectedDisplayString: '(reference) dirRef: StringModel'
+      });
+    });
+
     it('should work for ref- syntax', () => {
       expectQuickInfo({
         templateOverride: `<div ref-ch¦art></div>`,
-        expectedSpanText: 'ref-chart',
+        expectedSpanText: 'chart',
         expectedDisplayString: '(reference) chart: HTMLDivElement'
       });
       expectQuickInfo({
         templateOverride: `<div data-ref-ch¦art></div>`,
-        expectedSpanText: 'data-ref-chart',
+        expectedSpanText: 'chart',
         expectedDisplayString: '(reference) chart: HTMLDivElement'
       });
     });