From 76e4911e8b42589e9c8383bda6d60ea43b2903ee Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Wed, 2 Nov 2016 17:40:15 -0700 Subject: [PATCH] fix(core): fix placeholders handling in i18n. Prior to this commit, translations were built in the serializers. This could not work as a single translation can be used for different source messages having different placeholder content. Serializers do not try to replace the placeholders any more. Placeholders are replaced by the translation bundle and the source message is given as parameter so that the content of the placeholders is taken into account. Also XMB ids are now independent of the expression which is replaced by a placeholder in the extracted file. fixes #12512 --- .../@angular/compiler-cli/src/extract_i18n.ts | 3 +- modules/@angular/compiler/src/i18n/digest.ts | 21 +- .../compiler/src/i18n/extractor_merger.ts | 1 - .../@angular/compiler/src/i18n/i18n_ast.ts | 11 +- .../compiler/src/i18n/i18n_html_parser.ts | 18 +- .../@angular/compiler/src/i18n/i18n_parser.ts | 8 +- .../src/i18n/serializers/placeholder.ts | 4 + .../src/i18n/serializers/serializer.ts | 4 +- .../compiler/src/i18n/serializers/xliff.ts | 206 +++++++---------- .../compiler/src/i18n/serializers/xmb.ts | 6 +- .../compiler/src/i18n/serializers/xtb.ts | 214 ++++++++---------- .../compiler/src/i18n/translation_bundle.ts | 117 +++++++++- modules/@angular/compiler/src/util.ts | 4 +- .../compiler/test/i18n/digest_spec.ts | 2 - .../test/i18n/extractor_merger_spec.ts | 35 +-- .../compiler/test/i18n/i18n_parser_spec.ts | 10 +- .../compiler/test/i18n/integration_spec.ts | 30 ++- .../compiler/test/i18n/message_bundle_spec.ts | 8 +- .../test/i18n/serializers/placeholder_spec.ts | 2 - .../test/i18n/serializers/xliff_spec.ts | 98 ++++++-- .../test/i18n/serializers/xmb_spec.ts | 6 +- .../test/i18n/serializers/xml_helper_spec.ts | 2 - .../test/i18n/serializers/xtb_spec.ts | 142 +++++------- .../test/i18n/translation_bundle_spec.ts | 110 +++++++++ .../test/ml_parser/ast_serializer_spec.ts | 2 +- 25 files changed, 624 insertions(+), 440 deletions(-) create mode 100644 modules/@angular/compiler/test/i18n/translation_bundle_spec.ts diff --git a/modules/@angular/compiler-cli/src/extract_i18n.ts b/modules/@angular/compiler-cli/src/extract_i18n.ts index 4efcf1cbea..4e976706c1 100644 --- a/modules/@angular/compiler-cli/src/extract_i18n.ts +++ b/modules/@angular/compiler-cli/src/extract_i18n.ts @@ -51,9 +51,8 @@ function extract( case 'xliff': case 'xlf': default: - const htmlParser = new compiler.I18NHtmlParser(new compiler.HtmlParser()); ext = 'xlf'; - serializer = new compiler.Xliff(htmlParser, compiler.DEFAULT_INTERPOLATION_CONFIG); + serializer = new compiler.Xliff(); break; } diff --git a/modules/@angular/compiler/src/i18n/digest.ts b/modules/@angular/compiler/src/i18n/digest.ts index 2b35e63b8b..895e7a167a 100644 --- a/modules/@angular/compiler/src/i18n/digest.ts +++ b/modules/@angular/compiler/src/i18n/digest.ts @@ -13,7 +13,9 @@ export function digest(message: i18n.Message): string { } export function decimalDigest(message: i18n.Message): string { - return fingerprint(serializeNodes(message.nodes).join('') + `[${message.meaning}]`); + const visitor = new _SerializerIgnoreIcuExpVisitor(); + const parts = message.nodes.map(a => a.visit(visitor, null)); + return fingerprint(parts.join('') + `[${message.meaning}]`); } /** @@ -43,7 +45,7 @@ class _SerializerVisitor implements i18n.Visitor { } visitPlaceholder(ph: i18n.Placeholder, context: any): any { - return `${ph.value}`; + return ph.value ? `${ph.value}` : ``; } visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any { @@ -57,6 +59,21 @@ export function serializeNodes(nodes: i18n.Node[]): string[] { return nodes.map(a => a.visit(serializerVisitor, null)); } +/** + * Serialize the i18n ast to something xml-like in order to generate an UID. + * + * Ignore the ICU expressions so that message IDs stays identical if only the expression changes. + * + * @internal + */ +class _SerializerIgnoreIcuExpVisitor extends _SerializerVisitor { + visitIcu(icu: i18n.Icu, context: any): any { + let strCases = Object.keys(icu.cases).map((k: string) => `${k} {${icu.cases[k].visit(this)}}`); + // Do not take the expression into account + return `{${icu.type}, ${strCases.join(', ')}}`; + } +} + /** * Compute the SHA1 of the given string * diff --git a/modules/@angular/compiler/src/i18n/extractor_merger.ts b/modules/@angular/compiler/src/i18n/extractor_merger.ts index 703dd2de1b..ab73961294 100644 --- a/modules/@angular/compiler/src/i18n/extractor_merger.ts +++ b/modules/@angular/compiler/src/i18n/extractor_merger.ts @@ -10,7 +10,6 @@ import * as html from '../ml_parser/ast'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseTreeResult} from '../ml_parser/parser'; -import {digest} from './digest'; import * as i18n from './i18n_ast'; import {createI18nMessageFactory} from './i18n_parser'; import {I18nError} from './parse_util'; diff --git a/modules/@angular/compiler/src/i18n/i18n_ast.ts b/modules/@angular/compiler/src/i18n/i18n_ast.ts index 664feec264..ee33098324 100644 --- a/modules/@angular/compiler/src/i18n/i18n_ast.ts +++ b/modules/@angular/compiler/src/i18n/i18n_ast.ts @@ -22,7 +22,10 @@ export class Message { public description: string) {} } -export interface Node { visit(visitor: Visitor, context?: any): any; } +export interface Node { + sourceSpan: ParseSourceSpan; + visit(visitor: Visitor, context?: any): any; +} export class Text implements Node { constructor(public value: string, public sourceSpan: ParseSourceSpan) {} @@ -30,6 +33,7 @@ export class Text implements Node { visit(visitor: Visitor, context?: any): any { return visitor.visitText(this, context); } } +// TODO(vicb): do we really need this node (vs an array) ? export class Container implements Node { constructor(public children: Node[], public sourceSpan: ParseSourceSpan) {} @@ -37,6 +41,7 @@ export class Container implements Node { } export class Icu implements Node { + public expressionPlaceholder: string; constructor( public expression: string, public type: string, public cases: {[k: string]: Node}, public sourceSpan: ParseSourceSpan) {} @@ -54,13 +59,13 @@ export class TagPlaceholder implements Node { } export class Placeholder implements Node { - constructor(public value: string, public name: string = '', public sourceSpan: ParseSourceSpan) {} + constructor(public value: string, public name: string, public sourceSpan: ParseSourceSpan) {} visit(visitor: Visitor, context?: any): any { return visitor.visitPlaceholder(this, context); } } export class IcuPlaceholder implements Node { - constructor(public value: Icu, public name: string = '', public sourceSpan: ParseSourceSpan) {} + constructor(public value: Icu, public name: string, public sourceSpan: ParseSourceSpan) {} visit(visitor: Visitor, context?: any): any { return visitor.visitIcuPlaceholder(this, context); } } diff --git a/modules/@angular/compiler/src/i18n/i18n_html_parser.ts b/modules/@angular/compiler/src/i18n/i18n_html_parser.ts index 16cfb06304..78317f944f 100644 --- a/modules/@angular/compiler/src/i18n/i18n_html_parser.ts +++ b/modules/@angular/compiler/src/i18n/i18n_html_parser.ts @@ -11,7 +11,6 @@ import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/in import {ParseTreeResult} from '../ml_parser/parser'; import {mergeTranslations} from './extractor_merger'; -import {MessageBundle} from './message_bundle'; import {Serializer} from './serializers/serializer'; import {Xliff} from './serializers/xliff'; import {Xmb} from './serializers/xmb'; @@ -41,32 +40,29 @@ export class I18NHtmlParser implements HtmlParser { } // TODO(vicb): add support for implicit tags / attributes - const messageBundle = new MessageBundle(this._htmlParser, [], {}); - const errors = messageBundle.updateFromTemplate(source, url, interpolationConfig); - if (errors && errors.length) { - return new ParseTreeResult(parseResult.rootNodes, parseResult.errors.concat(errors)); + if (parseResult.errors.length) { + return new ParseTreeResult(parseResult.rootNodes, parseResult.errors); } - const serializer = this._createSerializer(interpolationConfig); - const translationBundle = - TranslationBundle.load(this._translations, url, messageBundle, serializer); + const serializer = this._createSerializer(); + const translationBundle = TranslationBundle.load(this._translations, url, serializer); return mergeTranslations(parseResult.rootNodes, translationBundle, interpolationConfig, [], {}); } - private _createSerializer(interpolationConfig: InterpolationConfig): Serializer { + private _createSerializer(): Serializer { const format = (this._translationsFormat || 'xlf').toLowerCase(); switch (format) { case 'xmb': return new Xmb(); case 'xtb': - return new Xtb(this._htmlParser, interpolationConfig); + return new Xtb(); case 'xliff': case 'xlf': default: - return new Xliff(this._htmlParser, interpolationConfig); + return new Xliff(); } } } diff --git a/modules/@angular/compiler/src/i18n/i18n_parser.ts b/modules/@angular/compiler/src/i18n/i18n_parser.ts index 41a6144b68..3f2856c32f 100644 --- a/modules/@angular/compiler/src/i18n/i18n_parser.ts +++ b/modules/@angular/compiler/src/i18n/i18n_parser.ts @@ -98,7 +98,13 @@ class _I18nVisitor implements html.Visitor { this._icuDepth--; if (this._isIcu || this._icuDepth > 0) { - // If the message (vs a part of the message) is an ICU message returns it + // Returns an ICU node when: + // - the message (vs a part of the message) is an ICU message, or + // - the ICU message is nested. + const expPh = this._placeholderRegistry.getUniquePlaceholder(`VAR_${icu.type}`); + i18nIcu.expressionPlaceholder = expPh; + this._placeholderToContent[expPh] = icu.switchValue; + return i18nIcu; } diff --git a/modules/@angular/compiler/src/i18n/serializers/placeholder.ts b/modules/@angular/compiler/src/i18n/serializers/placeholder.ts index 4c418fbe52..6de2423069 100644 --- a/modules/@angular/compiler/src/i18n/serializers/placeholder.ts +++ b/modules/@angular/compiler/src/i18n/serializers/placeholder.ts @@ -95,6 +95,10 @@ export class PlaceholderRegistry { return uniqueName; } + getUniquePlaceholder(name: string): string { + return this._generateUniqueName(name.toUpperCase()); + } + // Generate a hash for a tag - does not take attribute order into account private _hashTag(tag: string, attrs: {[k: string]: string}, isVoid: boolean): string { const start = `<${tag}`; diff --git a/modules/@angular/compiler/src/i18n/serializers/serializer.ts b/modules/@angular/compiler/src/i18n/serializers/serializer.ts index 5ad5c6997c..e39d8221e9 100644 --- a/modules/@angular/compiler/src/i18n/serializers/serializer.ts +++ b/modules/@angular/compiler/src/i18n/serializers/serializer.ts @@ -6,14 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import * as html from '../../ml_parser/ast'; import * as i18n from '../i18n_ast'; -import {MessageBundle} from '../message_bundle'; export interface Serializer { write(messages: i18n.Message[]): string; - load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: html.Node[]}; + load(content: string, url: string): {[msgId: string]: i18n.Node[]}; digest(message: i18n.Message): string; } \ No newline at end of file diff --git a/modules/@angular/compiler/src/i18n/serializers/xliff.ts b/modules/@angular/compiler/src/i18n/serializers/xliff.ts index 0f5d1b004d..bbb066770e 100644 --- a/modules/@angular/compiler/src/i18n/serializers/xliff.ts +++ b/modules/@angular/compiler/src/i18n/serializers/xliff.ts @@ -7,13 +7,9 @@ */ import * as ml from '../../ml_parser/ast'; -import {HtmlParser} from '../../ml_parser/html_parser'; -import {InterpolationConfig} from '../../ml_parser/interpolation_config'; import {XmlParser} from '../../ml_parser/xml_parser'; -import {ParseError} from '../../parse_util'; import {digest} from '../digest'; import * as i18n from '../i18n_ast'; -import {MessageBundle} from '../message_bundle'; import {I18nError} from '../parse_util'; import {Serializer} from './serializer'; @@ -24,6 +20,7 @@ const _XMLNS = 'urn:oasis:names:tc:xliff:document:1.2'; // TODO(vicb): make this a param (s/_/-/) const _SOURCE_LANG = 'en'; const _PLACEHOLDER_TAG = 'x'; + const _SOURCE_TAG = 'source'; const _TARGET_TAG = 'target'; const _UNIT_TAG = 'trans-unit'; @@ -31,8 +28,6 @@ const _UNIT_TAG = 'trans-unit'; // http://docs.oasis-open.org/xliff/v1.2/os/xliff-core.html // http://docs.oasis-open.org/xliff/v1.2/xliff-profile-html/xliff-profile-html-1.2.html export class Xliff implements Serializer { - constructor(private _htmlParser: HtmlParser, private _interpolationConfig: InterpolationConfig) {} - write(messages: i18n.Message[]): string { const visitor = new _WriteVisitor(); const visited: {[id: string]: boolean} = {}; @@ -80,37 +75,25 @@ export class Xliff implements Serializer { ]); } - load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: ml.Node[]} { - // Parse the xtb file into xml nodes - const result = new XmlParser().parse(content, url); + load(content: string, url: string): {[msgId: string]: i18n.Node[]} { + // xliff to xml nodes + const xliffParser = new XliffParser(); + const {mlNodesByMsgId, errors} = xliffParser.parse(content, url); - if (result.errors.length) { - throw new Error(`xtb parse errors:\n${result.errors.join('\n')}`); - } - - // Replace the placeholders, messages are now string - const {messages, errors} = new _LoadVisitor(this).parse(result.rootNodes, messageBundle); - - if (errors.length) { - throw new Error(`xtb parse errors:\n${errors.join('\n')}`); - } - - // Convert the string messages to html ast - // TODO(vicb): map error message back to the original message in xtb - const messageMap: {[id: string]: ml.Node[]} = {}; - const parseErrors: ParseError[] = []; - - Object.keys(messages).forEach((id) => { - const res = this._htmlParser.parse(messages[id], url, true, this._interpolationConfig); - parseErrors.push(...res.errors); - messageMap[id] = res.rootNodes; + // xml nodes to i18n nodes + const i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {}; + const converter = new XmlToI18n(); + Object.keys(mlNodesByMsgId).forEach(msgId => { + const {i18nNodes, errors: e} = converter.convert(mlNodesByMsgId[msgId]); + errors.push(...e); + i18nNodesByMsgId[msgId] = i18nNodes; }); - if (parseErrors.length) { - throw new Error(`xtb parse errors:\n${parseErrors.join('\n')}`); + if (errors.length) { + throw new Error(`xliff parse errors:\n${errors.join('\n')}`); } - return messageMap; + return i18nNodesByMsgId; } digest(message: i18n.Message): string { return digest(message); } @@ -173,74 +156,42 @@ class _WriteVisitor implements i18n.Visitor { } // TODO(vicb): add error management (structure) -// TODO(vicb): factorize (xtb) ? -class _LoadVisitor implements ml.Visitor { - private _messageNodes: [string, ml.Node[]][]; - private _translatedMessages: {[id: string]: string}; - private _msgId: string; - private _target: ml.Node[]; +// Extract messages as xml nodes from the xliff file +class XliffParser implements ml.Visitor { + private _unitMlNodes: ml.Node[]; private _errors: I18nError[]; - private _sourceMessage: i18n.Message; + private _mlNodesByMsgId: {[msgId: string]: ml.Node[]}; - constructor(private _serializer: Serializer) {} + parse(xliff: string, url: string) { + this._unitMlNodes = []; + this._mlNodesByMsgId = {}; - parse(nodes: ml.Node[], messageBundle: MessageBundle): - {messages: {[k: string]: string}, errors: I18nError[]} { - this._messageNodes = []; - this._translatedMessages = {}; - this._msgId = ''; - this._target = []; - this._errors = []; + const xml = new XmlParser().parse(xliff, url, false); - // Find all messages - ml.visitAll(this, nodes, null); + this._errors = xml.errors; + ml.visitAll(this, xml.rootNodes, null); - const messageMap: {[msgId: string]: i18n.Message} = {}; - messageBundle.getMessages().forEach(m => messageMap[this._serializer.digest(m)] = m); - - this._messageNodes - .filter(message => { - // Remove any messages that is not present in the source message bundle. - return messageMap.hasOwnProperty(message[0]); - }) - .sort((a, b) => { - // Because there could be no ICU placeholdersByMsgId inside an ICU message, - // we do not need to take into account the `placeholderToMsgIds` of the referenced - // messages, those would always be empty - // TODO(vicb): overkill - create 2 buckets and [...woDeps, ...wDeps].process() - if (Object.keys(messageMap[a[0]].placeholderToMessage).length == 0) { - return -1; - } - - if (Object.keys(messageMap[b[0]].placeholderToMessage).length == 0) { - return 1; - } - - return 0; - }) - .forEach(message => { - const msgId = message[0]; - this._sourceMessage = messageMap[msgId]; - // TODO(vicb): make sure there is no `_TRANSLATIONS_TAG` nor `_TRANSLATION_TAG` - this._translatedMessages[msgId] = ml.visitAll(this, message[1]).join(''); - }); - - return {messages: this._translatedMessages, errors: this._errors}; + return { + mlNodesByMsgId: this._mlNodesByMsgId, + errors: this._errors, + }; } visitElement(element: ml.Element, context: any): any { switch (element.name) { case _UNIT_TAG: - this._target = null; - const msgId = element.attrs.find((attr) => attr.name === 'id'); - if (!msgId) { + this._unitMlNodes = null; + const idAttr = element.attrs.find((attr) => attr.name === 'id'); + if (!idAttr) { this._addError(element, `<${_UNIT_TAG}> misses the "id" attribute`); } else { - this._msgId = msgId.value; - } - ml.visitAll(this, element.children, null); - if (this._msgId !== null) { - this._messageNodes.push([this._msgId, this._target]); + const id = idAttr.value; + if (this._mlNodesByMsgId.hasOwnProperty(id)) { + this._addError(element, `Duplicated translations for msg ${id}`); + } else { + ml.visitAll(this, element.children, null); + this._mlNodesByMsgId[id] = this._unitMlNodes; + } } break; @@ -249,52 +200,65 @@ class _LoadVisitor implements ml.Visitor { break; case _TARGET_TAG: - this._target = element.children; - break; - - case _PLACEHOLDER_TAG: - const idAttr = element.attrs.find((attr) => attr.name === 'id'); - if (!idAttr) { - this._addError(element, `<${_PLACEHOLDER_TAG}> misses the "id" attribute`); - } else { - const phName = idAttr.value; - if (this._sourceMessage.placeholders.hasOwnProperty(phName)) { - return this._sourceMessage.placeholders[phName]; - } - if (this._sourceMessage.placeholderToMessage.hasOwnProperty(phName)) { - const refMsg = this._sourceMessage.placeholderToMessage[phName]; - const refMsgId = this._serializer.digest(refMsg); - if (this._translatedMessages.hasOwnProperty(refMsgId)) { - return this._translatedMessages[refMsgId]; - } - } - // TODO(vicb): better error message for when - // !this._translatedMessages.hasOwnProperty(this._placeholderToIds[id]) - this._addError( - element, `The placeholder "${phName}" does not exists in the source message`); - } + this._unitMlNodes = element.children; break; default: + // TODO(vicb): assert file structure, xliff version + // For now only recurse on unhandled nodes ml.visitAll(this, element.children, null); } } - visitAttribute(attribute: ml.Attribute, context: any): any { - throw new Error('unreachable code'); + visitAttribute(attribute: ml.Attribute, context: any): any {} + + visitText(text: ml.Text, context: any): any {} + + visitComment(comment: ml.Comment, context: any): any {} + + visitExpansion(expansion: ml.Expansion, context: any): any {} + + visitExpansionCase(expansionCase: ml.ExpansionCase, context: any): any {} + + private _addError(node: ml.Node, message: string): void { + this._errors.push(new I18nError(node.sourceSpan, message)); + } +} + +// Convert ml nodes (xliff syntax) to i18n nodes +class XmlToI18n implements ml.Visitor { + private _errors: I18nError[]; + + convert(nodes: ml.Node[]) { + this._errors = []; + return { + i18nNodes: ml.visitAll(this, nodes), + errors: this._errors, + }; } - visitText(text: ml.Text, context: any): any { return text.value; } + visitText(text: ml.Text, context: any) { return new i18n.Text(text.value, text.sourceSpan); } - visitComment(comment: ml.Comment, context: any): any { return ''; } + visitElement(el: ml.Element, context: any): i18n.Placeholder { + if (el.name === _PLACEHOLDER_TAG) { + const nameAttr = el.attrs.find((attr) => attr.name === 'id'); + if (nameAttr) { + return new i18n.Placeholder('', nameAttr.value, el.sourceSpan); + } - visitExpansion(expansion: ml.Expansion, context: any): any { - throw new Error('unreachable code'); + this._addError(el, `<${_PLACEHOLDER_TAG}> misses the "id" attribute`); + } else { + this._addError(el, `Unexpected tag`); + } } - visitExpansionCase(expansionCase: ml.ExpansionCase, context: any): any { - throw new Error('unreachable code'); - } + visitExpansion(icu: ml.Expansion, context: any) {} + + visitExpansionCase(icuCase: ml.ExpansionCase, context: any): any {} + + visitComment(comment: ml.Comment, context: any) {} + + visitAttribute(attribute: ml.Attribute, context: any) {} private _addError(node: ml.Node, message: string): void { this._errors.push(new I18nError(node.sourceSpan, message)); diff --git a/modules/@angular/compiler/src/i18n/serializers/xmb.ts b/modules/@angular/compiler/src/i18n/serializers/xmb.ts index 97b3934467..3834ee3938 100644 --- a/modules/@angular/compiler/src/i18n/serializers/xmb.ts +++ b/modules/@angular/compiler/src/i18n/serializers/xmb.ts @@ -6,10 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import * as html from '../../ml_parser/ast'; import {decimalDigest} from '../digest'; import * as i18n from '../i18n_ast'; -import {MessageBundle} from '../message_bundle'; import {Serializer} from './serializer'; import * as xml from './xml_helper'; @@ -78,7 +76,7 @@ export class Xmb implements Serializer { ]); } - load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: html.Node[]} { + load(content: string, url: string): {[msgId: string]: i18n.Node[]} { throw new Error('Unsupported'); } @@ -95,7 +93,7 @@ class _Visitor implements i18n.Visitor { } visitIcu(icu: i18n.Icu, context?: any): xml.Node[] { - const nodes = [new xml.Text(`{${icu.expression}, ${icu.type}, `)]; + const nodes = [new xml.Text(`{${icu.expressionPlaceholder}, ${icu.type}, `)]; Object.keys(icu.cases).forEach((c: string) => { nodes.push(new xml.Text(`${c} {`), ...icu.cases[c].visit(this), new xml.Text(`} `)); diff --git a/modules/@angular/compiler/src/i18n/serializers/xtb.ts b/modules/@angular/compiler/src/i18n/serializers/xtb.ts index a708fc9113..5fee8229b4 100644 --- a/modules/@angular/compiler/src/i18n/serializers/xtb.ts +++ b/modules/@angular/compiler/src/i18n/serializers/xtb.ts @@ -7,12 +7,8 @@ */ import * as ml from '../../ml_parser/ast'; -import {HtmlParser} from '../../ml_parser/html_parser'; -import {InterpolationConfig} from '../../ml_parser/interpolation_config'; import {XmlParser} from '../../ml_parser/xml_parser'; -import {ParseError} from '../../parse_util'; import * as i18n from '../i18n_ast'; -import {MessageBundle} from '../message_bundle'; import {I18nError} from '../parse_util'; import {Serializer} from './serializer'; @@ -23,102 +19,51 @@ const _TRANSLATION_TAG = 'translation'; const _PLACEHOLDER_TAG = 'ph'; export class Xtb implements Serializer { - constructor(private _htmlParser: HtmlParser, private _interpolationConfig: InterpolationConfig) {} - write(messages: i18n.Message[]): string { throw new Error('Unsupported'); } - load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: ml.Node[]} { - // Parse the xtb file into xml nodes - const result = new XmlParser().parse(content, url, true); + load(content: string, url: string): {[msgId: string]: i18n.Node[]} { + // xtb to xml nodes + const xtbParser = new XtbParser(); + const {mlNodesByMsgId, errors} = xtbParser.parse(content, url); - if (result.errors.length) { - throw new Error(`xtb parse errors:\n${result.errors.join('\n')}`); - } - - // Replace the placeholders, messages are now string - const {messages, errors} = new _Visitor(this).parse(result.rootNodes, messageBundle); + // xml nodes to i18n nodes + const i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {}; + const converter = new XmlToI18n(); + Object.keys(mlNodesByMsgId).forEach(msgId => { + const {i18nNodes, errors: e} = converter.convert(mlNodesByMsgId[msgId]); + errors.push(...e); + i18nNodesByMsgId[msgId] = i18nNodes; + }); if (errors.length) { throw new Error(`xtb parse errors:\n${errors.join('\n')}`); } - // Convert the string messages to html ast - // TODO(vicb): map error message back to the original message in xtb - const messageMap: {[id: string]: ml.Node[]} = {}; - const parseErrors: ParseError[] = []; - - Object.keys(messages).forEach((msgId) => { - const res = this._htmlParser.parse(messages[msgId], url, true, this._interpolationConfig); - parseErrors.push(...res.errors); - messageMap[msgId] = res.rootNodes; - }); - - if (parseErrors.length) { - throw new Error(`xtb parse errors:\n${parseErrors.join('\n')}`); - } - - return messageMap; + return i18nNodesByMsgId; } - digest(message: i18n.Message): string { - // we must use the same digest as xmb - return digest(message); - } + digest(message: i18n.Message): string { return digest(message); } } -class _Visitor implements ml.Visitor { - private _messageNodes: [string, ml.Node[]][]; - private _translatedMessages: {[id: string]: string}; +// Extract messages as xml nodes from the xtb file +class XtbParser implements ml.Visitor { private _bundleDepth: number; - private _translationDepth: number; private _errors: I18nError[]; - private _sourceMessage: i18n.Message; + private _mlNodesByMsgId: {[msgId: string]: ml.Node[]}; - constructor(private _serializer: Serializer) {} - - parse(nodes: ml.Node[], messageBundle: MessageBundle): - {messages: {[k: string]: string}, errors: I18nError[]} { - // Tuple [, [ml nodes]] - this._messageNodes = []; - this._translatedMessages = {}; + parse(xtb: string, url: string) { this._bundleDepth = 0; - this._translationDepth = 0; - this._errors = []; + this._mlNodesByMsgId = {}; - // load all translations - ml.visitAll(this, nodes, null); + const xml = new XmlParser().parse(xtb, url, true); - const messageMap: {[msgId: string]: i18n.Message} = {}; - messageBundle.getMessages().forEach(m => messageMap[this._serializer.digest(m)] = m); + this._errors = xml.errors; + ml.visitAll(this, xml.rootNodes); - this._messageNodes - .filter(message => { - // Remove any messages that is not present in the source message bundle. - return messageMap.hasOwnProperty(message[0]); - }) - .sort((a, b) => { - // Because there could be no ICU placeholders inside an ICU message, - // we do not need to take into account the `placeholderToMsgIds` of the referenced - // messages, those would always be empty - // TODO(vicb): overkill - create 2 buckets and [...woDeps, ...wDeps].process() - if (Object.keys(messageMap[a[0]].placeholderToMessage).length == 0) { - return -1; - } - - if (Object.keys(messageMap[b[0]].placeholderToMessage).length == 0) { - return 1; - } - - return 0; - }) - .forEach(message => { - const msgId = message[0]; - this._sourceMessage = messageMap[msgId]; - // TODO(vicb): make sure there is no `_TRANSLATIONS_TAG` nor `_TRANSLATION_TAG` - this._translatedMessages[msgId] = ml.visitAll(this, message[1]).join(''); - }); - - return {messages: this._translatedMessages, errors: this._errors}; + return { + mlNodesByMsgId: this._mlNodesByMsgId, + errors: this._errors, + }; } visitElement(element: ml.Element, context: any): any { @@ -133,43 +78,16 @@ class _Visitor implements ml.Visitor { break; case _TRANSLATION_TAG: - this._translationDepth++; - if (this._translationDepth > 1) { - this._addError(element, `<${_TRANSLATION_TAG}> elements can not be nested`); - } const idAttr = element.attrs.find((attr) => attr.name === 'id'); if (!idAttr) { this._addError(element, `<${_TRANSLATION_TAG}> misses the "id" attribute`); } else { - // ICU placeholders are reference to other messages. - // The referenced message might not have been decoded yet. - // We need to have all messages available to make sure deps are decoded first. - // TODO(vicb): report an error on duplicate id - this._messageNodes.push([idAttr.value, element.children]); - } - this._translationDepth--; - break; - - case _PLACEHOLDER_TAG: - const nameAttr = element.attrs.find((attr) => attr.name === 'name'); - if (!nameAttr) { - this._addError(element, `<${_PLACEHOLDER_TAG}> misses the "name" attribute`); - } else { - const phName = nameAttr.value; - if (this._sourceMessage.placeholders.hasOwnProperty(phName)) { - return this._sourceMessage.placeholders[phName]; + const id = idAttr.value; + if (this._mlNodesByMsgId.hasOwnProperty(id)) { + this._addError(element, `Duplicated translations for msg ${id}`); + } else { + this._mlNodesByMsgId[id] = element.children; } - if (this._sourceMessage.placeholderToMessage.hasOwnProperty(phName)) { - const refMsg = this._sourceMessage.placeholderToMessage[phName]; - const refMsgId = this._serializer.digest(refMsg); - if (this._translatedMessages.hasOwnProperty(refMsgId)) { - return this._translatedMessages[refMsgId]; - } - } - // TODO(vicb): better error message for when - // !this._translatedMessages.hasOwnProperty(refMessageId) - this._addError( - element, `The placeholder "${phName}" does not exists in the source message`); } break; @@ -178,22 +96,68 @@ class _Visitor implements ml.Visitor { } } - visitAttribute(attribute: ml.Attribute, context: any): any { - throw new Error('unreachable code'); - } + visitAttribute(attribute: ml.Attribute, context: any): any {} - visitText(text: ml.Text, context: any): any { return text.value; } + visitText(text: ml.Text, context: any): any {} - visitComment(comment: ml.Comment, context: any): any { return ''; } + visitComment(comment: ml.Comment, context: any): any {} - visitExpansion(expansion: ml.Expansion, context: any): any { - const strCases = expansion.cases.map(c => c.visit(this, null)); - return `{${expansion.switchValue}, ${expansion.type}, ${strCases.join(' ')}}`; - } + visitExpansion(expansion: ml.Expansion, context: any): any {} - visitExpansionCase(expansionCase: ml.ExpansionCase, context: any): any { - return `${expansionCase.value} {${ml.visitAll(this, expansionCase.expression, null).join('')}}`; - } + visitExpansionCase(expansionCase: ml.ExpansionCase, context: any): any {} + + private _addError(node: ml.Node, message: string): void { + this._errors.push(new I18nError(node.sourceSpan, message)); + } +} + +// Convert ml nodes (xtb syntax) to i18n nodes +class XmlToI18n implements ml.Visitor { + private _errors: I18nError[]; + + convert(nodes: ml.Node[]) { + this._errors = []; + return { + i18nNodes: ml.visitAll(this, nodes), + errors: this._errors, + }; + } + + visitText(text: ml.Text, context: any) { return new i18n.Text(text.value, text.sourceSpan); } + + visitExpansion(icu: ml.Expansion, context: any) { + const caseMap: {[value: string]: i18n.Node} = {}; + + ml.visitAll(this, icu.cases).forEach(c => { + caseMap[c.value] = new i18n.Container(c.nodes, icu.sourceSpan); + }); + + return new i18n.Icu(icu.switchValue, icu.type, caseMap, icu.sourceSpan); + } + + visitExpansionCase(icuCase: ml.ExpansionCase, context: any): any { + return { + value: icuCase.value, + nodes: ml.visitAll(this, icuCase.expression), + }; + } + + visitElement(el: ml.Element, context: any): i18n.Placeholder { + if (el.name === _PLACEHOLDER_TAG) { + const nameAttr = el.attrs.find((attr) => attr.name === 'name'); + if (nameAttr) { + return new i18n.Placeholder('', nameAttr.value, el.sourceSpan); + } + + this._addError(el, `<${_PLACEHOLDER_TAG}> misses the "name" attribute`); + } else { + this._addError(el, `Unexpected tag`); + } + } + + visitComment(comment: ml.Comment, context: any) {} + + visitAttribute(attribute: ml.Attribute, context: any) {} private _addError(node: ml.Node, message: string): void { this._errors.push(new I18nError(node.sourceSpan, message)); diff --git a/modules/@angular/compiler/src/i18n/translation_bundle.ts b/modules/@angular/compiler/src/i18n/translation_bundle.ts index b76934f166..c21cbbbb3e 100644 --- a/modules/@angular/compiler/src/i18n/translation_bundle.ts +++ b/modules/@angular/compiler/src/i18n/translation_bundle.ts @@ -7,27 +7,120 @@ */ import * as html from '../ml_parser/ast'; +import {HtmlParser} from '../ml_parser/html_parser'; -import {Message} from './i18n_ast'; -import {MessageBundle} from './message_bundle'; +import * as i18n from './i18n_ast'; +import {I18nError} from './parse_util'; import {Serializer} from './serializers/serializer'; - /** * A container for translated messages */ export class TranslationBundle { - constructor( - private _messageMap: {[id: string]: html.Node[]} = {}, - public digest: (m: Message) => string) {} + private _i18nToHtml: I18nToHtmlVisitor; - static load(content: string, url: string, messageBundle: MessageBundle, serializer: Serializer): - TranslationBundle { - return new TranslationBundle( - serializer.load(content, url, messageBundle), (m: Message) => serializer.digest(m)); + constructor( + private _i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {}, + public digest: (m: i18n.Message) => string) { + this._i18nToHtml = new I18nToHtmlVisitor(_i18nNodesByMsgId, digest); } - get(message: Message): html.Node[] { return this._messageMap[this.digest(message)]; } + static load(content: string, url: string, serializer: Serializer): TranslationBundle { + const i18nNodesByMsgId = serializer.load(content, url); + const digestFn = (m: i18n.Message) => serializer.digest(m); + return new TranslationBundle(i18nNodesByMsgId, digestFn); + } - has(message: Message): boolean { return this.digest(message) in this._messageMap; } + get(srcMsg: i18n.Message): html.Node[] { + const html = this._i18nToHtml.convert(srcMsg); + + if (html.errors.length) { + throw new Error(html.errors.join('\n')); + } + + return html.nodes; + } + + has(srcMsg: i18n.Message): boolean { return this.digest(srcMsg) in this._i18nNodesByMsgId; } +} + +class I18nToHtmlVisitor implements i18n.Visitor { + private _srcMsg: i18n.Message; + private _srcMsgStack: i18n.Message[] = []; + private _errors: I18nError[] = []; + + constructor( + private _i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {}, + private _digest: (m: i18n.Message) => string) {} + + convert(srcMsg: i18n.Message): {nodes: html.Node[], errors: I18nError[]} { + this._srcMsgStack.length = 0; + this._errors.length = 0; + // i18n to text + const text = this._convertToText(srcMsg); + + // text to html + const url = srcMsg.nodes[0].sourceSpan.start.file.url; + const html = new HtmlParser().parse(text, url, true); + + return { + nodes: html.rootNodes, + errors: [...this._errors, ...html.errors], + }; + } + + visitText(text: i18n.Text, context?: any): string { return text.value; } + + visitContainer(container: i18n.Container, context?: any): any { + return container.children.map(n => n.visit(this)).join(''); + } + + visitIcu(icu: i18n.Icu, context?: any): any { + const cases = Object.keys(icu.cases).map(k => `${k} {${icu.cases[k].visit(this)}}`); + + // TODO(vicb): Once all format switch to using expression placeholders + // we should throw when the placeholder is not in the source message + const exp = this._srcMsg.placeholders.hasOwnProperty(icu.expression) ? + this._srcMsg.placeholders[icu.expression] : + icu.expression; + + return `{${exp}, ${icu.type}, ${cases.join(' ')}}`; + } + + visitPlaceholder(ph: i18n.Placeholder, context?: any): string { + const phName = ph.name; + if (this._srcMsg.placeholders.hasOwnProperty(phName)) { + return this._srcMsg.placeholders[phName]; + } + + if (this._srcMsg.placeholderToMessage.hasOwnProperty(phName)) { + return this._convertToText(this._srcMsg.placeholderToMessage[phName]); + } + + this._addError(ph, `Unknown placeholder`); + return ''; + } + + visitTagPlaceholder(ph: i18n.TagPlaceholder, context?: any): any { throw 'unreachable code'; } + + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any { throw 'unreachable code'; } + + private _convertToText(srcMsg: i18n.Message): string { + const digest = this._digest(srcMsg); + if (this._i18nNodesByMsgId.hasOwnProperty(digest)) { + this._srcMsgStack.push(this._srcMsg); + this._srcMsg = srcMsg; + const nodes = this._i18nNodesByMsgId[digest]; + const text = nodes.map(node => node.visit(this)).join(''); + this._srcMsg = this._srcMsgStack.pop(); + return text; + } + + this._addError(srcMsg.nodes[0], `Missing translation for message ${digest}`); + return ''; + } + + private _addError(el: i18n.Node, msg: string) { + this._errors.push(new I18nError(el.sourceSpan, msg)); + } } \ No newline at end of file diff --git a/modules/@angular/compiler/src/util.ts b/modules/@angular/compiler/src/util.ts index 48fa10fb18..cfddfb1142 100644 --- a/modules/@angular/compiler/src/util.ts +++ b/modules/@angular/compiler/src/util.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {isBlank, isPrimitive, isStrictStringMap} from './facade/lang'; +import {isPrimitive, isStrictStringMap} from './facade/lang'; export const MODULE_SUFFIX = ''; @@ -48,7 +48,7 @@ export function visitValue(value: any, visitor: ValueVisitor, context: any): any return visitor.visitStringMap(<{[key: string]: any}>value, context); } - if (isBlank(value) || isPrimitive(value)) { + if (value == null || isPrimitive(value)) { return visitor.visitPrimitive(value, context); } diff --git a/modules/@angular/compiler/test/i18n/digest_spec.ts b/modules/@angular/compiler/test/i18n/digest_spec.ts index bd5e579632..3fdc817e59 100644 --- a/modules/@angular/compiler/test/i18n/digest_spec.ts +++ b/modules/@angular/compiler/test/i18n/digest_spec.ts @@ -6,8 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {describe, expect, it} from '@angular/core/testing/testing_internal'; - import {fingerprint, sha1} from '../../src/i18n/digest'; export function main(): void { diff --git a/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts b/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts index acda260f59..c175425507 100644 --- a/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts +++ b/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts @@ -7,7 +7,6 @@ */ import {DEFAULT_INTERPOLATION_CONFIG, HtmlParser} from '@angular/compiler'; -import {describe, expect, it} from '@angular/core/testing/testing_internal'; import {digest, serializeNodes as serializeI18nNodes} from '../../src/i18n/digest'; import {extractMessages, mergeTranslations} from '../../src/i18n/extractor_merger'; @@ -93,9 +92,10 @@ export function main() { ], [ [ - 'text', - 'html, nested', - '{count, plural, =0 {[html]}}', + 'text', 'html, nested', + '{count, plural, =0 {[html]}}', '[interp]' ], '', '' @@ -189,9 +189,8 @@ export function main() { it('should extract from attributes in translatable elements', () => { expect(extract('

