refactor: code cleanup

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

@@ -17,7 +17,7 @@ import {ParseError, ParseSourceSpan} from '../parse_util';
 
 import {expandNodes} from './expander';
 import {Message, id} from './message';
-import {I18N_ATTR, I18N_ATTR_PREFIX, I18nError, Part, dedupePhName, getPhNameFromBinding, messageFromAttribute, messageFromI18nAttribute, partition} from './shared';
+import {I18N_ATTR, I18N_ATTR_PREFIX, I18nError, Part, dedupePhName, extractPhNameFromInterpolation, messageFromAttribute, messageFromI18nAttribute, partition} from './shared';
 
 const _PLACEHOLDER_ELEMENT = 'ph';
 const _NAME_ATTR = 'name';
@@ -289,7 +289,7 @@ export class I18nHtmlParser implements HtmlParser {
     let usedNames = new Map<string, number>();
 
     for (var i = 0; i < exps.length; i++) {
-      let phName = getPhNameFromBinding(exps[i], i);
+      let phName = extractPhNameFromInterpolation(exps[i], i);
       expMap.set(dedupePhName(usedNames, phName), exps[i]);
     }
     return expMap;

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

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Parser} from '../expression_parser/parser';
+import {Parser as ExpressionParser} from '../expression_parser/parser';
 import {StringWrapper, isBlank, isPresent} from '../facade/lang';
 import {HtmlAst, HtmlAstVisitor, HtmlAttrAst, HtmlCommentAst, HtmlElementAst, HtmlExpansionAst, HtmlExpansionCaseAst, HtmlTextAst, htmlVisitAll} from '../html_ast';
 import {InterpolationConfig} from '../interpolation_config';
@@ -15,7 +15,7 @@ import {Message} from './message';
 
 export const I18N_ATTR = 'i18n';
 export const I18N_ATTR_PREFIX = 'i18n-';
