From 8b7acc4f8f4f1f91f9dd94a2653644209da6f4cc Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Mon, 28 Sep 2020 21:23:34 -0700 Subject: [PATCH] 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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 --- .../compiler/src/render3/r3_template_transform.ts | 2 +- .../compiler/src/template_parser/binding_parser.ts | 7 ++++--- .../src/template_parser/template_parser.ts | 2 +- .../compiler/test/render3/r3_ast_spans_spec.ts | 14 ++++++-------- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts index 0018a68481..92d652d16d 100644 --- a/packages/compiler/src/render3/r3_template_transform.ts +++ b/packages/compiler/src/render3/r3_template_transform.ts @@ -157,7 +157,7 @@ class HtmlAstToIvyAst implements html.Visitor { this.bindingParser.parseInlineTemplateBinding( templateKey, templateValue, attribute.sourceSpan, absoluteValueOffset, [], - templateParsedProperties, parsedVariables); + templateParsedProperties, parsedVariables, true /* isIvyAst */); templateVariables.push(...parsedVariables.map( v => new t.Variable(v.name, v.value, v.sourceSpan, v.keySpan, v.valueSpan))); } else { diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 1cbe4530ab..bc4ccf9cd1 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -141,8 +141,8 @@ export class BindingParser { */ parseInlineTemplateBinding( tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteValueOffset: number, - targetMatchableAttrs: string[][], targetProps: ParsedProperty[], - targetVars: ParsedVariable[]) { + targetMatchableAttrs: string[][], targetProps: ParsedProperty[], targetVars: ParsedVariable[], + isIvyAst: boolean) { const absoluteKeyOffset = sourceSpan.start.offset + TEMPLATE_ATTR_PREFIX.length; const bindings = this._parseTemplateBindings( tplKey, tplValue, sourceSpan, absoluteKeyOffset, absoluteValueOffset); @@ -159,9 +159,10 @@ export class BindingParser { binding.value ? moveParseSourceSpan(sourceSpan, binding.value.span) : undefined; targetVars.push(new ParsedVariable(key, value, bindingSpan, keySpan, valueSpan)); } else if (binding.value) { + const srcSpan = isIvyAst ? bindingSpan : sourceSpan; const valueSpan = moveParseSourceSpan(sourceSpan, binding.value.ast.sourceSpan); this._parsePropertyAst( - key, binding.value, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps); + key, binding.value, srcSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps); } else { targetMatchableAttrs.push([key, '' /* value */]); // Since this is a literal attribute with no RHS, source span should be diff --git a/packages/compiler/src/template_parser/template_parser.ts b/packages/compiler/src/template_parser/template_parser.ts index 5c6bd78918..19dfe8f384 100644 --- a/packages/compiler/src/template_parser/template_parser.ts +++ b/packages/compiler/src/template_parser/template_parser.ts @@ -317,7 +317,7 @@ class TemplateParseVisitor implements html.Visitor { const absoluteOffset = (attr.valueSpan || attr.sourceSpan).start.offset; this._bindingParser.parseInlineTemplateBinding( templateKey!, templateValue!, attr.sourceSpan, absoluteOffset, templateMatchableAttrs, - templateElementOrDirectiveProps, parsedVariables); + templateElementOrDirectiveProps, parsedVariables, false /* isIvyAst */); templateElementVars.push(...parsedVariables.map(v => t.VariableAst.fromParsedVariable(v))); } diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts index e74722c109..1f82c8a78b 100644 --- a/packages/compiler/test/render3/r3_ast_spans_spec.ts +++ b/packages/compiler/test/render3/r3_ast_spans_spec.ts @@ -285,7 +285,7 @@ describe('R3 AST source spans', () => { '' ], ['TextAttribute', 'ngFor', ''], - ['BoundAttribute', '*ngFor="let item of items"', 'of', 'items'], + ['BoundAttribute', 'of items', 'of', 'items'], ['Variable', 'let item ', 'item', ''], [ 'Element', '
', '
', @@ -303,8 +303,8 @@ describe('R3 AST source spans', () => { [ 'Template', '
', '
', '
' ], - ['BoundAttribute', '*ngFor="item of items"', 'ngFor', 'item'], - ['BoundAttribute', '*ngFor="item of items"', 'of', 'items'], + ['BoundAttribute', 'ngFor="item ', 'ngFor', 'item'], + ['BoundAttribute', 'of items', 'of', 'items'], ['Element', '
', '
', '
'], ]); @@ -314,10 +314,8 @@ describe('R3 AST source spans', () => { '
', '
' ], ['TextAttribute', 'ngFor', ''], - ['BoundAttribute', '*ngFor="let item of items; trackBy: trackByFn"', 'of', 'items'], - [ - 'BoundAttribute', '*ngFor="let item of items; trackBy: trackByFn"', 'trackBy', 'trackByFn' - ], + ['BoundAttribute', 'of items; ', 'of', 'items'], + ['BoundAttribute', 'trackBy: trackByFn', 'trackBy', 'trackByFn'], ['Variable', 'let item ', 'item', ''], [ 'Element', '
', @@ -339,7 +337,7 @@ describe('R3 AST source spans', () => { it('is correct for variables via as ...', () => { expectFromHtml('
').toEqual([ ['Template', '
', '
', '
'], - ['BoundAttribute', '*ngIf="expr as local"', 'ngIf', 'expr'], + ['BoundAttribute', 'ngIf="expr ', 'ngIf', 'expr'], ['Variable', 'ngIf="expr as local', 'local', 'ngIf'], ['Element', '
', '
', '
'], ]);