From ddc229b69bbf26c7f0746aba270f200d451e077c Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 23 Jul 2019 15:51:42 -0700 Subject: [PATCH] fix(ivy): record correct absolute source span for ngForOf expressions (#31813) Expressions in an inline template binding are improperly recorded as spaning an offset calculated from the start of the template binding attribute key, whereas they should be calculated from the start of the attribute value, which contains the actual binding AST. PR Close #31813 --- .../compiler/src/expression_parser/parser.ts | 2 +- .../src/render3/r3_template_transform.ts | 11 +++++-- .../src/template_parser/binding_parser.ts | 30 +++++++++++++++---- .../test/render3/r3_ast_absolute_span_spec.ts | 14 +++++++++ 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 9e7977a80a..9e0205bd9e 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -764,7 +764,7 @@ export class _ParseAST { const ast = this.parsePipe(); const source = this.input.substring(start - this.offset, this.inputIndex - this.offset); expression = - new ASTWithSource(ast, source, this.location, this.absoluteOffset, this.errors); + new ASTWithSource(ast, source, this.location, this.absoluteOffset + start, this.errors); } bindings.push(new TemplateBinding( diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts index 4df6909321..8739b66fbf 100644 --- a/packages/compiler/src/render3/r3_template_transform.ts +++ b/packages/compiler/src/render3/r3_template_transform.ts @@ -151,10 +151,15 @@ class HtmlAstToIvyAst implements html.Visitor { const templateKey = normalizedName.substring(TEMPLATE_ATTR_PREFIX.length); const parsedVariables: ParsedVariable[] = []; - const absoluteOffset = attribute.valueSpan ? attribute.valueSpan.start.offset : - attribute.sourceSpan.start.offset; + const absoluteValueOffset = attribute.valueSpan ? + attribute.valueSpan.start.offset : + // If there is no value span the attribute does not have a value, like `attr` in + //`
`. In this case, point to one character beyond the last character of + // the attribute name. + attribute.sourceSpan.start.offset + attribute.name.length; + this.bindingParser.parseInlineTemplateBinding( - templateKey, templateValue, attribute.sourceSpan, absoluteOffset, [], + templateKey, templateValue, attribute.sourceSpan, absoluteValueOffset, [], templateParsedProperties, parsedVariables); templateVariables.push( ...parsedVariables.map(v => new t.Variable(v.name, v.value, v.sourceSpan))); diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 6da0d1495f..f6258db5e3 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -113,12 +113,22 @@ export class BindingParser { } } - // Parse an inline template binding. ie `` + /** + * Parses an inline template binding, e.g. + * + * @param tplKey template binding name + * @param tplValue template binding value + * @param sourceSpan span of template binding relative to entire the template + * @param absoluteValueOffset start of the tplValue relative to the entire template + * @param targetMatchableAttrs potential attributes to match in the template + * @param targetProps target property bindings in the template + * @param targetVars target variables in the template + */ parseInlineTemplateBinding( - tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteOffset: number, + tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteValueOffset: number, targetMatchableAttrs: string[][], targetProps: ParsedProperty[], targetVars: ParsedVariable[]) { - const bindings = this._parseTemplateBindings(tplKey, tplValue, sourceSpan, absoluteOffset); + const bindings = this._parseTemplateBindings(tplKey, tplValue, sourceSpan, absoluteValueOffset); for (let i = 0; i < bindings.length; i++) { const binding = bindings[i]; @@ -131,20 +141,28 @@ export class BindingParser { } else { targetMatchableAttrs.push([binding.key, '']); this.parseLiteralAttr( - binding.key, null, sourceSpan, absoluteOffset, undefined, targetMatchableAttrs, + binding.key, null, sourceSpan, absoluteValueOffset, undefined, targetMatchableAttrs, targetProps); } } } + /** + * Parses the bindings in an inline template binding, e.g. + * + * @param tplKey template binding name + * @param tplValue template binding value + * @param sourceSpan span of template binding relative to entire the template + * @param absoluteValueOffset start of the tplValue relative to the entire template + */ private _parseTemplateBindings( tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, - absoluteOffset: number): TemplateBinding[] { + absoluteValueOffset: number): TemplateBinding[] { const sourceInfo = sourceSpan.start.toString(); try { const bindingsResult = - this._exprParser.parseTemplateBindings(tplKey, tplValue, sourceInfo, absoluteOffset); + this._exprParser.parseTemplateBindings(tplKey, tplValue, sourceInfo, absoluteValueOffset); this._reportExpressionParserErrors(bindingsResult.errors, sourceSpan); bindingsResult.templateBindings.forEach((binding) => { if (binding.expression) { diff --git a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts index cc38497f26..6641e16108 100644 --- a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts +++ b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts @@ -322,4 +322,18 @@ describe('expression AST absolute source spans', () => { 'a:b', new AbsoluteSourceSpan(13, 16) ]); }); + + describe('absolute offsets for template expressions', () => { + it('should work for simple cases', () => { + expect( + humanizeExpressionSource(parse('
{{item}}
').nodes)) + .toContain(['items', new AbsoluteSourceSpan(25, 30)]); + }); + + it('should work with multiple bindings', () => { + expect(humanizeExpressionSource(parse('
').nodes)) + .toEqual(jasmine.arrayContaining( + [['As', new AbsoluteSourceSpan(22, 24)], ['Bs', new AbsoluteSourceSpan(35, 37)]])); + }); + }); });