-var CUSTOM_PH_EXP = /\/\/[\s\S]*i18n[\s\S]*\([\s\S]*ph[\s\S]*=[\s\S]*"([\s\S]*?)"[\s\S]*\)/g;
+const _CUSTOM_PH_EXP = /\/\/[\s\S]*i18n[\s\S]*\([\s\S]*ph[\s\S]*=[\s\S]*"([\s\S]*?)"[\s\S]*\)/g;
 
 /**
  * An i18n error.
@@ -73,7 +73,7 @@ export class Part {
     return this.children[0].sourceSpan;
   }
 
-  createMessage(parser: Parser, interpolationConfig: InterpolationConfig): Message {
+  createMessage(parser: ExpressionParser, interpolationConfig: InterpolationConfig): Message {
     return new Message(
         stringifyNodes(this.children, parser, interpolationConfig), meaning(this.i18n),
         description(this.i18n));
@@ -115,7 +115,7 @@ export function description(i18n: string): string {
  * @internal
  */
 export function messageFromI18nAttribute(
-    parser: Parser, interpolationConfig: InterpolationConfig, p: HtmlElementAst,
+    parser: ExpressionParser, interpolationConfig: InterpolationConfig, p: HtmlElementAst,
     i18nAttr: HtmlAttrAst): Message {
   let expectedName = i18nAttr.name.substring(5);
   let attr = p.attrs.find(a => a.name == expectedName);
@@ -129,62 +129,82 @@ export function messageFromI18nAttribute(
 }
 
 export function messageFromAttribute(
-    parser: Parser, interpolationConfig: InterpolationConfig, attr: HtmlAttrAst,
+    parser: ExpressionParser, interpolationConfig: InterpolationConfig, attr: HtmlAttrAst,
     meaning: string = null, description: string = null): Message {
   let value = removeInterpolation(attr.value, attr.sourceSpan, parser, interpolationConfig);
   return new Message(value, meaning, description);
 }
 
+/**
+ * Replace interpolation in the `value` string with placeholders
+ */
 export function removeInterpolation(
-    value: string, source: ParseSourceSpan, parser: Parser,
+    value: string, source: ParseSourceSpan, expressionParser: ExpressionParser,
     interpolationConfig: InterpolationConfig): string {
   try {
-    let parsed = parser.splitInterpolation(value, source.toString(), interpolationConfig);
+    const parsed =
-    let usedNames = new Map<string, number>();
+        expressionParser.splitInterpolation(value, source.toString(), interpolationConfig);
+    const usedNames = new Map<string, number>();
     if (isPresent(parsed)) {
       let res = '';
       for (let i = 0; i < parsed.strings.length; ++i) {
         res += parsed.strings[i];
         if (i != parsed.strings.length - 1) {
-          let customPhName = getPhNameFromBinding(parsed.expressions[i], i);
+          let customPhName = extractPhNameFromInterpolation(parsed.expressions[i], i);
           customPhName = dedupePhName(usedNames, customPhName);
           res += `<ph name="${customPhName}"/>`;
         }
       }
       return res;
-    } else {
-      return value;
     }
+
+    return value;
   } catch (e) {
     return value;
   }
 }
 
-export function getPhNameFromBinding(input: string, index: number): string {
+/**
-  let customPhMatch = StringWrapper.split(input, CUSTOM_PH_EXP);
+ * Extract the placeholder name from the interpolation.
-  return customPhMatch.length > 1 ? customPhMatch[1] : `${index}`;
+ *
+ * Use a custom name when specified (ie: `{{<expression> //i18n(ph="FIRST")}}`) otherwise generate a
+ * unique name.
+ */
+export function extractPhNameFromInterpolation(input: string, index: number): string {
+  let customPhMatch = StringWrapper.split(input, _CUSTOM_PH_EXP);
+  return customPhMatch.length > 1 ? customPhMatch[1] : `INTERPOLATION_${index}`;
 }
 
+/**
+ * Return a unique placeholder name based on the given name
+ */
 export function dedupePhName(usedNames: Map<string, number>, name: string): string {
-  let duplicateNameCount = usedNames.get(name);
+  const duplicateNameCount = usedNames.get(name);
-  if (isPresent(duplicateNameCount)) {
+
+  if (duplicateNameCount) {
     usedNames.set(name, duplicateNameCount + 1);
     return `${name}_${duplicateNameCount}`;
-  } else {
-    usedNames.set(name, 1);
-    return name;
   }
+
+  usedNames.set(name, 1);
+  return name;
 }
 
+/**
+ * Convert a list of nodes to a string message.
+ *
+ */
 export function stringifyNodes(
-    nodes: HtmlAst[], parser: Parser, interpolationConfig: InterpolationConfig): string {
+    nodes: HtmlAst[], expressionParser: ExpressionParser,
-  let visitor = new _StringifyVisitor(parser, interpolationConfig);
+    interpolationConfig: InterpolationConfig): string {
+  const visitor = new _StringifyVisitor(expressionParser, interpolationConfig);
   return htmlVisitAll(visitor, nodes).join('');
 }
 
 class _StringifyVisitor implements HtmlAstVisitor {
   private _index: number = 0;
-  constructor(private _parser: Parser, private _interpolationConfig: InterpolationConfig) {}
+  constructor(
+      private _parser: ExpressionParser, private _interpolationConfig: InterpolationConfig) {}
 
   visitElement(ast: HtmlElementAst, context: any): any {
     let name = this._index++;

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

@@ -67,8 +67,9 @@ export function main() {
 
     it('should handle interpolation', () => {
       let translations: {[key: string]: string} = {};
-      translations[id(new Message('<ph name="0"/> and <ph name="1"/>', null, null))] =
+      translations[id(new Message(
-          '<ph name="1"/> or <ph name="0"/>';
+          '<ph name="INTERPOLATION_0"/> and <ph name="INTERPOLATION_1"/>', null, null))] =
+          '<ph name="INTERPOLATION_1"/> or <ph name="INTERPOLATION_0"/>';
 
       expect(humanizeDom(parse('<div value=\'{{a}} and {{b}}\' i18n-value></div>', translations)))
           .toEqual([[HtmlElementAst, 'div', 0], [HtmlAttrAst, 'value', '{{b}} or {{a}}']]);
@@ -76,8 +77,9 @@ export function main() {
 
     it('should handle interpolation with config', () => {
       let translations: {[key: string]: string} = {};
-      translations[id(new Message('<ph name="0"/> and <ph name="1"/>', null, null))] =
+      translations[id(new Message(
-          '<ph name="1"/> or <ph name="0"/>';
+          '<ph name="INTERPOLATION_0"/> and <ph name="INTERPOLATION_1"/>', null, null))] =
+          '<ph name="INTERPOLATION_1"/> or <ph name="INTERPOLATION_0"/>';
 
       expect(humanizeDom(parse(
                  '<div value=\'{%a%} and {%b%}\' i18n-value></div>', translations, [], {},
@@ -135,8 +137,9 @@ export function main() {
     it('should support interpolation', () => {
       let translations: {[key: string]: string} = {};
       translations[id(new Message(
-          '<ph name="e0">a</ph><ph name="e2"><ph name="t3">b<ph name="0"/></ph></ph>', null,
+          '<ph name="e0">a</ph><ph name="e2"><ph name="t3">b<ph name="INTERPOLATION_0"/></ph></ph>',
-          null))] = '<ph name="e2"><ph name="t3"><ph name="0"/>B</ph></ph><ph name="e0">A</ph>';
+          null, null))] =
+          '<ph name="e2"><ph name="t3"><ph name="INTERPOLATION_0"/>B</ph></ph><ph name="e0">A</ph>';
       expect(humanizeDom(parse('<div i18n><a>a</a><b>b{{i}}</b></div>', translations))).toEqual([
         [HtmlElementAst, 'div', 0],
         [HtmlElementAst, 'b', 1],
@@ -237,11 +240,12 @@ export function main() {
 
       it('should error when the translation refers to an invalid expression', () => {
         let translations: {[key: string]: string} = {};
-        translations[id(new Message('hi <ph name="0"/>', null, null))] = 'hi <ph name="99"/>';
+        translations[id(new Message('hi <ph name="INTERPOLATION_0"/>', null, null))] =
+            'hi <ph name="INTERPOLATION_99"/>';
 
         expect(
             humanizeErrors(parse('<div value=\'hi {{a}}\' i18n-value></div>', translations).errors))
-            .toEqual(['Invalid interpolation name \'99\'']);
+            .toEqual(['Invalid interpolation name \'INTERPOLATION_99\'']);
       });
     });
 

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

@@ -82,14 +82,15 @@ export function main() {
     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(
-          '<ph name="t0">Hi <ph name="0"/> and <ph name="1"/></ph>', null, null)]);
+          '<ph name="t0">Hi <ph name="INTERPOLATION_0"/> and <ph name="INTERPOLATION_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 <ph name="0"/> and <ph name="1"/>', null, null)]);
+          'Hi <ph name="INTERPOLATION_0"/> and <ph name="INTERPOLATION_1"/>', null, null)]);
     });
 
     it('should replace interpolation with named placeholders if provided (text nodes)', () => {
@@ -142,7 +143,7 @@ export function main() {
       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>',
+          '<ph name="e0"><ph name="t1">zero<ph name="INTERPOLATION_0"/></ph><ph name="e2"><ph name="t3"><ph name="INTERPOLATION_0"/></ph></ph></ph>',
           null, null)]);
     });