refactor(compiler): Binding parser sets binding span as source span in Ivy (#39036)

Currently it is impossible to determine the source of a binding that
generates `BoundAttribute` because all bound attributes generated from a
microsyntax expression share the same source span.

For example, in
```html
<div *ngFor="let item of items; trackBy: trackByFn"></div>
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     source span for all `BoundAttribute`s generated from microsyntax
```
the `BoundAttribute` for both `ngForOf` and `ngForTrackBy`
share the same source span.

A lot of hacks were necessary in View Engine language service to work
around this limitation. It was done by inspecting the whole source span
then figuring out the relative position of the cursor.

With this change, we introduce a flag to set the binding span as the
source span of the `ParsedProperty` in Ivy AST.
This flag is needed so that we don't have to change VE ASTs.

Note that in the binding parser, we already set `bindingSpan` as the
source span for a `ParsedVariable`, and `keySpan` as the source span for
a literal attribute. This change makes the Ivy AST more consistent by
propagating the binding span to `ParsedProperty` as well.

PR Close #39036
This commit is contained in:
Keen Yee Liau 2020-09-28 21:23:34 -07:00 committed by Alex Rickabaugh
parent 393ce5574b
commit 8b7acc4f8f
4 changed files with 12 additions and 13 deletions

View File

@ -157,7 +157,7 @@ class HtmlAstToIvyAst implements html.Visitor {
this.bindingParser.parseInlineTemplateBinding( this.bindingParser.parseInlineTemplateBinding(
templateKey, templateValue, attribute.sourceSpan, absoluteValueOffset, [], templateKey, templateValue, attribute.sourceSpan, absoluteValueOffset, [],
templateParsedProperties, parsedVariables); templateParsedProperties, parsedVariables, true /* isIvyAst */);
templateVariables.push(...parsedVariables.map( templateVariables.push(...parsedVariables.map(
v => new t.Variable(v.name, v.value, v.sourceSpan, v.keySpan, v.valueSpan))); v => new t.Variable(v.name, v.value, v.sourceSpan, v.keySpan, v.valueSpan)));
} else { } else {

View File

@ -141,8 +141,8 @@ export class BindingParser {
*/ */
parseInlineTemplateBinding( parseInlineTemplateBinding(
tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteValueOffset: number, tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteValueOffset: number,
targetMatchableAttrs: string[][], targetProps: ParsedProperty[], targetMatchableAttrs: string[][], targetProps: ParsedProperty[], targetVars: ParsedVariable[],
targetVars: ParsedVariable[]) { isIvyAst: boolean) {
const absoluteKeyOffset = sourceSpan.start.offset + TEMPLATE_ATTR_PREFIX.length; const absoluteKeyOffset = sourceSpan.start.offset + TEMPLATE_ATTR_PREFIX.length;
const bindings = this._parseTemplateBindings( const bindings = this._parseTemplateBindings(
tplKey, tplValue, sourceSpan, absoluteKeyOffset, absoluteValueOffset); tplKey, tplValue, sourceSpan, absoluteKeyOffset, absoluteValueOffset);
@ -159,9 +159,10 @@ export class BindingParser {
binding.value ? moveParseSourceSpan(sourceSpan, binding.value.span) : undefined; binding.value ? moveParseSourceSpan(sourceSpan, binding.value.span) : undefined;
targetVars.push(new ParsedVariable(key, value, bindingSpan, keySpan, valueSpan)); targetVars.push(new ParsedVariable(key, value, bindingSpan, keySpan, valueSpan));
} else if (binding.value) { } else if (binding.value) {
const srcSpan = isIvyAst ? bindingSpan : sourceSpan;
const valueSpan = moveParseSourceSpan(sourceSpan, binding.value.ast.sourceSpan); const valueSpan = moveParseSourceSpan(sourceSpan, binding.value.ast.sourceSpan);
this._parsePropertyAst( this._parsePropertyAst(
key, binding.value, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps); key, binding.value, srcSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
} else { } else {
targetMatchableAttrs.push([key, '' /* value */]); targetMatchableAttrs.push([key, '' /* value */]);
// Since this is a literal attribute with no RHS, source span should be // Since this is a literal attribute with no RHS, source span should be

View File

@ -317,7 +317,7 @@ class TemplateParseVisitor implements html.Visitor {
const absoluteOffset = (attr.valueSpan || attr.sourceSpan).start.offset; const absoluteOffset = (attr.valueSpan || attr.sourceSpan).start.offset;
this._bindingParser.parseInlineTemplateBinding( this._bindingParser.parseInlineTemplateBinding(
templateKey!, templateValue!, attr.sourceSpan, absoluteOffset, templateMatchableAttrs, templateKey!, templateValue!, attr.sourceSpan, absoluteOffset, templateMatchableAttrs,
templateElementOrDirectiveProps, parsedVariables); templateElementOrDirectiveProps, parsedVariables, false /* isIvyAst */);
templateElementVars.push(...parsedVariables.map(v => t.VariableAst.fromParsedVariable(v))); templateElementVars.push(...parsedVariables.map(v => t.VariableAst.fromParsedVariable(v)));
} }

View File

@ -285,7 +285,7 @@ describe('R3 AST source spans', () => {
'</div>' '</div>'
], ],
['TextAttribute', 'ngFor', '<empty>'], ['TextAttribute', 'ngFor', '<empty>'],
['BoundAttribute', '*ngFor="let item of items"', 'of', 'items'], ['BoundAttribute', 'of items', 'of', 'items'],
['Variable', 'let item ', 'item', '<empty>'], ['Variable', 'let item ', 'item', '<empty>'],
[ [
'Element', '<div *ngFor="let item of items"></div>', '<div *ngFor="let item of items">', 'Element', '<div *ngFor="let item of items"></div>', '<div *ngFor="let item of items">',
@ -303,8 +303,8 @@ describe('R3 AST source spans', () => {
[ [
'Template', '<div *ngFor="item of items"></div>', '<div *ngFor="item of items">', '</div>' 'Template', '<div *ngFor="item of items"></div>', '<div *ngFor="item of items">', '</div>'
], ],
['BoundAttribute', '*ngFor="item of items"', 'ngFor', 'item'], ['BoundAttribute', 'ngFor="item ', 'ngFor', 'item'],
['BoundAttribute', '*ngFor="item of items"', 'of', 'items'], ['BoundAttribute', 'of items', 'of', 'items'],
['Element', '<div *ngFor="item of items"></div>', '<div *ngFor="item of items">', '</div>'], ['Element', '<div *ngFor="item of items"></div>', '<div *ngFor="item of items">', '</div>'],
]); ]);
@ -314,10 +314,8 @@ describe('R3 AST source spans', () => {
'<div *ngFor="let item of items; trackBy: trackByFn">', '</div>' '<div *ngFor="let item of items; trackBy: trackByFn">', '</div>'
], ],
['TextAttribute', 'ngFor', '<empty>'], ['TextAttribute', 'ngFor', '<empty>'],
['BoundAttribute', '*ngFor="let item of items; trackBy: trackByFn"', 'of', 'items'], ['BoundAttribute', 'of items; ', 'of', 'items'],
[ ['BoundAttribute', 'trackBy: trackByFn', 'trackBy', 'trackByFn'],
'BoundAttribute', '*ngFor="let item of items; trackBy: trackByFn"', 'trackBy', 'trackByFn'
],
['Variable', 'let item ', 'item', '<empty>'], ['Variable', 'let item ', 'item', '<empty>'],
[ [
'Element', '<div *ngFor="let item of items; trackBy: trackByFn"></div>', 'Element', '<div *ngFor="let item of items; trackBy: trackByFn"></div>',
@ -339,7 +337,7 @@ describe('R3 AST source spans', () => {
it('is correct for variables via as ...', () => { it('is correct for variables via as ...', () => {
expectFromHtml('<div *ngIf="expr as local"></div>').toEqual([ expectFromHtml('<div *ngIf="expr as local"></div>').toEqual([
['Template', '<div *ngIf="expr as local"></div>', '<div *ngIf="expr as local">', '</div>'], ['Template', '<div *ngIf="expr as local"></div>', '<div *ngIf="expr as local">', '</div>'],
['BoundAttribute', '*ngIf="expr as local"', 'ngIf', 'expr'], ['BoundAttribute', 'ngIf="expr ', 'ngIf', 'expr'],
['Variable', 'ngIf="expr as local', 'local', 'ngIf'], ['Variable', 'ngIf="expr as local', 'local', 'ngIf'],
['Element', '<div *ngIf="expr as local"></div>', '<div *ngIf="expr as local">', '</div>'], ['Element', '<div *ngIf="expr as local"></div>', '<div *ngIf="expr as local">', '</div>'],
]); ]);