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
This commit is contained in:
Pete Bacon Darwin 2020-10-29 09:45:01 +00:00 committed by Misko Hevery
parent 43d8e9aad2
commit 1f956184c4
4 changed files with 30 additions and 7 deletions

View File

@ -203,7 +203,7 @@ class _I18nVisitor implements html.Visitor {
function getOffsetSourceSpan( function getOffsetSourceSpan(
sourceSpan: ParseSourceSpan, {start, end}: {start: number, end: number}): ParseSourceSpan { 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 = const _CUSTOM_PH_EXP =

View File

@ -55,7 +55,7 @@ const EVENT_BINDING_SCOPE_GLOBALS = new Set<string>(['$event']);
const GLOBAL_TARGET_RESOLVERS = new Map<string, o.ExternalReference>( const GLOBAL_TARGET_RESOLVERS = new Map<string, o.ExternalReference>(
[['window', R3.resolveWindow], ['document', R3.resolveDocument], ['body', R3.resolveBody]]); [['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) { .. } // if (rf & flags) { .. }
export function renderFlagCheckIfStmt( export function renderFlagCheckIfStmt(

View File

@ -18,6 +18,7 @@ import {serializeIcuNode} from '../../../src/render3/view/i18n/icu_serializer';
import {serializeI18nMessageForLocalize} from '../../../src/render3/view/i18n/localize_utils'; import {serializeI18nMessageForLocalize} from '../../../src/render3/view/i18n/localize_utils';
import {I18nMeta, parseI18nMeta} from '../../../src/render3/view/i18n/meta'; import {I18nMeta, parseI18nMeta} from '../../../src/render3/view/i18n/meta';
import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util'; import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util';
import {LEADING_TRIVIA_CHARS} from '../../../src/render3/view/template';
import {parseR3 as parse} from './util'; import {parseR3 as parse} from './util';
@ -386,7 +387,7 @@ describe('serializeI18nMessageForGetMsg', () => {
describe('serializeI18nMessageForLocalize', () => { describe('serializeI18nMessageForLocalize', () => {
const serialize = (input: string) => { const serialize = (input: string) => {
const tree = parse(`<div i18n>${input}</div>`); const tree = parse(`<div i18n>${input}</div>`, {leadingTriviaChars: LEADING_TRIVIA_CHARS});
const root = tree.nodes[0] as t.Element; const root = tree.nodes[0] as t.Element;
return serializeI18nMessageForLocalize(root.i18n as i18n.Message); return serializeI18nMessageForLocalize(root.i18n as i18n.Message);
}; };
@ -509,6 +510,26 @@ describe('serializeI18nMessageForLocalize', () => {
expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"</b>" (22-26)'); expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"</b>" (22-26)');
}); });
it('should create the correct placeholder source-spans when there is skipped leading whitespace',
() => {
const {messageParts, placeHolders} = serialize('<b> {{value}}</b>');
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('"<b> " (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('"</b>" (25-29)');
});
it('should serialize simple ICU for `$localize()`', () => { it('should serialize simple ICU for `$localize()`', () => {
expect(serialize('{age, plural, 10 {ten} other {other}}')).toEqual({ expect(serialize('{age, plural, 10 {ten} other {other}}')).toEqual({
messageParts: [literal('{VAR_PLURAL, plural, 10 {ten} other {other}}')], messageParts: [literal('{VAR_PLURAL, plural, 10 {ten} other {other}}')],

View File

@ -78,11 +78,13 @@ export function toStringExpression(expr: e.AST): string {
// Parse an html string to IVY specific info // Parse an html string to IVY specific info
export function parseR3( export function parseR3(
input: string, options: {preserveWhitespaces?: boolean} = {}): Render3ParseResult { input: string, options: {preserveWhitespaces?: boolean, leadingTriviaChars?: string[]} = {}):
Render3ParseResult {
const htmlParser = new HtmlParser(); const htmlParser = new HtmlParser();
const parseResult = const parseResult = htmlParser.parse(
htmlParser.parse(input, 'path:://to/template', {tokenizeExpansionForms: true}); input, 'path:://to/template',
{tokenizeExpansionForms: true, leadingTriviaChars: options.leadingTriviaChars});
if (parseResult.errors.length > 0) { if (parseResult.errors.length > 0) {
const msg = parseResult.errors.map(e => e.toString()).join('\n'); const msg = parseResult.errors.map(e => e.toString()).join('\n');