From 70dd27ffd8f936727c61d81722217e8956c9c14d Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sun, 26 Apr 2020 18:15:43 +0100 Subject: [PATCH] fix(compiler): normalize line endings in ICU expansions (#36741) The html parser already normalizes line endings (converting `\r\n` to `\n`) for most text in templates but it was missing the expressions of ICU expansions. In ViewEngine backticked literal strings, used to define inline templates, were already normalized by the TypeScript parser. In Ivy we are parsing the raw text of the source file directly so the line endings need to be manually normalized. This change ensures that inline templates have the line endings of ICU expression normalized correctly, which matches the ViewEngine. In ViewEngine external templates, defined in HTML files, the behavior was different, since TypeScript was not normalizing the line endings. Specifically, ICU expansion "expressions" are not being normalized. This is a problem because it means that i18n message ids can be different on different machines that are setup with different line ending handling, or if the developer moves a template from inline to external or vice versa. The goal is always to normalize line endings, whether inline or external. But this would be a breaking change since it would change i18n message ids that have been previously computed. Therefore this commit aligns the ivy template parsing to have the same "buggy" behavior for external templates. There is now a compiler option `i18nNormalizeLineEndingsInICUs`, which if set to `true` will ensure the correct non-buggy behavior. For the time being this option defaults to `false` to ensure backward compatibility while allowing opt-in to the desired behavior. This option's default will be flipped in a future breaking change release. Further, when this option is set to `false`, any ICU expression tokens, which have not been normalized, are added to the `ParseResult` from the `HtmlParser.parse()` method. In the future, this collection of tokens could be used to diagnose and encourage developers to migrate their i18n message ids. See FW-2106. Closes #36725 PR Close #36741 --- .../compiler-cli/compiler_options.d.ts | 1 + .../ngcc/src/analysis/decoration_analyzer.ts | 5 +- .../src/ngtsc/annotations/src/component.ts | 3 + .../ngtsc/annotations/test/component_spec.ts | 3 +- .../src/ngtsc/core/api/src/public_options.ts | 13 ++ .../src/ngtsc/core/src/compiler.ts | 7 +- .../compliance/r3_view_compiler_i18n_spec.ts | 72 +++++- packages/compiler/src/ml_parser/lexer.ts | 35 ++- packages/compiler/src/ml_parser/parser.ts | 4 +- .../compiler/src/render3/view/template.ts | 9 + .../test/ml_parser/html_parser_spec.ts | 217 ++++++++++++++++++ .../compiler/test/ml_parser/lexer_spec.ts | 201 +++++++++++++++- 12 files changed, 553 insertions(+), 17 deletions(-) diff --git a/goldens/public-api/compiler-cli/compiler_options.d.ts b/goldens/public-api/compiler-cli/compiler_options.d.ts index b5ca8bf599..92723d3050 100644 --- a/goldens/public-api/compiler-cli/compiler_options.d.ts +++ b/goldens/public-api/compiler-cli/compiler_options.d.ts @@ -6,6 +6,7 @@ export interface BazelAndG3Options { export interface I18nOptions { enableI18nLegacyMessageIdFormat?: boolean; i18nInLocale?: string; + i18nNormalizeLineEndingsInICUs?: boolean; i18nUseExternalIds?: boolean; } diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index 2b66c5662d..32c8af2a09 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -96,8 +96,9 @@ export class DecorationAnalyzer { this.scopeRegistry, this.scopeRegistry, this.isCore, this.resourceManager, this.rootDirs, !!this.compilerOptions.preserveWhitespaces, /* i18nUseExternalIds */ true, this.bundle.enableI18nLegacyMessageIdFormat, - this.moduleResolver, this.cycleAnalyzer, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER, - NOOP_DEPENDENCY_TRACKER, this.injectableRegistry, /* annotateForClosureCompiler */ false), + /* i18nNormalizeLineEndingsInICUs */ false, this.moduleResolver, this.cycleAnalyzer, + this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER, NOOP_DEPENDENCY_TRACKER, + this.injectableRegistry, /* annotateForClosureCompiler */ false), // See the note in ngtsc about why this cast is needed. // clang-format off new DirectiveDecoratorHandler( diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 1aa775a1eb..7e7ed4c0ca 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -82,6 +82,7 @@ export class ComponentDecoratorHandler implements private isCore: boolean, private resourceLoader: ResourceLoader, private rootDirs: ReadonlyArray, private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean, private enableI18nLegacyMessageIdFormat: boolean, + private i18nNormalizeLineEndingsInICUs: boolean|undefined, private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer, private refEmitter: ReferenceEmitter, private defaultImportRecorder: DefaultImportRecorder, private depTracker: DependencyTracker|null, @@ -780,6 +781,7 @@ export class ComponentDecoratorHandler implements range: templateRange, escapedString, enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat, + i18nNormalizeLineEndingsInICUs: this.i18nNormalizeLineEndingsInICUs, }); // Unfortunately, the primary parse of the template above may not contain accurate source map @@ -801,6 +803,7 @@ export class ComponentDecoratorHandler implements range: templateRange, escapedString, enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat, + i18nNormalizeLineEndingsInICUs: this.i18nNormalizeLineEndingsInICUs, leadingTriviaChars: [], }); diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts index fa952d1a60..b2df657739 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts @@ -71,7 +71,8 @@ runInEachFileSystem(() => { reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, scopeRegistry, /* isCore */ false, new NoopResourceLoader(), /* rootDirs */[''], /* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true, - /* enableI18nLegacyMessageIdFormat */ false, moduleResolver, cycleAnalyzer, refEmitter, + /* enableI18nLegacyMessageIdFormat */ false, + /* i18nNormalizeLineEndingsInICUs */ undefined, moduleResolver, cycleAnalyzer, refEmitter, NOOP_DEFAULT_IMPORT_RECORDER, /* depTracker */ null, injectableRegistry, /* annotateForClosureCompiler */ false); const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration); diff --git a/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts b/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts index caa0670dcb..e946298883 100644 --- a/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts +++ b/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts @@ -313,6 +313,19 @@ export interface I18nOptions { * (used by Closure Compiler's output of `goog.getMsg` for transition period) */ i18nUseExternalIds?: boolean; + + /** + * If templates are stored in external files (e.g. via `templateUrl`) then we need to decide + * whether or not to normalize the line-endings (from `\r\n` to `\n`) when processing ICU + * expressions. + * + * Ideally we would always normalize, but for backward compatibility this flag allows the template + * parser to avoid normalizing line endings in ICU expressions. + * + * If `true` then we will normalize ICU expression line endings. + * The default is `false`, but this will be switched in a future major release. + */ + i18nNormalizeLineEndingsInICUs?: boolean; } /** diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index f932927066..91edb09bb0 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -659,9 +659,10 @@ export class NgCompiler { reflector, evaluator, metaRegistry, metaReader, scopeReader, scopeRegistry, isCore, this.resourceManager, this.host.rootDirs, this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false, - this.options.enableI18nLegacyMessageIdFormat !== false, this.moduleResolver, - this.cycleAnalyzer, refEmitter, defaultImportTracker, this.incrementalDriver.depGraph, - injectableRegistry, this.closureCompilerEnabled), + this.options.enableI18nLegacyMessageIdFormat !== false, + this.options.i18nNormalizeLineEndingsInICUs, this.moduleResolver, this.cycleAnalyzer, + refEmitter, defaultImportTracker, this.incrementalDriver.depGraph, injectableRegistry, + this.closureCompilerEnabled), // TODO(alxhub): understand why the cast here is necessary (something to do with `null` // not being assignable to `unknown` when wrapped in `Readonly`). // clang-format off diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts index d193d48970..cfb73846a3 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts @@ -124,6 +124,7 @@ const escapeTemplate = (template: string) => const getAppFilesWithTemplate = (template: string, args: any = {}) => ({ app: { + 'spec.template.html': template, 'spec.ts': ` import {Component, NgModule} from '@angular/core'; @@ -131,8 +132,9 @@ const getAppFilesWithTemplate = (template: string, args: any = {}) => ({ selector: 'my-component', ${args.preserveWhitespaces ? 'preserveWhitespaces: true,' : ''} ${args.interpolation ? 'interpolation: ' + JSON.stringify(args.interpolation) + ', ' : ''} - template: \`${escapeTemplate(template)}\` - }) + ${ + args.templateUrl ? `templateUrl: 'spec.template.html'` : + `template: \`${escapeTemplate(template)}\``}) export class MyComponent {} @NgModule({declarations: [MyComponent]}) @@ -3693,6 +3695,72 @@ describe('i18n support in the template compiler', () => { }); }); + describe('line ending normalization', () => { + [true, false].forEach( + templateUrl => describe(templateUrl ? '[templateUrl]' : '[inline template]', () => { + [true, false, undefined].forEach( + i18nNormalizeLineEndingsInICUs => describe( + `{i18nNormalizeLineEndingsInICUs: ${i18nNormalizeLineEndingsInICUs}}`, () => { + it('should normalize line endings in templates', () => { + const input = + `
\r\nSome Message\r\n{\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n}
`; + + const output = String.raw` + $I18N_0$ = $localize \`abc +def\`; + … + $I18N_4$ = $localize \`{VAR_SELECT, select, =0 {zero + }}\` + … + $I18N_3$ = $localize \` +Some Message +$` + String.raw`{$I18N_4$}:ICU:\`; + `; + + verify(input, output, { + inputArgs: {templateUrl}, + compilerOptions: {i18nNormalizeLineEndingsInICUs} + }); + }); + + it('should compute the correct message id for messages', () => { + const input = + `
\r\nSome Message\r\n{\r\n value,\r\n select,\r\n =0 {\r\n zero\r\n }\r\n}
`; + + // The ids generated by the compiler are different if the template is external + // and we are not explicitly normalizing the line endings. + const ICU_EXPRESSION_ID = + templateUrl && i18nNormalizeLineEndingsInICUs !== true ? + `␟70a685282be2d956e4db234fa3d985970672faa0` : + `␟b5fe162f4e47ab5b3e534491d30b715e0dff0f52`; + const ICU_ID = templateUrl && i18nNormalizeLineEndingsInICUs !== true ? + `␟6a55b51b9bcf8f84b1b868c585ae09949668a72b` : + `␟e31c7bc4db2f2e56dc40f005958055a02fd43a2e`; + + const output = + String.raw` + $I18N_0$ = $localize \`:␟4f9ce2c66b187afd9898b25f6336d1eb2be8b5dc␟7326958852138509669:abc +def\`; + … + $I18N_4$ = $localize \`:${ + ICU_EXPRESSION_ID}␟4863953183043480207:{VAR_SELECT, select, =0 {zero + }}\` + … + $I18N_3$ = $localize \`:${ICU_ID}␟2773178924738647105: +Some Message +$` + String.raw`{$I18N_4$}:ICU:\`; + `; + + verify(input, output, { + inputArgs: {templateUrl}, + compilerOptions: + {i18nNormalizeLineEndingsInICUs, enableI18nLegacyMessageIdFormat: true} + }); + }); + })); + })); + }); + describe('errors', () => { const verifyNestedSectionsError = (errorThrown: any, expectedErrorText: string) => { expect(errorThrown.ngParseErrors.length).toBe(1); diff --git a/packages/compiler/src/ml_parser/lexer.ts b/packages/compiler/src/ml_parser/lexer.ts index c9e6e33497..625cdff79c 100644 --- a/packages/compiler/src/ml_parser/lexer.ts +++ b/packages/compiler/src/ml_parser/lexer.ts @@ -48,7 +48,9 @@ export class TokenError extends ParseError { } export class TokenizeResult { - constructor(public tokens: Token[], public errors: TokenError[]) {} + constructor( + public tokens: Token[], public errors: TokenError[], + public nonNormalizedIcuExpressions: Token[]) {} } export interface LexerRange { @@ -95,6 +97,15 @@ export interface TokenizeOptions { * but the new line should increment the current line for source mapping. */ escapedString?: boolean; + /** + * If this text is stored in an external template (e.g. via `templateUrl`) then we need to decide + * whether or not to normalize the line-endings (from `\r\n` to `\n`) when processing ICU + * expressions. + * + * If `true` then we will normalize ICU expression line endings. + * The default is `false`, but this will be switched in a future major release. + */ + i18nNormalizeLineEndingsInICUs?: boolean; /** * An array of characters that should be considered as leading trivia. * Leading trivia are characters that are not important to the developer, and so should not be @@ -112,7 +123,8 @@ export function tokenize( options: TokenizeOptions = {}): TokenizeResult { const tokenizer = new _Tokenizer(new ParseSourceFile(source, url), getTagDefinition, options); tokenizer.tokenize(); - return new TokenizeResult(mergeTextTokens(tokenizer.tokens), tokenizer.errors); + return new TokenizeResult( + mergeTextTokens(tokenizer.tokens), tokenizer.errors, tokenizer.nonNormalizedIcuExpressions); } const _CR_OR_CRLF_REGEXP = /\r\n?/g; @@ -141,8 +153,11 @@ class _Tokenizer { private _expansionCaseStack: TokenType[] = []; private _inInterpolation: boolean = false; private readonly _preserveLineEndings: boolean; + private readonly _escapedString: boolean; + private readonly _i18nNormalizeLineEndingsInICUs: boolean; tokens: Token[] = []; errors: TokenError[] = []; + nonNormalizedIcuExpressions: Token[] = []; /** * @param _file The html source file being tokenized. @@ -161,6 +176,8 @@ class _Tokenizer { this._cursor = options.escapedString ? new EscapedCharacterCursor(_file, range) : new PlainCharacterCursor(_file, range); this._preserveLineEndings = options.preserveLineEndings || false; + this._escapedString = options.escapedString || false; + this._i18nNormalizeLineEndingsInICUs = options.i18nNormalizeLineEndingsInICUs || false; try { this._cursor.init(); } catch (e) { @@ -609,7 +626,19 @@ class _Tokenizer { this._beginToken(TokenType.RAW_TEXT); const condition = this._readUntil(chars.$COMMA); - this._endToken([condition]); + const normalizedCondition = this._processCarriageReturns(condition); + if (this._escapedString || this._i18nNormalizeLineEndingsInICUs) { + // Either the template is inline or, + // we explicitly want to normalize line endings for this text. + this._endToken([normalizedCondition]); + } else { + // The expression is in an external template and, for backward compatibility, + // we are not normalizing line endings. + const conditionToken = this._endToken([condition]); + if (normalizedCondition !== condition) { + this.nonNormalizedIcuExpressions.push(conditionToken); + } + } this._requireCharCode(chars.$COMMA); this._attemptCharCodeUntilFn(isNotWhitespace); diff --git a/packages/compiler/src/ml_parser/parser.ts b/packages/compiler/src/ml_parser/parser.ts index eb9564a407..fa7f8874a1 100644 --- a/packages/compiler/src/ml_parser/parser.ts +++ b/packages/compiler/src/ml_parser/parser.ts @@ -34,7 +34,9 @@ export class Parser { const parser = new _TreeBuilder(tokenizeResult.tokens, this.getTagDefinition); parser.build(); return new ParseTreeResult( - parser.rootNodes, (tokenizeResult.errors as ParseError[]).concat(parser.errors)); + parser.rootNodes, + (tokenizeResult.errors as ParseError[]).concat(parser.errors), + ); } } diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 0b477ef57d..6d33831324 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -1986,6 +1986,15 @@ export interface ParseTemplateOptions { * `$localize` message id format and you are not using compile time translation merging. */ enableI18nLegacyMessageIdFormat?: boolean; + /** + * If this text is stored in an external template (e.g. via `templateUrl`) then we need to decide + * whether or not to normalize the line-endings (from `\r\n` to `\n`) when processing ICU + * expressions. + * + * If `true` then we will normalize ICU expression line endings. + * The default is `false`, but this will be switched in a future major release. + */ + i18nNormalizeLineEndingsInICUs?: boolean; } /** diff --git a/packages/compiler/test/ml_parser/html_parser_spec.ts b/packages/compiler/test/ml_parser/html_parser_spec.ts index a488d827ac..25e4209b74 100644 --- a/packages/compiler/test/ml_parser/html_parser_spec.ts +++ b/packages/compiler/test/ml_parser/html_parser_spec.ts @@ -44,6 +44,14 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe [html.Text, 'text', 0] ]); }); + + it('should normalize line endings within CDATA', () => { + const parsed = parser.parse('', 'TestComp'); + expect(humanizeDom(parsed)).toEqual([ + [html.Text, ' line 1 \n line 2 ', 0], + ]); + expect(parsed.errors).toEqual([]); + }); }); describe('elements', () => { @@ -200,6 +208,37 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe [html.Text, '\n', 1], ]); }); + + it('should normalize line endings in text', () => { + let parsed: ParseTreeResult; + parsed = parser.parse(' line 1 \r\n line 2 ', 'TestComp'); + expect(humanizeDom(parsed)).toEqual([ + [html.Element, 'title', 0], + [html.Text, ' line 1 \n line 2 ', 1], + ]); + expect(parsed.errors).toEqual([]); + + parsed = parser.parse('', 'TestComp'); + expect(humanizeDom(parsed)).toEqual([ + [html.Element, 'script', 0], + [html.Text, ' line 1 \n line 2 ', 1], + ]); + expect(parsed.errors).toEqual([]); + + parsed = parser.parse('
line 1 \r\n line 2
', 'TestComp'); + expect(humanizeDom(parsed)).toEqual([ + [html.Element, 'div', 0], + [html.Text, ' line 1 \n line 2 ', 1], + ]); + expect(parsed.errors).toEqual([]); + + parsed = parser.parse(' line 1 \r\n line 2 ', 'TestComp'); + expect(humanizeDom(parsed)).toEqual([ + [html.Element, 'span', 0], + [html.Text, ' line 1 \n line 2 ', 1], + ]); + expect(parsed.errors).toEqual([]); + }); }); describe('attributes', () => { @@ -211,6 +250,16 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe ]); }); + it('should normalize line endings within attribute values', () => { + const result = + parser.parse('
', 'TestComp'); + expect(humanizeDom(result)).toEqual([ + [html.Element, 'div', 0], + [html.Attribute, 'key', ' \n line 1 \n line 2 '], + ]); + expect(result.errors).toEqual([]); + }); + it('should parse attributes without values', () => { expect(humanizeDom(parser.parse('
', 'TestComp'))).toEqual([ [html.Element, 'div', 0], @@ -248,6 +297,11 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe [html.Element, 'div', 0], ]); }); + it('should normalize line endings within comments', () => { + expect(humanizeDom(parser.parse('', 'TestComp'))).toEqual([ + [html.Comment, 'line 1 \n line 2', 0], + ]); + }); }); describe('expansion forms', () => { @@ -278,6 +332,111 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe ]); }); + it('should normalize line-endings in expansion forms in inline templates', () => { + const parsed = parser.parse( + `
\r\n` + + ` {\r\n` + + ` messages.length,\r\n` + + ` plural,\r\n` + + ` =0 {You have \r\nno\r\n messages}\r\n` + + ` =1 {One {{message}}}}\r\n` + + `
`, + 'TestComp', { + tokenizeExpansionForms: true, + escapedString: true, + }); + + expect(humanizeDom(parsed)).toEqual([ + [html.Element, 'div', 0], + [html.Text, '\n ', 1], + [html.Expansion, '\n messages.length', 'plural', 1], + [html.ExpansionCase, '=0', 2], + [html.ExpansionCase, '=1', 2], + [html.Text, '\n', 1], + ]); + const cases = (parsed.rootNodes[0]).children[1].cases; + + expect(humanizeDom(new ParseTreeResult(cases[0].expression, []))).toEqual([ + [html.Text, 'You have \nno\n messages', 0], + ]); + + expect(humanizeDom(new ParseTreeResult(cases[1].expression, []))).toEqual([ + [html.Text, 'One {{message}}', 0] + ]); + + expect(parsed.errors).toEqual([]); + }); + + it('should normalize line-endings in expansion forms in external templates if `i18nNormalizeLineEndingsInICUs` is true', + () => { + const parsed = parser.parse( + `
\r\n` + + ` {\r\n` + + ` messages.length,\r\n` + + ` plural,\r\n` + + ` =0 {You have \r\nno\r\n messages}\r\n` + + ` =1 {One {{message}}}}\r\n` + + `
`, + 'TestComp', { + tokenizeExpansionForms: true, + escapedString: false, + i18nNormalizeLineEndingsInICUs: true + }); + + expect(humanizeDom(parsed)).toEqual([ + [html.Element, 'div', 0], + [html.Text, '\n ', 1], + [html.Expansion, '\n messages.length', 'plural', 1], + [html.ExpansionCase, '=0', 2], + [html.ExpansionCase, '=1', 2], + [html.Text, '\n', 1], + ]); + const cases = (parsed.rootNodes[0]).children[1].cases; + + expect(humanizeDom(new ParseTreeResult(cases[0].expression, []))).toEqual([ + [html.Text, 'You have \nno\n messages', 0], + ]); + + expect(humanizeDom(new ParseTreeResult(cases[1].expression, []))).toEqual([ + [html.Text, 'One {{message}}', 0] + ]); + + expect(parsed.errors).toEqual([]); + }); + + it('should not normalize line-endings in ICU expressions in external templates when `i18nNormalizeLineEndingsInICUs` is not set', + () => { + const parsed = parser.parse( + `
\r\n` + + ` {\r\n` + + ` messages.length,\r\n` + + ` plural,\r\n` + + ` =0 {You have \r\nno\r\n messages}\r\n` + + ` =1 {One {{message}}}}\r\n` + + `
`, + 'TestComp', {tokenizeExpansionForms: true, escapedString: false}); + + expect(humanizeDom(parsed)).toEqual([ + [html.Element, 'div', 0], + [html.Text, '\n ', 1], + [html.Expansion, '\r\n messages.length', 'plural', 1], + [html.ExpansionCase, '=0', 2], + [html.ExpansionCase, '=1', 2], + [html.Text, '\n', 1], + ]); + const cases = (parsed.rootNodes[0]).children[1].cases; + + expect(humanizeDom(new ParseTreeResult(cases[0].expression, []))).toEqual([ + [html.Text, 'You have \nno\n messages', 0], + ]); + + expect(humanizeDom(new ParseTreeResult(cases[1].expression, []))).toEqual([ + [html.Text, 'One {{message}}', 0] + ]); + + expect(parsed.errors).toEqual([]); + }); + it('should parse out expansion forms', () => { const parsed = parser.parse( `
{a, plural, =0 {b}}
`, 'TestComp', @@ -309,6 +468,64 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe ]); }); + it('should normalize line endings in nested expansion forms for inline templates', () => { + const parsed = parser.parse( + `{\r\n` + + ` messages.length, plural,\r\n` + + ` =0 { zero \r\n` + + ` {\r\n` + + ` p.gender, select,\r\n` + + ` male {m}\r\n` + + ` }\r\n` + + ` }\r\n` + + `}`, + 'TestComp', {tokenizeExpansionForms: true, escapedString: true}); + expect(humanizeDom(parsed)).toEqual([ + [html.Expansion, '\n messages.length', 'plural', 0], + [html.ExpansionCase, '=0', 1], + ]); + + const expansion = parsed.rootNodes[0] as html.Expansion; + expect(humanizeDom(new ParseTreeResult(expansion.cases[0].expression, []))).toEqual([ + [html.Text, 'zero \n ', 0], + [html.Expansion, '\n p.gender', 'select', 0], + [html.ExpansionCase, 'male', 1], + [html.Text, '\n ', 0], + ]); + + expect(parsed.errors).toEqual([]); + }); + + it('should not normalize line endings in nested expansion forms for external templates, when `i18nNormalizeLineEndingsInICUs` is not set', + () => { + const parsed = parser.parse( + `{\r\n` + + ` messages.length, plural,\r\n` + + ` =0 { zero \r\n` + + ` {\r\n` + + ` p.gender, select,\r\n` + + ` male {m}\r\n` + + ` }\r\n` + + ` }\r\n` + + `}`, + 'TestComp', {tokenizeExpansionForms: true}); + expect(humanizeDom(parsed)).toEqual([ + [html.Expansion, '\r\n messages.length', 'plural', 0], + [html.ExpansionCase, '=0', 1], + ]); + + const expansion = parsed.rootNodes[0] as html.Expansion; + expect(humanizeDom(new ParseTreeResult(expansion.cases[0].expression, []))).toEqual([ + [html.Text, 'zero \n ', 0], + [html.Expansion, '\r\n p.gender', 'select', 0], + [html.ExpansionCase, 'male', 1], + [html.Text, '\n ', 0], + ]); + + + expect(parsed.errors).toEqual([]); + }); + it('should error when expansion form is not closed', () => { const p = parser.parse( `{messages.length, plural, =0 {one}`, 'TestComp', {tokenizeExpansionForms: true}); diff --git a/packages/compiler/test/ml_parser/lexer_spec.ts b/packages/compiler/test/ml_parser/lexer_spec.ts index 8874ad83d4..6189c38f01 100644 --- a/packages/compiler/test/ml_parser/lexer_spec.ts +++ b/packages/compiler/test/ml_parser/lexer_spec.ts @@ -878,8 +878,195 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u [lex.TokenType.EOF], ]); }); + + describe('[line ending normalization', () => { + describe('{escapedString: true}', () => { + it('should normalize line-endings in expansion forms', () => { + const result = tokenizeWithoutErrors( + `{\r\n` + + ` messages.length,\r\n` + + ` plural,\r\n` + + ` =0 {You have \r\nno\r\n messages}\r\n` + + ` =1 {One {{message}}}}\r\n`, + { + tokenizeExpansionForms: true, + escapedString: true, + }); + + expect(humanizeParts(result.tokens)).toEqual([ + [lex.TokenType.EXPANSION_FORM_START], + [lex.TokenType.RAW_TEXT, '\n messages.length'], + [lex.TokenType.RAW_TEXT, 'plural'], + [lex.TokenType.EXPANSION_CASE_VALUE, '=0'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'You have \nno\n messages'], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_CASE_VALUE, '=1'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'One {{message}}'], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_FORM_END], + [lex.TokenType.TEXT, '\n'], + [lex.TokenType.EOF], + ]); + + expect(result.nonNormalizedIcuExpressions).toEqual([]); + }); + + it('should normalize line endings in nested expansion forms for inline templates', () => { + const result = tokenizeWithoutErrors( + `{\r\n` + + ` messages.length, plural,\r\n` + + ` =0 { zero \r\n` + + ` {\r\n` + + ` p.gender, select,\r\n` + + ` male {m}\r\n` + + ` }\r\n` + + ` }\r\n` + + `}`, + {tokenizeExpansionForms: true, escapedString: true}); + expect(humanizeParts(result.tokens)).toEqual([ + [lex.TokenType.EXPANSION_FORM_START], + [lex.TokenType.RAW_TEXT, '\n messages.length'], + [lex.TokenType.RAW_TEXT, 'plural'], + [lex.TokenType.EXPANSION_CASE_VALUE, '=0'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'zero \n '], + + [lex.TokenType.EXPANSION_FORM_START], + [lex.TokenType.RAW_TEXT, '\n p.gender'], + [lex.TokenType.RAW_TEXT, 'select'], + [lex.TokenType.EXPANSION_CASE_VALUE, 'male'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'm'], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_FORM_END], + + [lex.TokenType.TEXT, '\n '], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_FORM_END], + [lex.TokenType.EOF], + ]); + + expect(result.nonNormalizedIcuExpressions).toEqual([]); + }); + }); + + describe('{escapedString: false}', () => { + it('should normalize line-endings in expansion forms if `i18nNormalizeLineEndingsInICUs` is true', + () => { + const result = tokenizeWithoutErrors( + `{\r\n` + + ` messages.length,\r\n` + + ` plural,\r\n` + + ` =0 {You have \r\nno\r\n messages}\r\n` + + ` =1 {One {{message}}}}\r\n`, + { + tokenizeExpansionForms: true, + escapedString: false, + i18nNormalizeLineEndingsInICUs: true + }); + + expect(humanizeParts(result.tokens)).toEqual([ + [lex.TokenType.EXPANSION_FORM_START], + [lex.TokenType.RAW_TEXT, '\n messages.length'], + [lex.TokenType.RAW_TEXT, 'plural'], + [lex.TokenType.EXPANSION_CASE_VALUE, '=0'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'You have \nno\n messages'], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_CASE_VALUE, '=1'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'One {{message}}'], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_FORM_END], + [lex.TokenType.TEXT, '\n'], + [lex.TokenType.EOF], + ]); + + expect(result.nonNormalizedIcuExpressions).toEqual([]); + }); + + it('should not normalize line-endings in ICU expressions when `i18nNormalizeLineEndingsInICUs` is not defined', + () => { + const result = tokenizeWithoutErrors( + `{\r\n` + + ` messages.length,\r\n` + + ` plural,\r\n` + + ` =0 {You have \r\nno\r\n messages}\r\n` + + ` =1 {One {{message}}}}\r\n`, + {tokenizeExpansionForms: true, escapedString: false}); + + expect(humanizeParts(result.tokens)).toEqual([ + [lex.TokenType.EXPANSION_FORM_START], + [lex.TokenType.RAW_TEXT, '\r\n messages.length'], + [lex.TokenType.RAW_TEXT, 'plural'], + [lex.TokenType.EXPANSION_CASE_VALUE, '=0'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'You have \nno\n messages'], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_CASE_VALUE, '=1'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'One {{message}}'], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_FORM_END], + [lex.TokenType.TEXT, '\n'], + [lex.TokenType.EOF], + ]); + + expect(result.nonNormalizedIcuExpressions!.length).toBe(1); + expect(result.nonNormalizedIcuExpressions![0].sourceSpan.toString()) + .toEqual('\r\n messages.length'); + }); + + it('should not normalize line endings in nested expansion forms when `i18nNormalizeLineEndingsInICUs` is not defined', + () => { + const result = tokenizeWithoutErrors( + `{\r\n` + + ` messages.length, plural,\r\n` + + ` =0 { zero \r\n` + + ` {\r\n` + + ` p.gender, select,\r\n` + + ` male {m}\r\n` + + ` }\r\n` + + ` }\r\n` + + `}`, + {tokenizeExpansionForms: true}); + + expect(humanizeParts(result.tokens)).toEqual([ + [lex.TokenType.EXPANSION_FORM_START], + [lex.TokenType.RAW_TEXT, '\r\n messages.length'], + [lex.TokenType.RAW_TEXT, 'plural'], + [lex.TokenType.EXPANSION_CASE_VALUE, '=0'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'zero \n '], + + [lex.TokenType.EXPANSION_FORM_START], + [lex.TokenType.RAW_TEXT, '\r\n p.gender'], + [lex.TokenType.RAW_TEXT, 'select'], + [lex.TokenType.EXPANSION_CASE_VALUE, 'male'], + [lex.TokenType.EXPANSION_CASE_EXP_START], + [lex.TokenType.TEXT, 'm'], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_FORM_END], + + [lex.TokenType.TEXT, '\n '], + [lex.TokenType.EXPANSION_CASE_EXP_END], + [lex.TokenType.EXPANSION_FORM_END], + [lex.TokenType.EOF], + ]); + + expect(result.nonNormalizedIcuExpressions!.length).toBe(2); + expect(result.nonNormalizedIcuExpressions![0].sourceSpan.toString()) + .toEqual('\r\n messages.length'); + expect(result.nonNormalizedIcuExpressions![1].sourceSpan.toString()) + .toEqual('\r\n p.gender'); + }); + }); + }); }); + describe('errors', () => { it('should report unescaped "{" on error', () => { expect(tokenizeAndHumanizeErrors(`

before { after

`, {tokenizeExpansionForms: true})) @@ -1241,7 +1428,7 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u }); } -function tokenizeWithoutErrors(input: string, options?: lex.TokenizeOptions): lex.Token[] { +function tokenizeWithoutErrors(input: string, options?: lex.TokenizeOptions): lex.TokenizeResult { const tokenizeResult = lex.tokenize(input, 'someUrl', getHtmlTagDefinition, options); if (tokenizeResult.errors.length > 0) { @@ -1249,16 +1436,20 @@ function tokenizeWithoutErrors(input: string, options?: lex.TokenizeOptions): le throw new Error(`Unexpected parse errors:\n${errorString}`); } - return tokenizeResult.tokens; + return tokenizeResult; +} + +function humanizeParts(tokens: lex.Token[]) { + return tokens.map(token => [token.type, ...token.parts] as [lex.TokenType, ...string[]]); } function tokenizeAndHumanizeParts(input: string, options?: lex.TokenizeOptions): any[] { - return tokenizeWithoutErrors(input, options).map(token => [token.type].concat(token.parts)); + return humanizeParts(tokenizeWithoutErrors(input, options).tokens); } function tokenizeAndHumanizeSourceSpans(input: string, options?: lex.TokenizeOptions): any[] { return tokenizeWithoutErrors(input, options) - .map(token => [token.type, token.sourceSpan.toString()]); + .tokens.map(token => [token.type, token.sourceSpan.toString()]); } function humanizeLineColumn(location: ParseLocation): string { @@ -1267,7 +1458,7 @@ function humanizeLineColumn(location: ParseLocation): string { function tokenizeAndHumanizeLineColumn(input: string, options?: lex.TokenizeOptions): any[] { return tokenizeWithoutErrors(input, options) - .map(token => [token.type, humanizeLineColumn(token.sourceSpan.start)]); + .tokens.map(token => [token.type, humanizeLineColumn(token.sourceSpan.start)]); } function tokenizeAndHumanizeErrors(input: string, options?: lex.TokenizeOptions): any[] {