From 2b7553db6f59e64b73fc2e5601db278f815a67a2 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Fri, 21 Jan 2022 14:41:03 -0800 Subject: [PATCH] fix(compiler): compute correct offsets when interpolations have HTML entities (#44811) When parsing interpolations, the input string is _decoded_ from what was in the orginal template. This means that we cannot soley rely on the input string to compute source spans because it does not necessarily reflect the exact content of the original template. Specifically, when there is an HTML entity (i.e. ` `), this will show up in its decoded form when processing the interpolation (' '). We need to compute offsets using the original _encoded_ string. Note that this problem only surfaces in the splitting of interpolations. The spans to this point have already been tracked accurately. For example, given the template ` 
`, the source span for the `div` is already correctly determined to be 6. Only when we encounter interpolations with many parts do we run into situations where we need to compute new spans for the individual parts of the interpolation. PR Close #44811 --- .../src/ngtsc/indexer/test/template_spec.ts | 21 +++++++++ .../compiler/src/expression_parser/parser.ts | 46 ++++++++++++++++++- .../src/render3/r3_template_transform.ts | 14 ++++-- .../src/template_parser/binding_parser.ts | 13 ++++-- .../test/expression_parser/parser_spec.ts | 7 +-- .../test/render3/r3_ast_absolute_span_spec.ts | 39 ++++++++++++++++ .../compiler/test/render3/view/i18n_spec.ts | 10 ++-- 7 files changed, 131 insertions(+), 19 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts b/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts index d7accd9aa45..da38b965aaf 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts @@ -889,5 +889,26 @@ runInEachFileSystem(() => { } ])); }); + + it('should handle interpolations in attributes, preceded by HTML entity', () => { + const template = ``; + const refs = getTemplateIdentifiers(bind(template)); + + expect(Array.from(refs)).toEqual([ + { + kind: IdentifierKind.Element, + name: 'img', + span: new AbsoluteSourceSpan(1, 4), + usedDirectives: new Set(), + attributes: new Set(), + }, + { + kind: IdentifierKind.Property, + name: 'foo', + span: new AbsoluteSourceSpan(18, 21), + target: null, + } + ]); + }); }); }); diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 739b8cb2df3..3d24447079a 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -8,6 +8,7 @@ import * as chars from '../chars'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; +import {InterpolatedAttributeToken, InterpolatedTextToken, TokenType as MlParserTokenType} from '../ml_parser/tokens'; import {AbsoluteSourceSpan, AST, ASTWithSource, Binary, BindingPipe, Call, Chain, Conditional, EmptyExpr, ExpressionBinding, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, RecursiveAstVisitor, SafeCall, SafeKeyedRead, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, ThisReceiver, Unary, VariableBinding} from './ast'; import {EOF, Lexer, Token, TokenType} from './lexer'; @@ -147,9 +148,10 @@ export class Parser { parseInterpolation( input: string, location: string, absoluteOffset: number, + interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource|null { const {strings, expressions, offsets} = - this.splitInterpolation(input, location, interpolationConfig); + this.splitInterpolation(input, location, interpolatedTokens, interpolationConfig); if (expressions.length === 0) return null; const expressionNodes: AST[] = []; @@ -203,10 +205,13 @@ export class Parser { */ splitInterpolation( input: string, location: string, + interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): SplitInterpolation { const strings: InterpolationPiece[] = []; const expressions: InterpolationPiece[] = []; const offsets: number[] = []; + const inputToTemplateIndexMap = + interpolatedTokens ? getIndexMapForOriginalTemplate(interpolatedTokens) : null; let i = 0; let atInterpolation = false; let extendLastString = false; @@ -244,7 +249,9 @@ export class Parser { `at column ${i} in`, location); } expressions.push({text, start: fullStart, end: fullEnd}); - offsets.push(exprStart); + const startInOriginalTemplate = inputToTemplateIndexMap?.get(fullStart) ?? fullStart; + const offset = startInOriginalTemplate + interpStart.length; + offsets.push(offset); i = fullEnd; atInterpolation = false; @@ -1314,3 +1321,38 @@ class SimpleExpressionChecker extends RecursiveAstVisitor { this.errors.push('pipes'); } } +/** + * Computes the real offset in the original template for indexes in an interpolation. + * + * Because templates can have encoded HTML entities and the input passed to the parser at this stage + * of the compiler is the _decoded_ value, we need to compute the real offset using the original + * encoded values in the interpolated tokens. Note that this is only a special case handling for + * `MlParserTokenType.ENCODED_ENTITY` token types. All other interpolated tokens are expected to + * have parts which exactly match the input string for parsing the interpolation. + * + * @param interpolatedTokens The tokens for the interpolated value. + * + * @returns A map of index locations in the decoded template to indexes in the original template + */ +function getIndexMapForOriginalTemplate(interpolatedTokens: InterpolatedAttributeToken[]| + InterpolatedTextToken[]): Map { + let offsetMap = new Map(); + let consumedInOriginalTemplate = 0; + let consumedInInput = 0; + let tokenIndex = 0; + while (tokenIndex < interpolatedTokens.length) { + const currentToken = interpolatedTokens[tokenIndex]; + if (currentToken.type === MlParserTokenType.ENCODED_ENTITY) { + const [decoded, encoded] = currentToken.parts; + consumedInOriginalTemplate += encoded.length; + consumedInInput += decoded.length; + } else { + const lengthOfParts = currentToken.parts.reduce((sum, current) => sum + current.length, 0); + consumedInInput += lengthOfParts; + consumedInOriginalTemplate += lengthOfParts; + } + offsetMap.set(consumedInInput, consumedInOriginalTemplate); + tokenIndex++; + } + return offsetMap; +} diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts index 048757f68a4..ead3d64651d 100644 --- a/packages/compiler/src/render3/r3_template_transform.ts +++ b/packages/compiler/src/render3/r3_template_transform.ts @@ -11,6 +11,7 @@ import * as i18n from '../i18n/i18n_ast'; import * as html from '../ml_parser/ast'; import {replaceNgsp} from '../ml_parser/html_whitespaces'; import {isNgTemplate} from '../ml_parser/tags'; +import {InterpolatedAttributeToken, InterpolatedTextToken} from '../ml_parser/tokens'; import {ParseError, ParseErrorLevel, ParseSourceSpan} from '../parse_util'; import {isStyleUrlResolvable} from '../style_url_resolver'; import {BindingParser} from '../template_parser/binding_parser'; @@ -255,7 +256,7 @@ class HtmlAstToIvyAst implements html.Visitor { } visitText(text: html.Text): t.Node { - return this._visitTextWithInterpolation(text.value, text.sourceSpan, text.i18n); + return this._visitTextWithInterpolation(text.value, text.sourceSpan, text.tokens, text.i18n); } visitExpansion(expansion: html.Expansion): t.Icu|null { @@ -288,7 +289,7 @@ class HtmlAstToIvyAst implements html.Visitor { vars[formattedKey] = new t.BoundText(ast, value.sourceSpan); } else { - placeholders[key] = this._visitTextWithInterpolation(value.text, value.sourceSpan); + placeholders[key] = this._visitTextWithInterpolation(value.text, value.sourceSpan, null); } }); return new t.Icu(vars, placeholders, expansion.sourceSpan, message); @@ -443,14 +444,17 @@ class HtmlAstToIvyAst implements html.Visitor { // No explicit binding found. const keySpan = createKeySpan(srcSpan, '' /* prefix */, name); const hasBinding = this.bindingParser.parsePropertyInterpolation( - name, value, srcSpan, attribute.valueSpan, matchableAttributes, parsedProperties, keySpan); + name, value, srcSpan, attribute.valueSpan, matchableAttributes, parsedProperties, keySpan, + attribute.valueTokens ?? null); return hasBinding; } private _visitTextWithInterpolation( - value: string, sourceSpan: ParseSourceSpan, i18n?: i18n.I18nMeta): t.Text|t.BoundText { + value: string, sourceSpan: ParseSourceSpan, + interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null, + i18n?: i18n.I18nMeta): t.Text|t.BoundText { const valueNoNgsp = replaceNgsp(value); - const expr = this.bindingParser.parseInterpolation(valueNoNgsp, sourceSpan); + const expr = this.bindingParser.parseInterpolation(valueNoNgsp, sourceSpan, interpolatedTokens); return expr ? new t.BoundText(expr, sourceSpan, i18n) : new t.Text(valueNoNgsp, sourceSpan); } diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 0cc05ddae15..0bab38106d9 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -11,6 +11,7 @@ import {AbsoluteSourceSpan, ASTWithSource, BindingPipe, BindingType, BoundElemen import {Parser} from '../expression_parser/parser'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {mergeNsAndName} from '../ml_parser/tags'; +import {InterpolatedAttributeToken, InterpolatedTextToken} from '../ml_parser/tokens'; import {ParseError, ParseErrorLevel, ParseLocation, ParseSourceSpan} from '../parse_util'; import {ElementSchemaRegistry} from '../schema/element_schema_registry'; import {CssSelector} from '../selector'; @@ -95,13 +96,16 @@ export class BindingParser { return targetEvents; } - parseInterpolation(value: string, sourceSpan: ParseSourceSpan): ASTWithSource { + parseInterpolation( + value: string, sourceSpan: ParseSourceSpan, + interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]| + null): ASTWithSource { const sourceInfo = sourceSpan.start.toString(); const absoluteOffset = sourceSpan.fullStart.offset; try { const ast = this._exprParser.parseInterpolation( - value, sourceInfo, absoluteOffset, this._interpolationConfig)!; + value, sourceInfo, absoluteOffset, interpolatedTokens, this._interpolationConfig)!; if (ast) this._reportExpressionParserErrors(ast.errors, sourceSpan); return ast; } catch (e) { @@ -275,8 +279,9 @@ export class BindingParser { parsePropertyInterpolation( name: string, value: string, sourceSpan: ParseSourceSpan, valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][], - targetProps: ParsedProperty[], keySpan: ParseSourceSpan): boolean { - const expr = this.parseInterpolation(value, valueSpan || sourceSpan); + targetProps: ParsedProperty[], keySpan: ParseSourceSpan, + interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null): boolean { + const expr = this.parseInterpolation(value, valueSpan || sourceSpan, interpolatedTokens); if (expr) { this._parsePropertyAst( name, expr, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps); diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts index a2171dd0887..e7e6c5fe79f 100644 --- a/packages/compiler/test/expression_parser/parser_spec.ts +++ b/packages/compiler/test/expression_parser/parser_spec.ts @@ -995,7 +995,8 @@ describe('parser', () => { it('should support custom interpolation', () => { const parser = new Parser(new Lexer()); - const ast = parser.parseInterpolation('{% a %}', '', 0, {start: '{%', end: '%}'})!.ast as any; + const ast = + parser.parseInterpolation('{% a %}', '', 0, null, {start: '{%', end: '%}'})!.ast as any; expect(ast.strings).toEqual(['', '']); expect(ast.expressions.length).toEqual(1); expect(ast.expressions[0].name).toEqual('a'); @@ -1194,11 +1195,11 @@ function _parseTemplateBindings(attribute: string, templateUrl: string) { function parseInterpolation(text: string, location: any = null, offset: number = 0): ASTWithSource| null { - return createParser().parseInterpolation(text, location, offset); + return createParser().parseInterpolation(text, location, offset, null); } function splitInterpolation(text: string, location: any = null): SplitInterpolation|null { - return createParser().splitInterpolation(text, location); + return createParser().splitInterpolation(text, location, null); } function parseSimpleBinding(text: string, location: any = null, offset: number = 0): ASTWithSource { 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 83bd27d5d4e..c9a7dc0ee3d 100644 --- a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts +++ b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts @@ -157,6 +157,45 @@ describe('expression AST absolute source spans', () => { ['2', new AbsoluteSourceSpan(11, 12)], ])); }); + + it('should handle HTML entity before interpolation', () => { + expect(humanizeExpressionSource(parse(' {{abc}}').nodes)) + .toEqual(jasmine.arrayContaining([ + ['abc', new AbsoluteSourceSpan(8, 11)], + ])); + }); + + it('should handle many HTML entities and many interpolations', () => { + expect(humanizeExpressionSource(parse('"{{abc}}"{{def}} {{ghi}}').nodes)) + .toEqual(jasmine.arrayContaining([ + ['abc', new AbsoluteSourceSpan(8, 11)], + ['def', new AbsoluteSourceSpan(21, 24)], + ['ghi', new AbsoluteSourceSpan(34, 37)], + ])); + }); + + it('should handle interpolation in attribute', () => { + expect(humanizeExpressionSource(parse('
').nodes)) + .toEqual(jasmine.arrayContaining([ + ['abc', new AbsoluteSourceSpan(14, 17)], + ])); + }); + + it('should handle interpolation preceded by HTML entity in attribute', () => { + expect(humanizeExpressionSource(parse('
').nodes)) + .toEqual(jasmine.arrayContaining([ + ['abc', new AbsoluteSourceSpan(20, 23)], + ])); + }); + + it('should handle many interpolation with HTML entities in attribute', () => { + expect(humanizeExpressionSource( + parse('
').nodes)) + .toEqual(jasmine.arrayContaining([ + ['abc', new AbsoluteSourceSpan(20, 23)], + ['def', new AbsoluteSourceSpan(39, 42)], + ])); + }); }); describe('keyed read', () => { diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index ec664981f62..c9f2e4add64 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -52,8 +52,8 @@ describe('I18nContext', () => { // binding collection checks expect(ctx.bindings.size).toBe(0); - ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0) as AST); - ctx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0) as AST); + ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0, null) as AST); + ctx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0, null) as AST); expect(ctx.bindings.size).toBe(2); }); @@ -80,7 +80,7 @@ describe('I18nContext', () => { // set data for root ctx ctx.appendBoundText(i18nOf(boundTextA)); - ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0) as AST); + ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0, null) as AST); ctx.appendElement(i18nOf(elementA), 0); ctx.appendTemplate(i18nOf(templateA), 1); ctx.appendElement(i18nOf(elementA), 0, true); @@ -96,11 +96,11 @@ describe('I18nContext', () => { // set data for child context childCtx.appendElement(i18nOf(elementB), 0); childCtx.appendBoundText(i18nOf(boundTextB)); - childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0) as AST); + childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0, null) as AST); childCtx.appendElement(i18nOf(elementC), 1); childCtx.appendElement(i18nOf(elementC), 1, true); childCtx.appendBoundText(i18nOf(boundTextC)); - childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueC }}', '', 0) as AST); + childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueC }}', '', 0, null) as AST); childCtx.appendElement(i18nOf(elementB), 0, true); expect(childCtx.bindings.size).toBe(2);