From 8f349b23751aefbb96d6174f9089795c5d725d78 Mon Sep 17 00:00:00 2001
From: Keen Yee Liau <kyliau@google.com>
Date: Wed, 9 Sep 2020 11:22:50 -0700
Subject: [PATCH] fix(compiler): source span for microsyntax text att should be
 key span (#38766)

In a microsyntax expressions, some attributes are not bound after
desugaring. For example,
```html
<div *ngFor="let item of items">
</div>
```
gets desugared to
```html
<ng-template ngFor let-items [ngForOf]="items">
</ngtemplate>
```
In this case, `ngFor` should be a literal attribute with no RHS value.
Therefore, its source span should be just the `keySpan` and not the
source span of the original template node.
This allows language service to precisely pinpoint different spans in a
microsyntax to provide accurate information.

PR Close #38766
---
 .../compiler/src/template_parser/binding_parser.ts   |  8 +++++---
 packages/compiler/test/render3/r3_ast_spans_spec.ts  | 12 ++++++------
 .../language-service/ivy/test/hybrid_visitor_spec.ts |  5 +++--
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts
index 1c19d69c9c..ca0001f44e 100644
--- a/packages/compiler/src/template_parser/binding_parser.ts
+++ b/packages/compiler/src/template_parser/binding_parser.ts
@@ -157,10 +157,12 @@ export class BindingParser {
         this._parsePropertyAst(
             key, binding.value, sourceSpan, valueSpan, targetMatchableAttrs, targetProps);
       } else {
-        targetMatchableAttrs.push([key, '']);
+        targetMatchableAttrs.push([key, '' /* value */]);
+        // Since this is a literal attribute with no RHS, source span should be
+        // just the key span.
         this.parseLiteralAttr(
-            key, null, sourceSpan, absoluteValueOffset, undefined, targetMatchableAttrs,
-            targetProps);
+            key, null /* value */, keySpan, absoluteValueOffset, undefined /* valueSpan */,
+            targetMatchableAttrs, targetProps);
       }
     }
   }
diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts
index 6fc6b3003b..791b36021f 100644
--- a/packages/compiler/test/render3/r3_ast_spans_spec.ts
+++ b/packages/compiler/test/render3/r3_ast_spans_spec.ts
@@ -174,7 +174,7 @@ describe('R3 AST source spans', () => {
     it('is correct for * directives', () => {
       expectFromHtml('<div *ngIf></div>').toEqual([
         ['Template', '0:17', '0:11', '11:17'],
-        ['TextAttribute', '5:10', '<empty>'],
+        ['TextAttribute', '6:10', '<empty>'],  // ngIf
         ['Element', '0:17', '0:11', '11:17'],
       ]);
     });
@@ -237,9 +237,9 @@ describe('R3 AST source spans', () => {
       // </ng-template>
       expectFromHtml('<div *ngFor="let item of items"></div>').toEqual([
         ['Template', '0:38', '0:32', '32:38'],
-        ['TextAttribute', '5:31', '<empty>'],
-        ['BoundAttribute', '5:31', '25:30'],  // *ngFor="let item of items" -> items
-        ['Variable', '13:22', '<empty>'],     // let item
+        ['TextAttribute', '6:11', '<empty>'],  // ngFor
+        ['BoundAttribute', '5:31', '25:30'],   // *ngFor="let item of items" -> items
+        ['Variable', '13:22', '<empty>'],      // let item
         ['Element', '0:38', '0:32', '32:38'],
       ]);
 
@@ -260,8 +260,8 @@ describe('R3 AST source spans', () => {
     it('is correct for variables via let ...', () => {
       expectFromHtml('<div *ngIf="let a=b"></div>').toEqual([
         ['Template', '0:27', '0:21', '21:27'],
-        ['TextAttribute', '5:20', '<empty>'],
-        ['Variable', '12:19', '18:19'],  // let a=b -> b
+        ['TextAttribute', '6:10', '<empty>'],  // ngIf
+        ['Variable', '12:19', '18:19'],        // let a=b -> b
         ['Element', '0:27', '0:21', '21:27'],
       ]);
     });
diff --git a/packages/language-service/ivy/test/hybrid_visitor_spec.ts b/packages/language-service/ivy/test/hybrid_visitor_spec.ts
index 43820c7e45..8a9d06fc6e 100644
--- a/packages/language-service/ivy/test/hybrid_visitor_spec.ts
+++ b/packages/language-service/ivy/test/hybrid_visitor_spec.ts
@@ -487,8 +487,9 @@ describe('findNodeAtPosition for microsyntax expression', () => {
     // <ng-template ngFor let-item [ngForOf]="items">
     expect(errors).toBe(null);
     const node = findNodeAtPosition(nodes, position);
-    // TODO: this is currently wrong because it should point to ngFor text
-    // attribute instead of ngForOf bound attribute
+    expect(isTemplateNode(node!)).toBeTrue();
+    expect(node).toBeInstanceOf(t.TextAttribute);
+    expect((node as t.TextAttribute).name).toBe('ngFor');
   });
 
   it('should locate not let keyword', () => {