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
This commit is contained in:
Keen Yee Liau 2020-09-09 11:22:50 -07:00 committed by Andrew Kushnir
parent 19598b47ca
commit 8f349b2375
3 changed files with 14 additions and 11 deletions

View File

@ -157,10 +157,12 @@ export class BindingParser {
this._parsePropertyAst( this._parsePropertyAst(
key, binding.value, sourceSpan, valueSpan, targetMatchableAttrs, targetProps); key, binding.value, sourceSpan, valueSpan, targetMatchableAttrs, targetProps);
} else { } 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( this.parseLiteralAttr(
key, null, sourceSpan, absoluteValueOffset, undefined, targetMatchableAttrs, key, null /* value */, keySpan, absoluteValueOffset, undefined /* valueSpan */,
targetProps); targetMatchableAttrs, targetProps);
} }
} }
} }

View File

@ -174,7 +174,7 @@ describe('R3 AST source spans', () => {
it('is correct for * directives', () => { it('is correct for * directives', () => {
expectFromHtml('<div *ngIf></div>').toEqual([ expectFromHtml('<div *ngIf></div>').toEqual([
['Template', '0:17', '0:11', '11:17'], ['Template', '0:17', '0:11', '11:17'],
['TextAttribute', '5:10', '<empty>'], ['TextAttribute', '6:10', '<empty>'], // ngIf
['Element', '0:17', '0:11', '11:17'], ['Element', '0:17', '0:11', '11:17'],
]); ]);
}); });
@ -237,9 +237,9 @@ describe('R3 AST source spans', () => {
// </ng-template> // </ng-template>
expectFromHtml('<div *ngFor="let item of items"></div>').toEqual([ expectFromHtml('<div *ngFor="let item of items"></div>').toEqual([
['Template', '0:38', '0:32', '32:38'], ['Template', '0:38', '0:32', '32:38'],
['TextAttribute', '5:31', '<empty>'], ['TextAttribute', '6:11', '<empty>'], // ngFor
['BoundAttribute', '5:31', '25:30'], // *ngFor="let item of items" -> items ['BoundAttribute', '5:31', '25:30'], // *ngFor="let item of items" -> items
['Variable', '13:22', '<empty>'], // let item ['Variable', '13:22', '<empty>'], // let item
['Element', '0:38', '0:32', '32:38'], ['Element', '0:38', '0:32', '32:38'],
]); ]);
@ -260,8 +260,8 @@ describe('R3 AST source spans', () => {
it('is correct for variables via let ...', () => { it('is correct for variables via let ...', () => {
expectFromHtml('<div *ngIf="let a=b"></div>').toEqual([ expectFromHtml('<div *ngIf="let a=b"></div>').toEqual([
['Template', '0:27', '0:21', '21:27'], ['Template', '0:27', '0:21', '21:27'],
['TextAttribute', '5:20', '<empty>'], ['TextAttribute', '6:10', '<empty>'], // ngIf
['Variable', '12:19', '18:19'], // let a=b -> b ['Variable', '12:19', '18:19'], // let a=b -> b
['Element', '0:27', '0:21', '21:27'], ['Element', '0:27', '0:21', '21:27'],
]); ]);
}); });

View File

@ -487,8 +487,9 @@ describe('findNodeAtPosition for microsyntax expression', () => {
// <ng-template ngFor let-item [ngForOf]="items"> // <ng-template ngFor let-item [ngForOf]="items">
expect(errors).toBe(null); expect(errors).toBe(null);
const node = findNodeAtPosition(nodes, position); const node = findNodeAtPosition(nodes, position);
// TODO: this is currently wrong because it should point to ngFor text expect(isTemplateNode(node!)).toBeTrue();
// attribute instead of ngForOf bound attribute expect(node).toBeInstanceOf(t.TextAttribute);
expect((node as t.TextAttribute).name).toBe('ngFor');
}); });
it('should locate not let keyword', () => { it('should locate not let keyword', () => {