refactor(compiler): [i18n] move dedup and placeholder mapping to the `MessageBundle`

It makes implementing a `Serializer` simpler as implementations do not have to
care any more about message dedup and placeholder mapping.

modules/@angular/compiler/src/i18n/message_bundle.ts

@@ -11,14 +11,15 @@ import {InterpolationConfig} from '../ml_parser/interpolation_config';
 import {ParseError} from '../parse_util';
 
 import {extractMessages} from './extractor_merger';
-import {Message} from './i18n_ast';
+import * as i18n from './i18n_ast';
-import {Serializer} from './serializers/serializer';
+import {PlaceholderMapper, Serializer} from './serializers/serializer';
+
 
 /**
  * A container for message extracted from the templates.
  */
 export class MessageBundle {
-  private _messages: Message[] = [];
+  private _messages: i18n.Message[] = [];
 
   constructor(
       private _htmlParser: HtmlParser, private _implicitTags: string[],
@@ -42,7 +43,70 @@ export class MessageBundle {
     this._messages.push(...i18nParserResult.messages);
   }
 
-  getMessages(): Message[] { return this._messages; }
+  // Return the message in the internal format
+  // The public (serialized) format might be different, see the `write` method.
+  getMessages(): i18n.Message[] { return this._messages; }
 
-  write(serializer: Serializer): string { return serializer.write(this._messages); }
+  write(serializer: Serializer): string {
+    const messages: {[id: string]: i18n.Message} = {};
+    const mapperVisitor = new MapPlaceholderNames();
+
+    // Deduplicate messages based on their ID
+    this._messages.forEach(message => {
+      const id = serializer.digest(message);
+      if (!messages.hasOwnProperty(id)) {
+        messages[id] = message;
+      }
+    });
+
+    // Transform placeholder names using the serializer mapping
+    const msgList = Object.keys(messages).map(id => {
+      const mapper = serializer.createNameMapper(messages[id]);
+      const src = messages[id];
+      const nodes = mapper ? mapperVisitor.convert(src.nodes, mapper) : src.nodes;
+      return new i18n.Message(nodes, {}, {}, src.meaning, src.description, id);
+    });
+
+    return serializer.write(msgList);
+  }
+}
+
+// Transform an i18n AST by renaming the placeholder nodes with the given mapper
+class MapPlaceholderNames implements i18n.Visitor {
+  convert(nodes: i18n.Node[], mapper: PlaceholderMapper): i18n.Node[] {
+    return mapper ? nodes.map(n => n.visit(this, mapper)) : nodes;
+  }
+
+  visitText(text: i18n.Text, mapper: PlaceholderMapper): i18n.Text {
+    return new i18n.Text(text.value, text.sourceSpan);
+  }
+
+  visitContainer(container: i18n.Container, mapper: PlaceholderMapper): i18n.Container {
+    const children = container.children.map(n => n.visit(this, mapper));
+    return new i18n.Container(children, container.sourceSpan);
+  }
+
+  visitIcu(icu: i18n.Icu, mapper: PlaceholderMapper): i18n.Icu {
+    const cases: {[k: string]: i18n.Node} = {};
+    Object.keys(icu.cases).forEach(key => cases[key] = icu.cases[key].visit(this, mapper));
+    const msg = new i18n.Icu(icu.expression, icu.type, cases, icu.sourceSpan);
+    msg.expressionPlaceholder = icu.expressionPlaceholder;
+    return msg;
+  }
+
+  visitTagPlaceholder(ph: i18n.TagPlaceholder, mapper: PlaceholderMapper): i18n.TagPlaceholder {
+    const startName = mapper.toPublicName(ph.startName);
+    const closeName = ph.closeName ? mapper.toPublicName(ph.closeName) : ph.closeName;
+    const children = ph.children.map(n => n.visit(this, mapper));
+    return new i18n.TagPlaceholder(
+        ph.tag, ph.attrs, startName, closeName, children, ph.isVoid, ph.sourceSpan);
+  }
+
+  visitPlaceholder(ph: i18n.Placeholder, mapper: PlaceholderMapper): i18n.Placeholder {
+    return new i18n.Placeholder(ph.value, mapper.toPublicName(ph.name), ph.sourceSpan);
+  }
+
+  visitIcuPlaceholder(ph: i18n.IcuPlaceholder, mapper: PlaceholderMapper): i18n.IcuPlaceholder {
+    return new i18n.IcuPlaceholder(ph.value, mapper.toPublicName(ph.name), ph.sourceSpan);
+  }
 }

modules/@angular/compiler/src/i18n/serializers/serializer.ts

@@ -9,6 +9,9 @@
 import * as i18n from '../i18n_ast';
 
 export abstract class Serializer {
+  // - The `placeholders` and `placeholderToMessage` properties are irrelevant in the input messages
+  // - The `id` contains the message id that the serializer is expected to use
+  // - Placeholder names are already map to public names using the provided mapper
   abstract write(messages: i18n.Message[]): string;
 
   abstract load(content: string, url: string): {[msgId: string]: i18n.Node[]};

modules/@angular/compiler/src/i18n/serializers/xliff.ts

@@ -30,17 +30,10 @@ const _UNIT_TAG = 'trans-unit';
 export class Xliff extends Serializer {
   write(messages: i18n.Message[]): string {
     const visitor = new _WriteVisitor();
-    const visited: {[id: string]: boolean} = {};
     const transUnits: xml.Node[] = [];
 
     messages.forEach(message => {
-      const id = this.digest(message);
+      const transUnit = new xml.Tag(_UNIT_TAG, {id: message.id, datatype: 'html'});
-
-      // deduplicate messages
-      if (visited[id]) return;
-      visited[id] = true;
-
-      const transUnit = new xml.Tag(_UNIT_TAG, {id, datatype: 'html'});
       transUnit.children.push(
           new xml.CR(8), new xml.Tag(_SOURCE_TAG, {}, visitor.serialize(message.nodes)),
           new xml.CR(8), new xml.Tag(_TARGET_TAG));

modules/@angular/compiler/src/i18n/serializers/xmb.ts

@@ -41,19 +41,10 @@ export class Xmb extends Serializer {
   write(messages: i18n.Message[]): string {
     const exampleVisitor = new ExampleVisitor();
     const visitor = new _Visitor();
-    const visited: {[id: string]: boolean} = {};
     let rootNode = new xml.Tag(_MESSAGES_TAG);
 
     messages.forEach(message => {
-      const id = this.digest(message);
+      const attrs: {[k: string]: string} = {id: message.id};
-
-      // deduplicate messages
-      if (visited[id]) return;
-      visited[id] = true;
-
-      const mapper = this.createNameMapper(message);
-
-      const attrs: {[k: string]: string} = {id};
 
       if (message.description) {
         attrs['desc'] = message.description;
@@ -64,8 +55,7 @@ export class Xmb extends Serializer {
       }
 
       rootNode.children.push(
-          new xml.CR(2),
+          new xml.CR(2), new xml.Tag(_MESSAGE_TAG, attrs, visitor.serialize(message.nodes)));
-          new xml.Tag(_MESSAGE_TAG, attrs, visitor.serialize(message.nodes, {mapper})));
     });
 
     rootNode.children.push(new xml.CR());
@@ -93,21 +83,19 @@ export class Xmb extends Serializer {
 }
 
 class _Visitor implements i18n.Visitor {
-  visitText(text: i18n.Text, ctx: {mapper: PlaceholderMapper}): xml.Node[] {
+  visitText(text: i18n.Text, context?: any): xml.Node[] { return [new xml.Text(text.value)]; }
-    return [new xml.Text(text.value)];
-  }
 
-  visitContainer(container: i18n.Container, ctx: any): xml.Node[] {
+  visitContainer(container: i18n.Container, context: any): xml.Node[] {
     const nodes: xml.Node[] = [];
-    container.children.forEach((node: i18n.Node) => nodes.push(...node.visit(this, ctx)));
+    container.children.forEach((node: i18n.Node) => nodes.push(...node.visit(this)));
     return nodes;
   }
 
-  visitIcu(icu: i18n.Icu, ctx: {mapper: PlaceholderMapper}): xml.Node[] {
+  visitIcu(icu: i18n.Icu, context?: any): xml.Node[] {
     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, ctx), new xml.Text(`} `));
+      nodes.push(new xml.Text(`${c} {`), ...icu.cases[c].visit(this), new xml.Text(`} `));
     });
 
     nodes.push(new xml.Text(`}`));
@@ -115,34 +103,30 @@ class _Visitor implements i18n.Visitor {
     return nodes;
   }
 
-  visitTagPlaceholder(ph: i18n.TagPlaceholder, ctx: {mapper: PlaceholderMapper}): xml.Node[] {
+  visitTagPlaceholder(ph: i18n.TagPlaceholder, context?: any): xml.Node[] {
     const startEx = new xml.Tag(_EXEMPLE_TAG, {}, [new xml.Text(`<${ph.tag}>`)]);
-    let name = ctx.mapper.toPublicName(ph.startName);
+    const startTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name: ph.startName}, [startEx]);
-    const startTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name}, [startEx]);
     if (ph.isVoid) {
       // void tags have no children nor closing tags
       return [startTagPh];
     }
 
     const closeEx = new xml.Tag(_EXEMPLE_TAG, {}, [new xml.Text(`</${ph.tag}>`)]);
-    name = ctx.mapper.toPublicName(ph.closeName);
+    const closeTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name: ph.closeName}, [closeEx]);
-    const closeTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name}, [closeEx]);
 
-    return [startTagPh, ...this.serialize(ph.children, ctx), closeTagPh];
+    return [startTagPh, ...this.serialize(ph.children), closeTagPh];
   }
 
-  visitPlaceholder(ph: i18n.Placeholder, ctx: {mapper: PlaceholderMapper}): xml.Node[] {
+  visitPlaceholder(ph: i18n.Placeholder, context?: any): xml.Node[] {
-    const name = ctx.mapper.toPublicName(ph.name);
+    return [new xml.Tag(_PLACEHOLDER_TAG, {name: ph.name})];
-    return [new xml.Tag(_PLACEHOLDER_TAG, {name})];
   }
 
-  visitIcuPlaceholder(ph: i18n.IcuPlaceholder, ctx: {mapper: PlaceholderMapper}): xml.Node[] {
+  visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): xml.Node[] {
-    const name = ctx.mapper.toPublicName(ph.name);
+    return [new xml.Tag(_PLACEHOLDER_TAG, {name: ph.name})];
-    return [new xml.Tag(_PLACEHOLDER_TAG, {name})];
   }
 
-  serialize(nodes: i18n.Node[], ctx: {mapper: PlaceholderMapper}): xml.Node[] {
+  serialize(nodes: i18n.Node[]): xml.Node[] {
-    return [].concat(...nodes.map(node => node.visit(this, ctx)));
+    return [].concat(...nodes.map(node => node.visit(this)));
   }
 }
 
@@ -197,25 +181,25 @@ export class XmbPlaceholderMapper implements PlaceholderMapper, i18n.Visitor {
     return this.xmbToInternal.hasOwnProperty(publicName) ? this.xmbToInternal[publicName] : null;
   }
 
-  visitText(text: i18n.Text, ctx?: any): any { return null; }
+  visitText(text: i18n.Text, context?: any): any { return null; }
 
-  visitContainer(container: i18n.Container, ctx?: any): any {
+  visitContainer(container: i18n.Container, context?: any): any {
     container.children.forEach(child => child.visit(this));
   }
 
-  visitIcu(icu: i18n.Icu, ctx?: any): any {
+  visitIcu(icu: i18n.Icu, context?: any): any {
     Object.keys(icu.cases).forEach(k => { icu.cases[k].visit(this); });
   }
 
-  visitTagPlaceholder(ph: i18n.TagPlaceholder, ctx?: any): any {
+  visitTagPlaceholder(ph: i18n.TagPlaceholder, context?: any): any {
     this.addPlaceholder(ph.startName);
     ph.children.forEach(child => child.visit(this));
     this.addPlaceholder(ph.closeName);
   }
 
-  visitPlaceholder(ph: i18n.Placeholder, ctx?: any): any { this.addPlaceholder(ph.name); }
+  visitPlaceholder(ph: i18n.Placeholder, context?: any): any { this.addPlaceholder(ph.name); }
 
-  visitIcuPlaceholder(ph: i18n.IcuPlaceholder, ctx?: any): any { this.addPlaceholder(ph.name); }
+  visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any { this.addPlaceholder(ph.name); }
 
   // XMB placeholders could only contains A-Z, 0-9 and _
   private addPlaceholder(internalName: string): void {

modules/@angular/compiler/test/i18n/message_bundle_spec.ts

@@ -28,14 +28,13 @@ export function main(): void {
         ]);
       });
 
-      it('should extract the all messages and duplicates', () => {
+      it('should extract and dedup messages', () => {
         messages.updateFromTemplate(
-            '<p i18n="m|d">Translate Me</p><p i18n>Translate Me</p><p i18n>Translate Me</p>', 'url',
+            '<p i18n="m|d@@1">Translate Me</p><p i18n="@@2">Translate Me</p><p i18n="@@2">Translate Me</p>',
-            DEFAULT_INTERPOLATION_CONFIG);
+            'url', DEFAULT_INTERPOLATION_CONFIG);
         expect(humanizeMessages(messages)).toEqual([
           'Translate Me (m|d)',
           'Translate Me (|)',
-          'Translate Me (|)',
         ]);
       });
     });
@@ -50,7 +49,7 @@ class _TestSerializer extends Serializer {
 
   load(content: string, url: string): {} { return null; }
 
-  digest(msg: i18n.Message): string { return 'unused'; }
+  digest(msg: i18n.Message): string { return msg.id || `default`; }
 }
 
 function humanizeMessages(catalog: MessageBundle): string[] {