')).toEqual([ [ - [ - '' - ], + [''], '', '' ], [['msg'], 'm', 'd'], @@ -203,9 +202,8 @@ export function main() { .toEqual([ [['msg'], 'm', 'd'], [ - [ - '' - ], + [''], '', '' ], ]); @@ -219,7 +217,8 @@ export function main() { [['msg'], 'm', 'd'], [ [ - '{count, plural, =0 {[]}}' + '{count, plural, =0 {[]}}' ], '', '' ], @@ -350,7 +349,9 @@ export function main() { const HTML = `before

foo

barafter`; expect(fakeTranslate(HTML)) .toEqual( - 'before**foobar**after'); + 'before**[ph tag name="START_PARAGRAPH">foo[/ph name="CLOSE_PARAGRAPH">[ph tag' + + ' name="START_TAG_SPAN">[ph tag name="START_ITALIC_TEXT">bar[/ph' + + ' name="CLOSE_ITALIC_TEXT">[/ph name="CLOSE_TAG_SPAN">**after'); }); it('should merge nested blocks', () => { @@ -358,7 +359,9 @@ export function main() { `
before

foo

barafter
`; expect(fakeTranslate(HTML)) .toEqual( - '
before**foobar**after
'); + '
before**[ph tag name="START_PARAGRAPH">foo[/ph name="CLOSE_PARAGRAPH">[ph' + + ' tag name="START_TAG_SPAN">[ph tag name="START_ITALIC_TEXT">bar[/ph' + + ' name="CLOSE_ITALIC_TEXT">[/ph name="CLOSE_TAG_SPAN">**after
'); }); }); @@ -399,12 +402,12 @@ function fakeTranslate( extractMessages(htmlNodes, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs) .messages; - const i18nMsgMap: {[id: string]: html.Node[]} = {}; + const i18nMsgMap: {[id: string]: i18n.Node[]} = {}; messages.forEach(message => { const id = digest(message); - const text = serializeI18nNodes(message.nodes).join(''); - i18nMsgMap[id] = [new html.Text(`**${text}**`, null)]; + const text = serializeI18nNodes(message.nodes).join('').replace(/

imbriqué

'); expectHtml(el, '#i18n-3') .toBe('

avec des espaces réservés

'); + expectHtml(el, '#i18n-3b') + .toBe( + '

avec des espaces réservés

'); expectHtml(el, '#i18n-4') .toBe('

'); expectHtml(el, '#i18n-5').toBe('

'); @@ -66,8 +69,10 @@ export function main() { expect(el.query(By.css('#i18n-14')).nativeElement).toHaveText('beaucoup'); cmp.sex = 'm'; + cmp.sexB = 'f'; tb.detectChanges(); expect(el.query(By.css('#i18n-8')).nativeElement).toHaveText('homme'); + expect(el.query(By.css('#i18n-8b')).nativeElement).toHaveText('femme'); cmp.sex = 'f'; tb.detectChanges(); expect(el.query(By.css('#i18n-8')).nativeElement).toHaveText('femme'); @@ -106,6 +111,7 @@ function expectHtml(el: DebugElement, cssSelector: string): any {

nested

with placeholders

+

with placeholders

@@ -119,6 +125,9 @@ function expectHtml(el: DebugElement, cssSelector: string): any {
{sex, select, m {male} f {female}}
+
+ {sexB, select, m {male} f {female}} +
{{ "count = " + count }}
sex = {{ sex }}
@@ -135,8 +144,9 @@ function expectHtml(el: DebugElement, cssSelector: string): any { ` }) class I18nComponent { - count: number = 0; - sex: string = 'm'; + count: number; + sex: string; + sexB: string; } class FrLocalization extends NgLocalization { @@ -159,14 +169,14 @@ const XTB = ` avec des espaces réservés sur des balises non traductibles sur des balises traductibles - {count, plural, =0 {zero} =1 {un} =2 {deux} other {beaucoup}} - - {sex, select, m {homme} f {femme}} + {VAR_PLURAL, plural, =0 {zero} =1 {un} =2 {deux} other {beaucoup}} + + {VAR_SELECT, select, m {homme} f {femme}} sexe = dans une section traductible - + Balises dans les commentaires html @@ -185,16 +195,16 @@ const XMB = ` <i>with placeholders</i> on not translatable node on translatable node - {count, plural, =0 {zero} =1 {one} =2 {two} other {<b>many</b>} } - + {VAR_PLURAL, plural, =0 {zero} =1 {one} =2 {two} other {<b>many</b>} } + - {sex, select, m {male} f {female} } + {VAR_SELECT, select, m {male} f {female} } sex = in a translatable section - + <h1>Markers in html comments</h1> <div></div> <div></div> diff --git a/modules/@angular/compiler/test/i18n/message_bundle_spec.ts b/modules/@angular/compiler/test/i18n/message_bundle_spec.ts index ed1692094f..b82a30322b 100644 --- a/modules/@angular/compiler/test/i18n/message_bundle_spec.ts +++ b/modules/@angular/compiler/test/i18n/message_bundle_spec.ts @@ -6,12 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import * as i18n from '@angular/compiler/src/i18n/i18n_ast'; -import {Serializer} from '@angular/compiler/src/i18n/serializers/serializer'; -import {beforeEach, describe, expect, it} from '@angular/core/testing/testing_internal'; - import {serializeNodes} from '../../src/i18n/digest'; +import * as i18n from '../../src/i18n/i18n_ast'; import {MessageBundle} from '../../src/i18n/message_bundle'; +import {Serializer} from '../../src/i18n/serializers/serializer'; import {HtmlParser} from '../../src/ml_parser/html_parser'; import {DEFAULT_INTERPOLATION_CONFIG} from '../../src/ml_parser/interpolation_config'; @@ -50,7 +48,7 @@ class _TestSerializer implements Serializer { .join('//'); } - load(content: string, url: string, placeholders: {}): {} { return null; } + load(content: string, url: string): {} { return null; } digest(msg: i18n.Message): string { return 'unused'; } } diff --git a/modules/@angular/compiler/test/i18n/serializers/placeholder_spec.ts b/modules/@angular/compiler/test/i18n/serializers/placeholder_spec.ts index ce6e3b1811..eb068695c0 100644 --- a/modules/@angular/compiler/test/i18n/serializers/placeholder_spec.ts +++ b/modules/@angular/compiler/test/i18n/serializers/placeholder_spec.ts @@ -6,8 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {beforeEach, describe, expect, it} from '@angular/core/testing/testing_internal'; - import {PlaceholderRegistry} from '../../../src/i18n/serializers/placeholder'; export function main(): void { diff --git a/modules/@angular/compiler/test/i18n/serializers/xliff_spec.ts b/modules/@angular/compiler/test/i18n/serializers/xliff_spec.ts index 43639813c0..2dcadf090d 100644 --- a/modules/@angular/compiler/test/i18n/serializers/xliff_spec.ts +++ b/modules/@angular/compiler/test/i18n/serializers/xliff_spec.ts @@ -6,12 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {Xliff} from '@angular/compiler/src/i18n/serializers/xliff'; -import {beforeEach, describe, expect, it} from '@angular/core/testing/testing_internal'; +import {escapeRegExp} from '@angular/core/src/facade/lang'; + +import {serializeNodes} from '../../../src/i18n/digest'; import {MessageBundle} from '../../../src/i18n/message_bundle'; +import {Xliff} from '../../../src/i18n/serializers/xliff'; import {HtmlParser} from '../../../src/ml_parser/html_parser'; import {DEFAULT_INTERPOLATION_CONFIG} from '../../../src/ml_parser/interpolation_config'; -import {serializeNodes} from '../../ml_parser/ast_serializer_spec'; const HTML = `

not translatable

@@ -77,8 +78,7 @@ const LOAD_XLIFF = ` `; export function main(): void { - let serializer: Xliff; - let htmlParser: HtmlParser; + const serializer = new Xliff(); function toXliff(html: string): string { const catalog = new MessageBundle(new HtmlParser, [], {}); @@ -86,37 +86,89 @@ export function main(): void { return catalog.write(serializer); } - function loadAsText(template: string, xliff: string): {[id: string]: string} { - const messageBundle = new MessageBundle(htmlParser, [], {}); - messageBundle.updateFromTemplate(template, 'url', DEFAULT_INTERPOLATION_CONFIG); + function loadAsMap(xliff: string): {[id: string]: string} { + const i18nNodesByMsgId = serializer.load(xliff, 'url'); + const msgMap: {[id: string]: string} = {}; + Object.keys(i18nNodesByMsgId) + .forEach(id => msgMap[id] = serializeNodes(i18nNodesByMsgId[id]).join('')); - const asAst = serializer.load(xliff, 'url', messageBundle); - const asText: {[id: string]: string} = {}; - Object.keys(asAst).forEach(id => { asText[id] = serializeNodes(asAst[id]).join(''); }); - - return asText; + return msgMap; } describe('XLIFF serializer', () => { - - beforeEach(() => { - htmlParser = new HtmlParser(); - serializer = new Xliff(htmlParser, DEFAULT_INTERPOLATION_CONFIG); - }); - - describe('write', () => { it('should write a valid xliff file', () => { expect(toXliff(HTML)).toEqual(WRITE_XLIFF); }); }); describe('load', () => { it('should load XLIFF files', () => { - expect(loadAsText(HTML, LOAD_XLIFF)).toEqual({ + expect(loadAsMap(LOAD_XLIFF)).toEqual({ '983775b9a51ce14b036be72d4cfd65d68d64e231': 'etubirtta elbatalsnart', 'ec1d033f2436133c14ab038286c4f5df4697484a': - '{{ interpolation}} footnemele elbatalsnart sredlohecalp htiw', + ' footnemele elbatalsnart sredlohecalp htiw', 'db3e0a6a5a96481f60aec61d98c3eecddef5ac23': 'oof', - 'd7fa2d59aaedcaa5309f13028c59af8c85b8c49d': '

', + 'd7fa2d59aaedcaa5309f13028c59af8c85b8c49d': + '', + }); + }); + + describe('errors', () => { + it('should throw when a placeholder has no id attribute', () => { + const XLIFF = ` + + + + + + + + + +`; + + expect(() => { + loadAsMap(XLIFF); + }).toThrowError(/ misses the "id" attribute/); + }); + + it('should throw on unknown message tags', () => { + const XLIFF = ` + + + + + + msg should contain only ph tags + + + +`; + + expect(() => { loadAsMap(XLIFF); }) + .toThrowError( + new RegExp(escapeRegExp(`[ERROR ->]msg should contain only ph tags`))); + }); + + it('should throw when a placeholder has no name attribute', () => { + const XLIFF = ` + + + + + + + + + + + + + +`; + + expect(() => { + loadAsMap(XLIFF); + }).toThrowError(/Duplicated translations for msg deadbeef/); }); }); }); diff --git a/modules/@angular/compiler/test/i18n/serializers/xmb_spec.ts b/modules/@angular/compiler/test/i18n/serializers/xmb_spec.ts index 315a451c05..0b00577c0a 100644 --- a/modules/@angular/compiler/test/i18n/serializers/xmb_spec.ts +++ b/modules/@angular/compiler/test/i18n/serializers/xmb_spec.ts @@ -44,9 +44,9 @@ export function main(): void { ]> translatable element <b>with placeholders</b> - { count, plural, =0 {<p>test</p>} } + {VAR_PLURAL, plural, =0 {<p>test</p>} } foo - { count, plural, =0 {{ sex, gender, other {<p>deeply nested</p>} } } } + {VAR_PLURAL, plural, =0 {{VAR_GENDER, gender, other {<p>deeply nested</p>} } } } `; @@ -55,7 +55,7 @@ export function main(): void { it('should throw when trying to load an xmb file', () => { expect(() => { const serializer = new Xmb(); - serializer.load(XMB, 'url', null); + serializer.load(XMB, 'url'); }).toThrowError(/Unsupported/); }); }); diff --git a/modules/@angular/compiler/test/i18n/serializers/xml_helper_spec.ts b/modules/@angular/compiler/test/i18n/serializers/xml_helper_spec.ts index 8bcd7e3d30..e1f9215b4d 100644 --- a/modules/@angular/compiler/test/i18n/serializers/xml_helper_spec.ts +++ b/modules/@angular/compiler/test/i18n/serializers/xml_helper_spec.ts @@ -6,8 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {describe, expect, it} from '@angular/core/testing/testing_internal'; - import * as xml from '../../../src/i18n/serializers/xml_helper'; export function main(): void { diff --git a/modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts b/modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts index 2b8d0828d4..57c9c52693 100644 --- a/modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts +++ b/modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts @@ -8,37 +8,24 @@ import {escapeRegExp} from '@angular/core/src/facade/lang'; -import {MessageBundle} from '../../../src/i18n/message_bundle'; +import {serializeNodes} from '../../../src/i18n/digest'; import {Xtb} from '../../../src/i18n/serializers/xtb'; -import {HtmlParser} from '../../../src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '../../../src/ml_parser/interpolation_config'; -import {serializeNodes} from '../../ml_parser/ast_serializer_spec'; export function main(): void { describe('XTB serializer', () => { - let serializer: Xtb; - let htmlParser: HtmlParser; + const serializer = new Xtb(); - function loadAsText(template: string, xtb: string): {[id: string]: string} { - const messageBundle = new MessageBundle(htmlParser, [], {}); - messageBundle.updateFromTemplate(template, 'url', DEFAULT_INTERPOLATION_CONFIG); - - const asAst = serializer.load(xtb, 'url', messageBundle); - const asText: {[id: string]: string} = {}; - Object.keys(asAst).forEach(id => { asText[id] = serializeNodes(asAst[id]).join(''); }); - - return asText; + function loadAsMap(xtb: string): {[id: string]: string} { + const i18nNodesByMsgId = serializer.load(xtb, 'url'); + const msgMap: {[id: string]: string} = {}; + Object.keys(i18nNodesByMsgId).forEach(id => { + msgMap[id] = serializeNodes(i18nNodesByMsgId[id]).join(''); + }); + return msgMap; } - beforeEach(() => { - htmlParser = new HtmlParser(); - serializer = new Xtb(htmlParser, DEFAULT_INTERPOLATION_CONFIG); - }); - describe('load', () => { it('should load XTB files with a doctype', () => { - const HTML = `
bar
`; - const XTB = ` @@ -53,67 +40,61 @@ export function main(): void { rab `; - expect(loadAsText(HTML, XTB)).toEqual({'8841459487341224498': 'rab'}); + expect(loadAsMap(XTB)).toEqual({'8841459487341224498': 'rab'}); }); it('should load XTB files without placeholders', () => { - const HTML = `
bar
`; - const XTB = ` rab `; - expect(loadAsText(HTML, XTB)).toEqual({'8841459487341224498': 'rab'}); + expect(loadAsMap(XTB)).toEqual({'8841459487341224498': 'rab'}); }); it('should load XTB files with placeholders', () => { - const HTML = `

bar

`; - const XTB = ` rab `; - expect(loadAsText(HTML, XTB)).toEqual({'8877975308926375834': '

rab

'}); + expect(loadAsMap(XTB)).toEqual({ + '8877975308926375834': 'rab' + }); }); it('should replace ICU placeholders with their translations', () => { - const HTML = `
-{ count, plural, =0 {

bar

}}-
`; - const XTB = ` - ** - { count, plural, =1 {rab}} + ** + {VAR_PLURAL, plural, =1 {rab}} `; - expect(loadAsText(HTML, XTB)).toEqual({ - '1430521728694081603': `*{ count, plural, =1 {

rab

}}*`, - '4004755025589356097': `{ count, plural, =1 {

rab

}}`, + expect(loadAsMap(XTB)).toEqual({ + '7717087045075616176': `**`, + '5115002811911870583': + `{VAR_PLURAL, plural, =1 {[, rab, ]}}`, }); }); it('should load complex XTB files', () => { - const HTML = ` -
foo bar {{ a + b }}
-
{ count, plural, =0 {

bar

}}
-
foo
-
{ count, plural, =0 {{ sex, select, other {

bar

}} }}
`; - const XTB = ` rab oof - { count, plural, =1 {rab}} + {VAR_PLURAL, plural, =1 {rab}} oof - { count, plural, =1 {{ sex, gender, male {rab}} }} + {VAR_PLURAL, plural, =1 {{VAR_GENDER, gender, male {rab}} }} `; - expect(loadAsText(HTML, XTB)).toEqual({ - '8281795707202401639': `{{ a + b }}rab oof`, - '4004755025589356097': `{ count, plural, =1 {

rab

}}`, + expect(loadAsMap(XTB)).toEqual({ + '8281795707202401639': + `rab oof`, + '5115002811911870583': + `{VAR_PLURAL, plural, =1 {[, rab, ]}}`, '130772889486467622': `oof`, - '4244993204427636474': `{ count, plural, =1 {{ sex, gender, male {

rab

}} }}`, + '4739316421648347533': + `{VAR_PLURAL, plural, =1 {[{VAR_GENDER, gender, male {[, rab, ]}}, ]}}`, }); }); }); @@ -124,7 +105,7 @@ export function main(): void { ''; expect(() => { - loadAsText('', XTB); + loadAsMap(XTB); }).toThrowError(/ elements can not be nested/); }); @@ -133,58 +114,49 @@ export function main(): void { `; - expect(() => { - loadAsText('', XTB); - }).toThrowError(/ misses the "id" attribute/); + expect(() => { loadAsMap(XTB); }).toThrowError(/ misses the "id" attribute/); }); it('should throw when a placeholder has no name attribute', () => { - const HTML = '
give me a message
'; - const XTB = ` `; - expect(() => { loadAsText(HTML, XTB); }).toThrowError(/ misses the "name" attribute/); + expect(() => { loadAsMap(XTB); }).toThrowError(/ misses the "name" attribute/); }); - it('should throw when a placeholder is not present in the source message', () => { - const HTML = `
bar
`; + it('should throw on unknown xtb tags', () => { + const XTB = ``; - const XTB = ` - - + expect(() => { + loadAsMap(XTB); + }).toThrowError(new RegExp(escapeRegExp(`Unexpected tag ("[ERROR ->]")`))); + }); + + it('should throw on unknown message tags', () => { + const XTB = ` + msg should contain only ph tags +`; + + expect(() => { loadAsMap(XTB); }) + .toThrowError( + new RegExp(escapeRegExp(`[ERROR ->]msg should contain only ph tags`))); + }); + + it('should throw on duplicate message id', () => { + const XTB = ` + msg1 + msg2 `; expect(() => { - loadAsText(HTML, XTB); - }).toThrowError(/The placeholder "UNKNOWN" does not exists in the source message/); + loadAsMap(XTB); + }).toThrowError(/Duplicated translations for msg 1186013544048295927/); }); - }); - it('should throw when the translation results in invalid html', () => { - const HTML = `

bar

`; - - const XTB = ` - - rab -`; - - expect(() => { - loadAsText(HTML, XTB); - }).toThrowError(/xtb parse errors:\nUnexpected closing tag "p"/); + it('should throw when trying to save an xtb file', + () => { expect(() => { serializer.write([]); }).toThrowError(/Unsupported/); }); }); - - it('should throw on unknown tags', () => { - const XTB = ``; - - expect(() => { - loadAsText('', XTB); - }).toThrowError(new RegExp(escapeRegExp(`Unexpected tag ("[ERROR ->]")`))); - }); - - it('should throw when trying to save an xtb file', - () => { expect(() => { serializer.write([]); }).toThrowError(/Unsupported/); }); }); } \ No newline at end of file diff --git a/modules/@angular/compiler/test/i18n/translation_bundle_spec.ts b/modules/@angular/compiler/test/i18n/translation_bundle_spec.ts new file mode 100644 index 0000000000..d6f51e331d --- /dev/null +++ b/modules/@angular/compiler/test/i18n/translation_bundle_spec.ts @@ -0,0 +1,110 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as i18n from '../../src/i18n/i18n_ast'; +import {TranslationBundle} from '../../src/i18n/translation_bundle'; +import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_util'; +import {serializeNodes} from '../ml_parser/ast_serializer_spec'; + +export function main(): void { + describe('TranslationBundle', () => { + const file = new ParseSourceFile('content', 'url'); + const location = new ParseLocation(file, 0, 0, 0); + const span = new ParseSourceSpan(location, null); + const srcNode = new i18n.Text('src', span); + + it('should translate a plain message', () => { + const msgMap = {foo: [new i18n.Text('bar', null)]}; + const tb = new TranslationBundle(msgMap, (_) => 'foo'); + const msg = new i18n.Message([srcNode], {}, {}, 'm', 'd'); + expect(serializeNodes(tb.get(msg))).toEqual(['bar']); + }); + + it('should translate a message with placeholder', () => { + const msgMap = { + foo: [ + new i18n.Text('bar', null), + new i18n.Placeholder('', 'ph1', null), + ] + }; + const phMap = { + ph1: '*phContent*', + }; + const tb = new TranslationBundle(msgMap, (_) => 'foo'); + const msg = new i18n.Message([srcNode], phMap, {}, 'm', 'd'); + expect(serializeNodes(tb.get(msg))).toEqual(['bar*phContent*']); + }); + + it('should translate a message with placeholder referencing messages', () => { + const msgMap = { + foo: [ + new i18n.Text('--', null), + new i18n.Placeholder('', 'ph1', null), + new i18n.Text('++', null), + ], + ref: [ + new i18n.Text('*refMsg*', null), + ], + }; + const refMsg = new i18n.Message([srcNode], {}, {}, 'm', 'd'); + const msg = new i18n.Message([srcNode], {}, {ph1: refMsg}, 'm', 'd'); + let count = 0; + const digest = (_: any) => count++ ? 'ref' : 'foo'; + const tb = new TranslationBundle(msgMap, digest); + + expect(serializeNodes(tb.get(msg))).toEqual(['--*refMsg*++']); + }); + + describe('errors', () => { + it('should report unknown placeholders', () => { + const msgMap = { + foo: [ + new i18n.Text('bar', null), + new i18n.Placeholder('', 'ph1', span), + ] + }; + const tb = new TranslationBundle(msgMap, (_) => 'foo'); + const msg = new i18n.Message([srcNode], {}, {}, 'm', 'd'); + expect(() => tb.get(msg)).toThrowError(/Unknown placeholder/); + }); + + it('should report missing translation', () => { + const tb = new TranslationBundle({}, (_) => 'foo'); + const msg = new i18n.Message([srcNode], {}, {}, 'm', 'd'); + expect(() => tb.get(msg)).toThrowError(/Missing translation for message foo/); + }); + + it('should report missing referenced message', () => { + const msgMap = { + foo: [new i18n.Placeholder('', 'ph1', span)], + }; + const refMsg = new i18n.Message([srcNode], {}, {}, 'm', 'd'); + const msg = new i18n.Message([srcNode], {}, {ph1: refMsg}, 'm', 'd'); + let count = 0; + const digest = (_: any) => count++ ? 'ref' : 'foo'; + const tb = new TranslationBundle(msgMap, digest); + expect(() => tb.get(msg)).toThrowError(/Missing translation for message ref/); + }); + + it('should report invalid translated html', () => { + const msgMap = { + foo: [ + new i18n.Text('text', null), + new i18n.Placeholder('', 'ph1', null), + ] + }; + const phMap = { + ph1: '', + }; + const tb = new TranslationBundle(msgMap, (_) => 'foo'); + const msg = new i18n.Message([srcNode], phMap, {}, 'm', 'd'); + expect(() => tb.get(msg)).toThrowError(/Unexpected closing tag "b"/); + }); + }); + }); +} diff --git a/modules/@angular/compiler/test/ml_parser/ast_serializer_spec.ts b/modules/@angular/compiler/test/ml_parser/ast_serializer_spec.ts index 5e33a302d5..9452c3c9fb 100644 --- a/modules/@angular/compiler/test/ml_parser/ast_serializer_spec.ts +++ b/modules/@angular/compiler/test/ml_parser/ast_serializer_spec.ts @@ -97,4 +97,4 @@ const serializerVisitor = new _SerializerVisitor(); export function serializeNodes(nodes: html.Node[]): string[] { return nodes.map(node => node.visit(serializerVisitor, null)); -} +} \ No newline at end of file