refactor: code cleanup

modules/@angular/compiler/src/directive_normalizer.ts

@@ -20,7 +20,6 @@ import {PreparsedElementType, preparseElement} from './template_preparser';
 import {UrlResolver} from './url_resolver';
 import {SyncAsyncResult} from './util';
 import {XHR} from './xhr';
-import {InterpolationConfig} from './interpolation_config';
 
 @Injectable()
 export class DirectiveNormalizer {

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

@@ -19,7 +19,7 @@ import {I18N_ATTR, I18N_ATTR_PREFIX, I18nError, Part, dedupePhName, extractPhNam
 
 const _PLACEHOLDER_ELEMENT = 'ph';
 const _NAME_ATTR = 'name';
-const _PLACEHOLDER_EXPANDED_REGEXP = /<ph(\s)+name=("(\w)+")><\/ph>/gi;
+const _PLACEHOLDER_EXPANDED_REGEXP = /<ph\s+name="(\w+)"><\/ph>/gi;
 
 /**
  * Creates an i18n-ed version of the parsed template.
@@ -49,7 +49,7 @@ const _PLACEHOLDER_EXPANDED_REGEXP = /<ph(\s)+name=("(\w)+")><\/ph>/gi;
  *     corresponding original expressions
  */
 export class I18nHtmlParser implements HtmlParser {
-  errors: ParseError[];
+  private _errors: ParseError[];
   private _interpolationConfig: InterpolationConfig;
 
   constructor(
@@ -61,7 +61,7 @@ export class I18nHtmlParser implements HtmlParser {
       sourceContent: string, sourceUrl: string, parseExpansionForms: boolean = false,
       interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG):
       HtmlParseTreeResult {
-    this.errors = [];
+    this._errors = [];
     this._interpolationConfig = interpolationConfig;
 
     let res = this._htmlParser.parse(sourceContent, sourceUrl, true, interpolationConfig);
@@ -72,16 +72,17 @@ export class I18nHtmlParser implements HtmlParser {
 
     const nodes = this._recurse(res.rootNodes);
 
-    return this.errors.length > 0 ? new HtmlParseTreeResult([], this.errors) :
+    return this._errors.length > 0 ? new HtmlParseTreeResult([], this._errors) :
-                                    new HtmlParseTreeResult(nodes, []);
+                                     new HtmlParseTreeResult(nodes, []);
   }
 
+  // Merge the translation recursively
   private _processI18nPart(part: Part): HtmlAst[] {
     try {
       return part.hasI18n ? this._mergeI18Part(part) : this._recurseIntoI18nPart(part);
     } catch (e) {
       if (e instanceof I18nError) {
-        this.errors.push(e);
+        this._errors.push(e);
         return [];
       } else {
         throw e;
@@ -89,128 +90,143 @@ export class I18nHtmlParser implements HtmlParser {
     }
   }
 
+  private _recurseIntoI18nPart(p: Part): HtmlAst[] {
+    // we found an element without an i18n attribute
+    // we need to recurse in case its children may have i18n set
+    // we also need to translate its attributes
+    if (isPresent(p.rootElement)) {
+      const root = p.rootElement;
+      const children = this._recurse(p.children);
+      const attrs = this._i18nAttributes(root);
+      return [new HtmlElementAst(
+          root.name, attrs, children, root.sourceSpan, root.startSourceSpan, root.endSourceSpan)];
+    }
+
+    if (isPresent(p.rootTextNode)) {
+      // a text node without i18n or interpolation, nothing to do
+      return [p.rootTextNode];
+    }
+
+    return this._recurse(p.children);
+  }
+
+  private _recurse(nodes: HtmlAst[]): HtmlAst[] {
+    let parts = partition(nodes, this._errors, this._implicitTags);
+    return ListWrapper.flatten(parts.map(p => this._processI18nPart(p)));
+  }
+
+  // Look for the translated message and merge it back to the tree
   private _mergeI18Part(part: Part): HtmlAst[] {
     let message = part.createMessage(this._expressionParser, this._interpolationConfig);
     let messageId = id(message);
+
     if (!StringMapWrapper.contains(this._messages, messageId)) {
       throw new I18nError(
           part.sourceSpan,
           `Cannot find message for id '${messageId}', content '${message.content}'.`);
     }
 
-    let parsedMessage = this._messages[messageId];
+    const translation = this._messages[messageId];
-    return this._mergeTrees(part, parsedMessage, part.children);
+    return this._mergeTrees(part, translation);
   }
 
-  private _recurseIntoI18nPart(p: Part): HtmlAst[] {
-    // we found an element without an i18n attribute
-    // we need to recurse in cause its children may have i18n set
-    // we also need to translate its attributes
-    if (isPresent(p.rootElement)) {
-      let root = p.rootElement;
-      let children = this._recurse(p.children);
-      let attrs = this._i18nAttributes(root);
-      return [new HtmlElementAst(
-          root.name, attrs, children, root.sourceSpan, root.startSourceSpan, root.endSourceSpan)];
 
-      // a text node without i18n or interpolation, nothing to do
+  private _mergeTrees(part: Part, translation: HtmlAst[]): HtmlAst[] {
-    } else if (isPresent(p.rootTextNode)) {
+    if (isPresent(part.rootTextNode)) {
-      return [p.rootTextNode];
+      // this should never happen with a part. Parts that have root text node should not be merged.
-
+      throw new BaseException('should not be reached');
-    } else {
-      return this._recurse(p.children);
     }
-  }
 
-  private _recurse(nodes: HtmlAst[]): HtmlAst[] {
+    const visitor = new _NodeMappingVisitor();
-    let parts = partition(nodes, this.errors, this._implicitTags);
+    htmlVisitAll(visitor, part.children);
-    return ListWrapper.flatten(parts.map(p => this._processI18nPart(p)));
-  }
-
-  private _mergeTrees(p: Part, translated: HtmlAst[], original: HtmlAst[]): HtmlAst[] {
-    let l = new _CreateNodeMapping();
-    htmlVisitAll(l, original);
 
     // merge the translated tree with the original tree.
     // we do it by preserving the source code position of the original tree
-    let merged = this._mergeTreesHelper(translated, l.mapping);
+    const translatedAst = this._expandPlaceholders(translation, visitor.mapping);
 
     // if the root element is present, we need to create a new root element with its attributes
     // translated
-    if (isPresent(p.rootElement)) {
+    if (part.rootElement) {
-      let root = p.rootElement;
+      const root = part.rootElement;
-      let attrs = this._i18nAttributes(root);
+      const attrs = this._i18nAttributes(root);
       return [new HtmlElementAst(
-          root.name, attrs, merged, root.sourceSpan, root.startSourceSpan, root.endSourceSpan)];
+          root.name, attrs, translatedAst, root.sourceSpan, root.startSourceSpan,
-
+          root.endSourceSpan)];
-      // this should never happen with a part. Parts that have root text node should not be merged.
-    } else if (isPresent(p.rootTextNode)) {
-      throw new BaseException('should not be reached');
-
-    } else {
-      return merged;
     }
+
+    return translatedAst;
   }
 
-  private _mergeTreesHelper(translated: HtmlAst[], mapping: HtmlAst[]): HtmlAst[] {
+  /**
-    return translated.map(t => {
+   * The translation AST is composed on text nodes and placeholder elements
-      if (t instanceof HtmlElementAst) {
+   */
-        return this._mergeElementOrInterpolation(t, translated, mapping);
+  private _expandPlaceholders(translation: HtmlAst[], mapping: HtmlAst[]): HtmlAst[] {
-
+    return translation.map(node => {
-      } else if (t instanceof HtmlTextAst) {
+      if (node instanceof HtmlElementAst) {
-        return t;
+        // This node is a placeholder, replace with the original content
-
+        return this._expandPlaceholdersInNode(node, mapping);
-      } else {
-        throw new BaseException('should not be reached');
       }
+
+      if (node instanceof HtmlTextAst) {
+        return node;
+      }
+
+      throw new BaseException('should not be reached');
     });
   }
 
-  private _mergeElementOrInterpolation(
+  private _expandPlaceholdersInNode(node: HtmlElementAst, mapping: HtmlAst[]): HtmlAst {
-      t: HtmlElementAst, translated: HtmlAst[], mapping: HtmlAst[]): HtmlAst {
+    let name = this._getName(node);
-    let name = this._getName(t);
-    let type = name[0];
     let index = NumberWrapper.parseInt(name.substring(1), 10);
     let originalNode = mapping[index];
 
-    if (type == 't') {
+    if (originalNode instanceof HtmlTextAst) {
-      return this._mergeTextInterpolation(t, <HtmlTextAst>originalNode);
+      return this._mergeTextInterpolation(node, originalNode);
-    } else if (type == 'e') {
-      return this._mergeElement(t, <HtmlElementAst>originalNode, mapping);
-    } else {
-      throw new BaseException('should not be reached');
     }
+
+    if (originalNode instanceof HtmlElementAst) {
+      return this._mergeElement(node, originalNode, mapping);
+    }
+
+    throw new BaseException('should not be reached');
   }
 
-  private _getName(t: HtmlElementAst): string {
+  // Extract the value of a <ph> name attribute
-    if (t.name != _PLACEHOLDER_ELEMENT) {
+  private _getName(node: HtmlElementAst): string {
+    if (node.name != _PLACEHOLDER_ELEMENT) {
       throw new I18nError(
-          t.sourceSpan,
+          node.sourceSpan,
-          `Unexpected tag "${t.name}". Only "${_PLACEHOLDER_ELEMENT}" tags are allowed.`);
+          `Unexpected tag "${node.name}". Only "${_PLACEHOLDER_ELEMENT}" tags are allowed.`);
     }
-    let names = t.attrs.filter(a => a.name == _NAME_ATTR);
+
-    if (names.length == 0) {
+    const nameAttr = node.attrs.find(a => a.name == _NAME_ATTR);
-      throw new I18nError(t.sourceSpan, `Missing "${_NAME_ATTR}" attribute.`);
+
+    if (nameAttr) {
+      return nameAttr.value;
     }
-    return names[0].value;
+
+    throw new I18nError(node.sourceSpan, `Missing "${_NAME_ATTR}" attribute.`);
   }
 
-  private _mergeTextInterpolation(t: HtmlElementAst, originalNode: HtmlTextAst): HtmlTextAst {
+  private _mergeTextInterpolation(node: HtmlElementAst, originalNode: HtmlTextAst): HtmlTextAst {
-    let split = this._expressionParser.splitInterpolation(
+    const split = this._expressionParser.splitInterpolation(
         originalNode.value, originalNode.sourceSpan.toString(), this._interpolationConfig);
-    let exps = isPresent(split) ? split.expressions : [];
 
-    let messageSubstring =
+    const exps = split ? split.expressions : [];
-        this._messagesContent.substring(t.startSourceSpan.end.offset, t.endSourceSpan.start.offset);
+
-    let translated =
+    const messageSubstring = this._messagesContent.substring(
-        this._replacePlaceholdersWithExpressions(messageSubstring, exps, originalNode.sourceSpan);
+        node.startSourceSpan.end.offset, node.endSourceSpan.start.offset);
+
+    let translated = this._replacePlaceholdersWithInterpolations(
+        messageSubstring, exps, originalNode.sourceSpan);
 
     return new HtmlTextAst(translated, originalNode.sourceSpan);
   }
 
-  private _mergeElement(t: HtmlElementAst, originalNode: HtmlElementAst, mapping: HtmlAst[]):
+  private _mergeElement(node: HtmlElementAst, originalNode: HtmlElementAst, mapping: HtmlAst[]):
       HtmlElementAst {
-    let children = this._mergeTreesHelper(t.children, mapping);
+    const children = this._expandPlaceholders(node.children, mapping);
+
     return new HtmlElementAst(
         originalNode.name, this._i18nAttributes(originalNode), children, originalNode.sourceSpan,
         originalNode.startSourceSpan, originalNode.endSourceSpan);
@@ -226,9 +242,9 @@ export class I18nHtmlParser implements HtmlParser {
 
       let message: Message;
 
-      let i18ns = el.attrs.filter(a => a.name == `${I18N_ATTR_PREFIX}${attr.name}`);
+      let i18nAttr = el.attrs.find(a => a.name == `${I18N_ATTR_PREFIX}${attr.name}`);
 
-      if (i18ns.length == 0) {
+      if (!i18nAttr) {
         if (implicitAttrs.indexOf(attr.name) == -1) {
           res.push(attr);
           return;
@@ -236,13 +252,13 @@ export class I18nHtmlParser implements HtmlParser {
         message = messageFromAttribute(this._expressionParser, this._interpolationConfig, attr);
       } else {
         message = messageFromI18nAttribute(
-            this._expressionParser, this._interpolationConfig, el, i18ns[0]);
+            this._expressionParser, this._interpolationConfig, el, i18nAttr);
       }
 
       let messageId = id(message);
 
       if (StringMapWrapper.contains(this._messages, messageId)) {
-        let updatedMessage = this._replaceInterpolationInAttr(attr, this._messages[messageId]);
+        const updatedMessage = this._replaceInterpolationInAttr(attr, this._messages[messageId]);
         res.push(new HtmlAttrAst(attr.name, updatedMessage, attr.sourceSpan));
 
       } else {
@@ -251,43 +267,44 @@ export class I18nHtmlParser implements HtmlParser {
             `Cannot find message for id '${messageId}', content '${message.content}'.`);
       }
     });
+
     return res;
   }
 
   private _replaceInterpolationInAttr(attr: HtmlAttrAst, msg: HtmlAst[]): string {
-    let split = this._expressionParser.splitInterpolation(
+    const split = this._expressionParser.splitInterpolation(
         attr.value, attr.sourceSpan.toString(), this._interpolationConfig);
-    let exps = isPresent(split) ? split.expressions : [];
+    const exps = isPresent(split) ? split.expressions : [];
 
-    let first = msg[0];
+    const first = msg[0];
-    let last = msg[msg.length - 1];
+    const last = msg[msg.length - 1];
 
-    let start = first.sourceSpan.start.offset;
+    const start = first.sourceSpan.start.offset;
-    let end =
+    const end =
         last instanceof HtmlElementAst ? last.endSourceSpan.end.offset : last.sourceSpan.end.offset;
-    let messageSubstring = this._messagesContent.substring(start, end);
+    const messageSubstring = this._messagesContent.substring(start, end);
 
-    return this._replacePlaceholdersWithExpressions(messageSubstring, exps, attr.sourceSpan);
+    return this._replacePlaceholdersWithInterpolations(messageSubstring, exps, attr.sourceSpan);
   };
 
-  private _replacePlaceholdersWithExpressions(
+  private _replacePlaceholdersWithInterpolations(
       message: string, exps: string[], sourceSpan: ParseSourceSpan): string {
-    let expMap = this._buildExprMap(exps);
+    const expMap = this._buildExprMap(exps);
-    return RegExpWrapper.replaceAll(_PLACEHOLDER_EXPANDED_REGEXP, message, (match: string[]) => {
+
-      let nameWithQuotes = match[2];
+    return message.replace(
-      let name = nameWithQuotes.substring(1, nameWithQuotes.length - 1);
+        _PLACEHOLDER_EXPANDED_REGEXP,
-      return this._convertIntoExpression(name, expMap, sourceSpan);
+        (_: string, name: string) => this._convertIntoExpression(name, expMap, sourceSpan));
-    });
   }
 
   private _buildExprMap(exps: string[]): Map<string, string> {
-    let expMap = new Map<string, string>();
+    const expMap = new Map<string, string>();
-    let usedNames = new Map<string, number>();
+    const usedNames = new Map<string, number>();
 
-    for (var i = 0; i < exps.length; i++) {
+    for (let i = 0; i < exps.length; i++) {
-      let phName = extractPhNameFromInterpolation(exps[i], i);
+      const phName = extractPhNameFromInterpolation(exps[i], i);
       expMap.set(dedupePhName(usedNames, phName), exps[i]);
     }
+
     return expMap;
   }
 
@@ -295,31 +312,26 @@ export class I18nHtmlParser implements HtmlParser {
       name: string, expMap: Map<string, string>, sourceSpan: ParseSourceSpan) {
     if (expMap.has(name)) {
       return `${this._interpolationConfig.start}${expMap.get(name)}${this._interpolationConfig.end}`;
-    } else {
-      throw new I18nError(sourceSpan, `Invalid interpolation name '${name}'`);
     }
+
+    throw new I18nError(sourceSpan, `Invalid interpolation name '${name}'`);
   }
 }
 
-class _CreateNodeMapping implements HtmlAstVisitor {
+// Creates a list of elements and text nodes in the AST
+// The indexes match the placeholders indexes
+class _NodeMappingVisitor implements HtmlAstVisitor {
   mapping: HtmlAst[] = [];
 
   visitElement(ast: HtmlElementAst, context: any): any {
     this.mapping.push(ast);
     htmlVisitAll(this, ast.children);
-    return null;
   }
 
-  visitAttr(ast: HtmlAttrAst, context: any): any { return null; }
+  visitText(ast: HtmlTextAst, context: any): any { this.mapping.push(ast); }
 
-  visitText(ast: HtmlTextAst, context: any): any {
+  visitAttr(ast: HtmlAttrAst, context: any): any {}
-    this.mapping.push(ast);
+  visitExpansion(ast: HtmlExpansionAst, context: any): any {}
-    return null;
+  visitExpansionCase(ast: HtmlExpansionCaseAst, context: any): any {}
-  }
+  visitComment(ast: HtmlCommentAst, context: any): any {}
-
-  visitExpansion(ast: HtmlExpansionAst, context: any): any { return null; }
-
-  visitExpansionCase(ast: HtmlExpansionCaseAst, context: any): any { return null; }
-
-  visitComment(ast: HtmlCommentAst, context: any): any { return ''; }
 }

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

@@ -70,7 +70,8 @@ export class Part {
       return this.rootTextNode.sourceSpan;
     }
 
-    return this.children[0].sourceSpan;
+    return new ParseSourceSpan(
+        this.children[0].sourceSpan.start, this.children[this.children.length - 1].sourceSpan.end);
   }
 
   createMessage(parser: ExpressionParser, interpolationConfig: InterpolationConfig): Message {
@@ -117,8 +118,8 @@ export function description(i18n: string): string {
 export function messageFromI18nAttribute(
     parser: ExpressionParser, interpolationConfig: InterpolationConfig, p: HtmlElementAst,
     i18nAttr: HtmlAttrAst): Message {
-  let expectedName = i18nAttr.name.substring(5);
+  const expectedName = i18nAttr.name.substring(5);
-  let attr = p.attrs.find(a => a.name == expectedName);
+  const attr = p.attrs.find(a => a.name == expectedName);
 
   if (attr) {
     return messageFromAttribute(
@@ -131,7 +132,7 @@ export function messageFromI18nAttribute(
 export function messageFromAttribute(
     parser: ExpressionParser, interpolationConfig: InterpolationConfig, attr: HtmlAttrAst,
     meaning: string = null, description: string = null): Message {
-  let value = removeInterpolation(attr.value, attr.sourceSpan, parser, interpolationConfig);
+  const value = removeInterpolation(attr.value, attr.sourceSpan, parser, interpolationConfig);
   return new Message(value, meaning, description);
 }
 

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

@@ -34,9 +34,8 @@ export class XmbDeserializationError extends ParseError {
 }
 
 export function deserializeXmb(content: string, url: string): XmbDeserializationResult {
-  const parser = new HtmlParser();
   const normalizedContent = _expandPlaceholder(content.trim());
-  const parsed = parser.parse(normalizedContent, url);
+  const parsed = new HtmlParser().parse(normalizedContent, url);
 
   if (parsed.errors.length > 0) {
     return new XmbDeserializationResult(null, {}, parsed.errors);
@@ -47,9 +46,9 @@ export function deserializeXmb(content: string, url: string): XmbDeserialization
         null, {}, [new XmbDeserializationError(null, `Missing element "${_BUNDLE_ELEMENT}"`)]);
   }
 
-  let bundleEl = <HtmlElementAst>parsed.rootNodes[0];  // test this
+  const bundleEl = <HtmlElementAst>parsed.rootNodes[0];  // test this
-  let errors: ParseError[] = [];
+  const errors: ParseError[] = [];
-  let messages: {[key: string]: HtmlAst[]} = {};
+  const messages: {[key: string]: HtmlAst[]} = {};
 
   _createMessages(bundleEl.children, messages, errors);
 
@@ -65,33 +64,28 @@ function _checkRootElement(nodes: HtmlAst[]): boolean {
 
 function _createMessages(
     nodes: HtmlAst[], messages: {[key: string]: HtmlAst[]}, errors: ParseError[]): void {
-  nodes.forEach((item) => {
+  nodes.forEach((node) => {
-    if (item instanceof HtmlElementAst) {
+    if (node instanceof HtmlElementAst) {
-      let msg = <HtmlElementAst>item;
+      let msg = <HtmlElementAst>node;
 
       if (msg.name != _MSG_ELEMENT) {
         errors.push(
-            new XmbDeserializationError(item.sourceSpan, `Unexpected element "${msg.name}"`));
+            new XmbDeserializationError(node.sourceSpan, `Unexpected element "${msg.name}"`));
         return;
       }
 
-      let id = _id(msg);
+      let idAttr = msg.attrs.find(a => a.name == _ID_ATTR);
-      if (isBlank(id)) {
+
+      if (idAttr) {
+        messages[idAttr.value] = msg.children;
+      } else {
         errors.push(
-            new XmbDeserializationError(item.sourceSpan, `"${_ID_ATTR}" attribute is missing`));
+            new XmbDeserializationError(node.sourceSpan, `"${_ID_ATTR}" attribute is missing`));
-        return;
       }
-
-      messages[id] = msg.children;
     }
   });
 }
 
-function _id(el: HtmlElementAst): string {
-  let ids = el.attrs.filter(a => a.name == _ID_ATTR);
-  return ids.length > 0 ? ids[0].value : null;
-}
-
 function _serializeMessage(m: Message): string {
   const desc = isPresent(m.description) ? ` desc='${_escapeXml(m.description)}'` : '';
   const meaning = isPresent(m.meaning) ? ` meaning='${_escapeXml(m.meaning)}'` : '';

modules/@angular/compiler/src/interpolation_config.ts

@@ -11,8 +11,7 @@ import {isBlank} from './facade/lang';
 
 export class InterpolationConfig {
   static fromArray(markers: [string, string]): InterpolationConfig {
-    if (isBlank(markers)) {
+    if (!markers) {
-      // TODO:bad ??
       return DEFAULT_INTERPOLATION_CONFIG;
     }
 

modules/@angular/compiler/src/template_parser.ts

@@ -101,7 +101,6 @@ export class TemplateParser {
   tryParse(
       component: CompileDirectiveMetadata, template: string, directives: CompileDirectiveMetadata[],
       pipes: CompilePipeMetadata[], templateUrl: string): TemplateParseResult {
-    // TODO: bad ???
     let interpolationConfig: any;
     if (component.template) {
       interpolationConfig = InterpolationConfig.fromArray(component.template.interpolation);