Skip to content

Commit

Permalink
fix(compiler): compute correct offsets when interpolations have HTML …
Browse files Browse the repository at this point in the history
…entities

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 `&nbsp;<div></div>`, 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.
  • Loading branch information
atscott committed Jan 25, 2022
1 parent 197f3f4 commit eae58a9
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 19 deletions.
21 changes: 21 additions & 0 deletions packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -841,5 +841,26 @@ runInEachFileSystem(() => {
}
]));
});

it('should handle interpolations in attributes, preceded by HTML entity', () => {
const template = `<img src="&nbsp;{{foo}}" />`;
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,
}
]);
});
});
});
45 changes: 43 additions & 2 deletions packages/compiler/src/expression_parser/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, Quote, RecursiveAstVisitor, SafeCall, SafeKeyedRead, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, ThisReceiver, Unary, VariableBinding} from './ast';
import {EOF, isIdentifier, Lexer, Token, TokenType} from './lexer';
Expand Down Expand Up @@ -148,9 +149,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[] = [];
Expand Down Expand Up @@ -205,6 +207,7 @@ export class Parser {
*/
splitInterpolation(
input: string, location: string,
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null,
interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): SplitInterpolation {
const strings: InterpolationPiece[] = [];
const expressions: InterpolationPiece[] = [];
Expand Down Expand Up @@ -246,7 +249,10 @@ export class Parser {
`at column ${i} in`, location);
}
expressions.push({text, start: fullStart, end: fullEnd});
offsets.push(exprStart);
const offset = interpolatedTokens ?
computeOffsetInOriginalTemplate(fullStart, interpolatedTokens) + interpStart.length :
exprStart;
offsets.push(offset);

i = fullEnd;
atInterpolation = false;
Expand Down Expand Up @@ -1298,3 +1304,38 @@ class SimpleExpressionChecker extends RecursiveAstVisitor {
this.errors.push('pipes');
}
}
/**
* Computes the real offset in the original template for a given input index 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 exprStart The start of the expression being processed. This start location is the
* _decoded_ template input to the prase interpolation.
* @param interpolatedTokens The tokens for the interpolated value.
* @returns
*/
function computeOffsetInOriginalTemplate(
exprStart: number,
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]): number {
let consumedInOriginalTemplate = 0;
let consumedInInput = 0;
let tokenIndex = 0;
while (consumedInInput < exprStart && 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;
}
tokenIndex++;
}
return consumedInOriginalTemplate;
}
14 changes: 9 additions & 5 deletions packages/compiler/src/render3/r3_template_transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -256,7 +257,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 {
Expand Down Expand Up @@ -289,7 +290,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);
Expand Down Expand Up @@ -444,14 +445,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);
}

Expand Down
13 changes: 9 additions & 4 deletions packages/compiler/src/template_parser/binding_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -93,13 +94,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) {
Expand Down Expand Up @@ -273,8 +277,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);
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler/test/expression_parser/parser_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,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');
Expand Down Expand Up @@ -1211,11 +1212,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 {
Expand Down
39 changes: 39 additions & 0 deletions packages/compiler/test/render3/r3_ast_absolute_span_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,45 @@ describe('expression AST absolute source spans', () => {
['2', new AbsoluteSourceSpan(11, 12)],
]));
});

it('should handle HTML entity before interpolation', () => {
expect(humanizeExpressionSource(parse('&nbsp;{{abc}}').nodes))
.toEqual(jasmine.arrayContaining([
['abc', new AbsoluteSourceSpan(8, 11)],
]));
});

it('should handle many HTML entities and many interpolations', () => {
expect(humanizeExpressionSource(parse('&quot;{{abc}}&quot;{{def}}&nbsp;{{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('<div class="{{abc}}"><div>').nodes))
.toEqual(jasmine.arrayContaining([
['abc', new AbsoluteSourceSpan(14, 17)],
]));
});

it('should handle interpolation preceded by HTML entity in attribute', () => {
expect(humanizeExpressionSource(parse('<div class="&nbsp;{{abc}}"><div>').nodes))
.toEqual(jasmine.arrayContaining([
['abc', new AbsoluteSourceSpan(20, 23)],
]));
});

it('should handle many interpolation with HTML entities in attribute', () => {
expect(humanizeExpressionSource(
parse('<div class="&quot;{{abc}}&quot;&nbsp;{{def}}"><div>').nodes))
.toEqual(jasmine.arrayContaining([
['abc', new AbsoluteSourceSpan(20, 23)],
['def', new AbsoluteSourceSpan(39, 42)],
]));
});
});

describe('keyed read', () => {
Expand Down
10 changes: 5 additions & 5 deletions packages/compiler/test/render3/view/i18n_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand All @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit eae58a9

Please sign in to comment.