refactor(i18n): remove utility functions into a separate file

modules/angular2/src/i18n/message.ts

@@ -8,7 +8,7 @@ import {isPresent, escape} from 'angular2/src/facade/lang';
  * `description` is additional information provided to the translator.
  */
 export class Message {
-  constructor(public content: string, public meaning: string, public description: string) {}
+  constructor(public content: string, public meaning: string, public description: string = null) {}
 }
 
 /**

modules/angular2/src/i18n/message_extractor.ts

@@ -12,11 +12,17 @@ import {
 import {isPresent, isBlank} from 'angular2/src/facade/lang';
 import {StringMapWrapper} from 'angular2/src/facade/collection';
 import {Parser} from 'angular2/src/core/change_detection/parser/parser';
-import {Interpolation} from 'angular2/src/core/change_detection/parser/ast';
 import {Message, id} from './message';
-
+import {
-const I18N_ATTR = "i18n";
+  I18nError,
-const I18N_ATTR_PREFIX = "i18n-";
+  Part,
+  partition,
+  meaning,
+  description,
+  isI18nAttr,
+  stringifyNodes,
+  messageFromAttribute
+} from './shared';
 
 /**
  * All messages extracted from a template.
@@ -25,13 +31,6 @@ export class ExtractionResult {
   constructor(public messages: Message[], public errors: ParseError[]) {}
 }
 
-/**
- * An extraction error.
- */
-export class I18nExtractionError extends ParseError {
-  constructor(span: ParseSourceSpan, msg: string) { super(span, msg); }
-}
-
 /**
  * Removes duplicate messages.
  *
@@ -56,20 +55,61 @@ export function removeDuplicates(messages: Message[]): Message[] {
 /**
  * Extracts all messages from a template.
  *
- * It works like this. First, the extractor uses the provided html parser to get
+ * Algorithm:
- * the html AST of the template. Then it partitions the root nodes into parts.
- * Everything between two i18n comments becomes a single part. Every other nodes becomes
- * a part too.
  *
- * We process every part as follows. Say we have a part A.
+ * To understand the algorithm, you need to know how partitioning works.
+ * Partitioning is required as we can use two i18n comments to group node siblings together.
+ * That is why we cannot just use nodes.
  *
- * If the part has the i18n attribute, it gets converted into a message.
+ * Partitioning transforms an array of HtmlAst into an array of Part.
- * And we do not recurse into that part, except to extract messages from the attributes.
+ * A part can optionally contain a root element or a root text node. And it can also contain
+ * children.
+ * A part can contain i18n property, in which case it needs to be extracted.
  *
- * If the part doesn't have the i18n attribute, we recurse into that part and
+ * Example:
- * partition its children.
  *
- * While walking the AST we also remove i18n attributes from messages.
+ * The following array of nodes will be split into four parts:
+ *
+ * ```
+ * <a>A</a>
+ * <b i18n>B</b>
+ * <!-- i18n -->
+ * <c>C</c>
+ * D
+ * <!-- /i18n -->
+ * E
+ * ```
+ *
+ * Part 1 containing the a tag. It should not be translated.
+ * Part 2 containing the b tag. It should be translated.
+ * Part 3 containing the c tag and the D text node. It should be translated.
+ * Part 4 containing the E text node. It should not be translated..
+ *
+ * It is also important to understand how we stringify nodes to create a message.
+ *
+ * We walk the tree and replace every element node with a placeholder. We also replace
+ * all expressions in interpolation with placeholders. We also insert a placeholder element
+ * to wrap a text node containing interpolation.
+ *
+ * Example:
+ *
+ * The following tree:
+ *
+ * ```
+ * <a>A{{I}}</a><b>B</b>
+ * ```
+ *
+ * will be stringified into:
+ * ```
+ * <ph name="e0"><ph name="t1">A<ph name="0"/></ph></ph><ph name="e2">B</ph>
+ * ```
+ *
+ * This is what the algorithm does:
+ *
+ * 1. Use the provided html parser to get the html AST of the template.
+ * 2. Partition the root nodes, and process each part separately.
+ * 3. If a part does not have the i18n attribute, recurse to process children and attributes.
+ * 4. If a part has the i18n attribute, stringify the nodes to create a Message.
  */
 export class MessageExtractor {
   messages: Message[];
@@ -85,16 +125,14 @@ export class MessageExtractor {
     if (res.errors.length > 0) {
       return new ExtractionResult([], res.errors);
     } else {
-      let ps = this._partition(res.rootNodes);
+      this._recurse(res.rootNodes);
-      ps.forEach(p => this._extractMessagesFromPart(p));
       return new ExtractionResult(this.messages, this.errors);
     }
   }
 
-  private _extractMessagesFromPart(p: _Part): void {
+  private _extractMessagesFromPart(p: Part): void {
     if (p.hasI18n) {
-      this.messages.push(new Message(_stringifyNodes(p.children, this._parser), _meaning(p.i18n),
+      this.messages.push(p.createMessage(this._parser));
-                                     _description(p.i18n)));
       this._recurseToExtractMessagesFromAttributes(p.children);
     } else {
       this._recurse(p.children);
@@ -106,9 +144,11 @@ export class MessageExtractor {
   }
 
   private _recurse(nodes: HtmlAst[]): void {
-    let ps = this._partition(nodes);
+    if (isPresent(nodes)) {
+      let ps = partition(nodes, this.errors);
       ps.forEach(p => this._extractMessagesFromPart(p));
     }
+  }
 
   private _recurseToExtractMessagesFromAttributes(nodes: HtmlAst[]): void {
     nodes.forEach(n => {
@@ -121,130 +161,17 @@ export class MessageExtractor {
 
   private _extractMessagesFromAttributes(p: HtmlElementAst): void {
     p.attrs.forEach(attr => {
-      if (attr.name.startsWith(I18N_ATTR_PREFIX)) {
+      if (isI18nAttr(attr.name)) {
-        let expectedName = attr.name.substring(5);
+        try {
-        let matching = p.attrs.filter(a => a.name == expectedName);
+          this.messages.push(messageFromAttribute(this._parser, p, attr));
-
+        } catch (e) {
-        if (matching.length > 0) {
+          if (e instanceof I18nError) {
-          let value = _removeInterpolation(matching[0].value, p.sourceSpan, this._parser);
+            this.errors.push(e);
-          this.messages.push(new Message(value, _meaning(attr.value), _description(attr.value)));
           } else {
-          this.errors.push(
+            throw e;
-              new I18nExtractionError(p.sourceSpan, `Missing attribute '${expectedName}'.`));
+          }
         }
       }
     });
   }
-
-  // Man, this is so ugly!
-  private _partition(nodes: HtmlAst[]): _Part[] {
-    let res = [];
-
-    for (let i = 0; i < nodes.length; ++i) {
-      let n = nodes[i];
-      let temp = [];
-      if (_isOpeningComment(n)) {
-        let i18n = (<HtmlCommentAst>n).value.substring(5).trim();
-        i++;
-        while (!_isClosingComment(nodes[i])) {
-          temp.push(nodes[i++]);
-          if (i === nodes.length) {
-            this.errors.push(
-                new I18nExtractionError(n.sourceSpan, "Missing closing 'i18n' comment."));
-            break;
-          }
-        }
-        res.push(new _Part(null, temp, i18n, true));
-
-      } else if (n instanceof HtmlElementAst) {
-        let i18n = _findI18nAttr(n);
-        res.push(new _Part(n, n.children, isPresent(i18n) ? i18n.value : null, isPresent(i18n)));
-      }
-    }
-
-    return res;
-  }
-}
-
-class _Part {
-  constructor(public rootElement: HtmlElementAst, public children: HtmlAst[], public i18n: string,
-              public hasI18n: boolean) {}
-}
-
-function _isOpeningComment(n: HtmlAst): boolean {
-  return n instanceof HtmlCommentAst && isPresent(n.value) && n.value.startsWith("i18n:");
-}
-
-function _isClosingComment(n: HtmlAst): boolean {
-  return n instanceof HtmlCommentAst && isPresent(n.value) && n.value == "/i18n";
-}
-
-function _stringifyNodes(nodes: HtmlAst[], parser: Parser) {
-  let visitor = new _StringifyVisitor(parser);
-  return htmlVisitAll(visitor, nodes).join("");
-}
-
-class _StringifyVisitor implements HtmlAstVisitor {
-  constructor(private _parser: Parser) {}
-
-  visitElement(ast: HtmlElementAst, context: any): any {
-    let attrs = this._join(htmlVisitAll(this, ast.attrs), " ");
-    let children = this._join(htmlVisitAll(this, ast.children), "");
-    return `<${ast.name} ${attrs}>${children}</${ast.name}>`;
-  }
-
-  visitAttr(ast: HtmlAttrAst, context: any): any {
-    if (ast.name.startsWith(I18N_ATTR_PREFIX)) {
-      return "";
-    } else {
-      return `${ast.name}="${ast.value}"`;
-    }
-  }
-
-  visitText(ast: HtmlTextAst, context: any): any {
-    return _removeInterpolation(ast.value, ast.sourceSpan, this._parser);
-  }
-
-  visitComment(ast: HtmlCommentAst, context: any): any { return ""; }
-
-  private _join(strs: string[], str: string): string {
-    return strs.filter(s => s.length > 0).join(str);
-  }
-}
-
-function _removeInterpolation(value: string, source: ParseSourceSpan, parser: Parser): string {
-  try {
-    let parsed = parser.parseInterpolation(value, source.toString());
-    if (isPresent(parsed)) {
-      let ast: Interpolation = <any>parsed.ast;
-      let res = "";
-      for (let i = 0; i < ast.strings.length; ++i) {
-        res += ast.strings[i];
-        if (i != ast.strings.length - 1) {
-          res += `{{I${i}}}`;
-        }
-      }
-      return res;
-    } else {
-      return value;
-    }
-  } catch (e) {
-    return value;
-  }
-}
-
-function _findI18nAttr(p: HtmlElementAst): HtmlAttrAst {
-  let i18n = p.attrs.filter(a => a.name == I18N_ATTR);
-  return i18n.length == 0 ? null : i18n[0];
-}
-
-function _meaning(i18n: string): string {
-  if (isBlank(i18n) || i18n == "") return null;
-  return i18n.split("|")[0];
-}
-
-function _description(i18n: string): string {
-  if (isBlank(i18n) || i18n == "") return null;
-  let parts = i18n.split("|");
-  return parts.length > 1 ? parts[1] : null;
 }

modules/angular2/src/i18n/shared.ts

@@ -0,0 +1,169 @@
+import {ParseSourceSpan, ParseError} from 'angular2/src/compiler/parse_util';
+import {
+  HtmlAst,
+  HtmlAstVisitor,
+  HtmlElementAst,
+  HtmlAttrAst,
+  HtmlTextAst,
+  HtmlCommentAst,
+  htmlVisitAll
+} from 'angular2/src/compiler/html_ast';
+import {isPresent, isBlank} from 'angular2/src/facade/lang';
+import {Message} from './message';
+import {Parser} from 'angular2/src/core/change_detection/parser/parser';
+
+const I18N_ATTR = "i18n";
+const I18N_ATTR_PREFIX = "i18n-";
+
+/**
+ * An i18n error.
+ */
+export class I18nError extends ParseError {
+  constructor(span: ParseSourceSpan, msg: string) { super(span, msg); }
+}
+
+
+// Man, this is so ugly!
+export function partition(nodes: HtmlAst[], errors: ParseError[]): Part[] {
+  let res = [];
+
+  for (let i = 0; i < nodes.length; ++i) {
+    let n = nodes[i];
+    let temp = [];
+    if (_isOpeningComment(n)) {
+      let i18n = (<HtmlCommentAst>n).value.substring(5).trim();
+      i++;
+      while (!_isClosingComment(nodes[i])) {
+        temp.push(nodes[i++]);
+        if (i === nodes.length) {
+          errors.push(new I18nError(n.sourceSpan, "Missing closing 'i18n' comment."));
+          break;
+        }
+      }
+      res.push(new Part(null, null, temp, i18n, true));
+
+    } else if (n instanceof HtmlElementAst) {
+      let i18n = _findI18nAttr(n);
+      res.push(new Part(n, null, n.children, isPresent(i18n) ? i18n.value : null, isPresent(i18n)));
+    } else if (n instanceof HtmlTextAst) {
+      res.push(new Part(null, n, null, null, false));
+    }
+  }
+
+  return res;
+}
+
+export class Part {
+  constructor(public rootElement: HtmlElementAst, public rootTextNode: HtmlTextAst,
+              public children: HtmlAst[], public i18n: string, public hasI18n: boolean) {}
+
+  get sourceSpan(): ParseSourceSpan {
+    if (isPresent(this.rootElement))
+      return this.rootElement.sourceSpan;
+    else if (isPresent(this.rootTextNode))
+      return this.rootTextNode.sourceSpan;
+    else
+      return this.children[0].sourceSpan;
+  }
+
+  createMessage(parser: Parser): Message {
+    return new Message(stringifyNodes(this.children, parser), meaning(this.i18n),
+                       description(this.i18n));
+  }
+}
+
+function _isOpeningComment(n: HtmlAst): boolean {
+  return n instanceof HtmlCommentAst && isPresent(n.value) && n.value.startsWith("i18n:");
+}
+
+function _isClosingComment(n: HtmlAst): boolean {
+  return n instanceof HtmlCommentAst && isPresent(n.value) && n.value == "/i18n";
+}
+
+export function isI18nAttr(n: string): boolean {
+  return n.startsWith(I18N_ATTR_PREFIX);
+}
+
+function _findI18nAttr(p: HtmlElementAst): HtmlAttrAst {
+  let i18n = p.attrs.filter(a => a.name == I18N_ATTR);
+  return i18n.length == 0 ? null : i18n[0];
+}
+
+export function meaning(i18n: string): string {
+  if (isBlank(i18n) || i18n == "") return null;
+  return i18n.split("|")[0];
+}
+
+export function description(i18n: string): string {
+  if (isBlank(i18n) || i18n == "") return null;
+  let parts = i18n.split("|");
+  return parts.length > 1 ? parts[1] : null;
+}
+
+export function messageFromAttribute(parser: Parser, p: HtmlElementAst,
+                                     attr: HtmlAttrAst): Message {
+  let expectedName = attr.name.substring(5);
+  let matching = p.attrs.filter(a => a.name == expectedName);
+
+  if (matching.length > 0) {
+    let value = removeInterpolation(matching[0].value, matching[0].sourceSpan, parser);
+    return new Message(value, meaning(attr.value), description(attr.value));
+  } else {
+    throw new I18nError(p.sourceSpan, `Missing attribute '${expectedName}'.`);
+  }
+}
+
+export function removeInterpolation(value: string, source: ParseSourceSpan,
+                                    parser: Parser): string {
+  try {
+    let parsed = parser.splitInterpolation(value, source.toString());
+    if (isPresent(parsed)) {
+      let res = "";
+      for (let i = 0; i < parsed.strings.length; ++i) {
+        res += parsed.strings[i];
+        if (i != parsed.strings.length - 1) {
+          res += `<ph name="${i}"/>`;
+        }
+      }
+      return res;
+    } else {
+      return value;
+    }
+  } catch (e) {
+    return value;
+  }
+}
+
+export function stringifyNodes(nodes: HtmlAst[], parser: Parser) {
+  let visitor = new _StringifyVisitor(parser);
+  return htmlVisitAll(visitor, nodes).join("");
+}
+
+class _StringifyVisitor implements HtmlAstVisitor {
+  private _index: number = 0;
+  constructor(private _parser: Parser) {}
+
+  visitElement(ast: HtmlElementAst, context: any): any {
+    let name = this._index++;
+    let children = this._join(htmlVisitAll(this, ast.children), "");
+    return `<ph name="e${name}">${children}</ph>`;
+  }
+
+  visitAttr(ast: HtmlAttrAst, context: any): any { return null; }
+
+  visitText(ast: HtmlTextAst, context: any): any {
+    let index = this._index++;
+    let noInterpolation = removeInterpolation(ast.value, ast.sourceSpan, this._parser);
+    if (noInterpolation != ast.value) {
+      return `<ph name="t${index}">${noInterpolation}</ph>`;
+    } else {
+      return ast.value;
+    }
+  }
+
+  visitComment(ast: HtmlCommentAst, context: any): any { return ""; }
+
+  private _join(strs: string[], str: string): string {
+    return strs.filter(s => s.length > 0).join(str);
+  }
+}

modules/angular2/test/i18n/message_extractor_spec.ts

@@ -1,8 +1,8 @@
 import {
   AsyncTestCompleter,
   beforeEach,
-  ddescribe,
   describe,
+  ddescribe,
   expect,
   iit,
   inject,
@@ -58,18 +58,6 @@ export function main() {
           ]);
     });
 
-    it('should error on i18n attributes without matching "real" attributes', () => {
-      let res = extractor.extract(`
-        <div
-          title1='message1' i18n-title1='meaning1|desc1' i18n-title2='meaning2|desc2'>
-        </div>
-      `,
-                                  "someurl");
-
-      expect(res.errors.length).toEqual(1);
-      expect(res.errors[0].msg).toEqual("Missing attribute 'title2'.");
-    });
-
     it('should extract from partitions', () => {
       let res = extractor.extract(`
          <!-- i18n: meaning1|desc1 -->message1<!-- /i18n -->
@@ -91,40 +79,39 @@ export function main() {
       expect(res.messages).toEqual([new Message("message1", "meaning1", "desc1")]);
     });
 
-    it('should error when cannot find a matching desc', () => {
-      let res = extractor.extract(`
-         <!-- i18n: meaning1|desc1 -->message1`,
-                                  "someUrl");
-
-      expect(res.errors.length).toEqual(1);
-      expect(res.errors[0].msg).toEqual("Missing closing 'i18n' comment.");
-    });
-
     it('should replace interpolation with placeholders (text nodes)', () => {
       let res = extractor.extract("<div i18n>Hi {{one}} and {{two}}</div>", "someurl");
-      expect(res.messages).toEqual([new Message("Hi {{I0}} and {{I1}}", null, null)]);
+      expect(res.messages)
+          .toEqual(
+              [new Message('<ph name="t0">Hi <ph name="0"/> and <ph name="1"/></ph>', null, null)]);
     });
 
     it('should replace interpolation with placeholders (attributes)', () => {
       let res =
           extractor.extract("<div title='Hi {{one}} and {{two}}' i18n-title></div>", "someurl");
-      expect(res.messages).toEqual([new Message("Hi {{I0}} and {{I1}}", null, null)]);
+      expect(res.messages)
-    });
+          .toEqual([new Message('Hi <ph name="0"/> and <ph name="1"/>', null, null)]);
-
-    it('should ignore errors in interpolation', () => {
-      let res = extractor.extract("<div i18n>Hi {{on???.s}}</div>", "someurl");
-      expect(res.messages).toEqual([new Message("Hi {{on???.s}}", null, null)]);
-    });
-
-    it("should return parse errors when the template is invalid", () => {
-      let res = extractor.extract("<input&#Besfs", "someurl");
-      expect(res.errors.length).toEqual(1);
-      expect(res.errors[0].msg).toEqual('Unexpected character "s"');
     });
 
     it("should handle html content", () => {
-      let res = extractor.extract('<div i18n><div attr="value">message</div></div>', "someurl");
+      let res = extractor.extract(
-      expect(res.messages).toEqual([new Message('<div attr="value">message</div>', null, null)]);
+          '<div i18n><div attr="value">zero<div>one</div></div><div>two</div></div>', "someurl");
+      expect(res.messages)
+          .toEqual([
+            new Message('<ph name="e0">zero<ph name="e2">one</ph></ph><ph name="e4">two</ph>', null,
+                        null)
+          ]);
+    });
+
+    it("should handle html content with interpolation", () => {
+      let res =
+          extractor.extract('<div i18n><div>zero{{a}}<div>{{b}}</div></div></div>', "someurl");
+      expect(res.messages)
+          .toEqual([
+            new Message(
+                '<ph name="e0"><ph name="t1">zero<ph name="0"/></ph><ph name="e2"><ph name="t3"><ph name="0"/></ph></ph></ph>',
+                null, null)
+          ]);
     });
 
     it("should extract from nested elements", () => {
@@ -143,7 +130,7 @@ export function main() {
           '<div i18n><div attr="value" i18n-attr="meaning|desc">message</div></div>', "someurl");
       expect(res.messages)
           .toEqual([
-            new Message('<div attr="value">message</div>', null, null),
+            new Message('<ph name="e0">message</ph>', null, null),
             new Message('value', "meaning", "desc")
           ]);
     });
@@ -159,5 +146,34 @@ export function main() {
             new Message("message", "meaning", "desc1"),
           ]);
     });
+
+    describe("errors", () => {
+      it('should error on i18n attributes without matching "real" attributes', () => {
+        let res = extractor.extract(`
+        <div
+          title1='message1' i18n-title1='meaning1|desc1' i18n-title2='meaning2|desc2'>
+        </div>
+      `,
+                                    "someurl");
+
+        expect(res.errors.length).toEqual(1);
+        expect(res.errors[0].msg).toEqual("Missing attribute 'title2'.");
+      });
+
+      it('should error when cannot find a matching desc', () => {
+        let res = extractor.extract(`
+         <!-- i18n: meaning1|desc1 -->message1`,
+                                    "someUrl");
+
+        expect(res.errors.length).toEqual(1);
+        expect(res.errors[0].msg).toEqual("Missing closing 'i18n' comment.");
+      });
+
+      it("should return parse errors when the template is invalid", () => {
+        let res = extractor.extract("<input&#Besfs", "someurl");
+        expect(res.errors.length).toEqual(1);
+        expect(res.errors[0].msg).toEqual('Unexpected character "s"');
+      });
+    });
   });
 }