From 1f956184c426814be8fcbdab03396c910e29d935 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 29 Oct 2020 09:45:01 +0000 Subject: [PATCH] fix(compiler): skipping leading whitespace should not break placeholder source-spans (#39486) Tokenized text node may have leading whitespace skipped from their source-span. But the source-span is used to compute where there are interpolated blocks, resulting in placeholder nodes whose source-spans are offset by the amount of skipped characters. This fix uses the `fullStart` location of text source-spans for computing the source-span of placeholders, so that they are accurate. Fixes #39195 PR Close #39486 --- packages/compiler/src/i18n/i18n_parser.ts | 2 +- .../compiler/src/render3/view/template.ts | 2 +- .../compiler/test/render3/view/i18n_spec.ts | 25 +++++++++++++++++-- packages/compiler/test/render3/view/util.ts | 8 +++--- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/compiler/src/i18n/i18n_parser.ts b/packages/compiler/src/i18n/i18n_parser.ts index 4fa1f55f4b..c22941f16f 100644 --- a/packages/compiler/src/i18n/i18n_parser.ts +++ b/packages/compiler/src/i18n/i18n_parser.ts @@ -203,7 +203,7 @@ class _I18nVisitor implements html.Visitor { function getOffsetSourceSpan( sourceSpan: ParseSourceSpan, {start, end}: {start: number, end: number}): ParseSourceSpan { - return new ParseSourceSpan(sourceSpan.start.moveBy(start), sourceSpan.start.moveBy(end)); + return new ParseSourceSpan(sourceSpan.fullStart.moveBy(start), sourceSpan.fullStart.moveBy(end)); } const _CUSTOM_PH_EXP = diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index f6e8707313..126c970212 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -55,7 +55,7 @@ const EVENT_BINDING_SCOPE_GLOBALS = new Set(['$event']); const GLOBAL_TARGET_RESOLVERS = new Map( [['window', R3.resolveWindow], ['document', R3.resolveDocument], ['body', R3.resolveBody]]); -const LEADING_TRIVIA_CHARS = [' ', '\n', '\r', '\t']; +export const LEADING_TRIVIA_CHARS = [' ', '\n', '\r', '\t']; // if (rf & flags) { .. } export function renderFlagCheckIfStmt( diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index 2e8ca157b6..b6fca34b77 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -18,6 +18,7 @@ import {serializeIcuNode} from '../../../src/render3/view/i18n/icu_serializer'; import {serializeI18nMessageForLocalize} from '../../../src/render3/view/i18n/localize_utils'; import {I18nMeta, parseI18nMeta} from '../../../src/render3/view/i18n/meta'; import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util'; +import {LEADING_TRIVIA_CHARS} from '../../../src/render3/view/template'; import {parseR3 as parse} from './util'; @@ -386,7 +387,7 @@ describe('serializeI18nMessageForGetMsg', () => { describe('serializeI18nMessageForLocalize', () => { const serialize = (input: string) => { - const tree = parse(`
${input}
`); + const tree = parse(`
${input}
`, {leadingTriviaChars: LEADING_TRIVIA_CHARS}); const root = tree.nodes[0] as t.Element; return serializeI18nMessageForLocalize(root.i18n as i18n.Message); }; @@ -477,7 +478,7 @@ describe('serializeI18nMessageForLocalize', () => { expect(messageParts[3].text).toEqual(''); expect(messageParts[3].sourceSpan.toString()).toEqual(''); expect(messageParts[4].text).toEqual(' D'); - expect(messageParts[4].sourceSpan.toString()).toEqual(' D'); + expect(messageParts[4].sourceSpan.toString()).toEqual('D'); expect(placeHolders[0].text).toEqual('START_TAG_SPAN'); expect(placeHolders[0].sourceSpan.toString()).toEqual(''); @@ -509,6 +510,26 @@ describe('serializeI18nMessageForLocalize', () => { expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"" (22-26)'); }); + it('should create the correct placeholder source-spans when there is skipped leading whitespace', + () => { + const {messageParts, placeHolders} = serialize(' {{value}}'); + expect(messageParts[0].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[0].sourceSpan)).toEqual('"" (10-10)'); + expect(messageParts[1].text).toEqual(' '); + expect(humanizeSourceSpan(messageParts[1].sourceSpan)).toEqual('" " (13-16)'); + expect(messageParts[2].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[2].sourceSpan)).toEqual('"" (25-25)'); + expect(messageParts[3].text).toEqual(''); + expect(humanizeSourceSpan(messageParts[3].sourceSpan)).toEqual('"" (29-29)'); + + expect(placeHolders[0].text).toEqual('START_BOLD_TEXT'); + expect(humanizeSourceSpan(placeHolders[0].sourceSpan)).toEqual('" " (10-16)'); + expect(placeHolders[1].text).toEqual('INTERPOLATION'); + expect(humanizeSourceSpan(placeHolders[1].sourceSpan)).toEqual('"{{value}}" (16-25)'); + expect(placeHolders[2].text).toEqual('CLOSE_BOLD_TEXT'); + expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"" (25-29)'); + }); + it('should serialize simple ICU for `$localize()`', () => { expect(serialize('{age, plural, 10 {ten} other {other}}')).toEqual({ messageParts: [literal('{VAR_PLURAL, plural, 10 {ten} other {other}}')], diff --git a/packages/compiler/test/render3/view/util.ts b/packages/compiler/test/render3/view/util.ts index af3ed25c11..e974e0e155 100644 --- a/packages/compiler/test/render3/view/util.ts +++ b/packages/compiler/test/render3/view/util.ts @@ -78,11 +78,13 @@ export function toStringExpression(expr: e.AST): string { // Parse an html string to IVY specific info export function parseR3( - input: string, options: {preserveWhitespaces?: boolean} = {}): Render3ParseResult { + input: string, options: {preserveWhitespaces?: boolean, leadingTriviaChars?: string[]} = {}): + Render3ParseResult { const htmlParser = new HtmlParser(); - const parseResult = - htmlParser.parse(input, 'path:://to/template', {tokenizeExpansionForms: true}); + const parseResult = htmlParser.parse( + input, 'path:://to/template', + {tokenizeExpansionForms: true, leadingTriviaChars: options.leadingTriviaChars}); if (parseResult.errors.length > 0) { const msg = parseResult.errors.map(e => e.toString()).join('\n');