angular · kara · Apr 14, 2016
diff --git a/modules/angular2/src/compiler/expression_parser/lexer.ts b/modules/angular2/src/compiler/expression_parser/lexer.ts
@@ -145,7 +145,7 @@ export const $BACKSLASH = 92;
 export const $RBRACKET = 93;
 const $CARET = 94;
 const $_ = 95;
-
+export const $BT = 96;
 const $a = 97, $e = 101, $f = 102, $n = 110, $r = 114, $t = 116, $u = 117, $v = 118, $z = 122;
 
 export const $LBRACE = 123;
@@ -415,6 +415,10 @@ function isExponentSign(code: number): boolean {
   return code == $MINUS || code == $PLUS;
 }
 
+export function isQuote(code: number): boolean {
+  return code === $SQ || code === $DQ || code === $BT;
+}
+
 function unescape(code: number): number {
   switch (code) {
     case $n:

diff --git a/modules/angular2/src/compiler/expression_parser/parser.ts b/modules/angular2/src/compiler/expression_parser/parser.ts
@@ -6,6 +6,7 @@ import {
   Lexer,
   EOF,
   isIdentifier,
+  isQuote,
   Token,
   $PERIOD,
   $COLON,
@@ -16,7 +17,8 @@ import {
   $LBRACE,
   $RBRACE,
   $LPAREN,
-  $RPAREN
+  $RPAREN,
+  $SLASH
 } from './lexer';
 import {
   AST,
@@ -67,7 +69,7 @@ export class Parser {
 
   parseAction(input: string, location: any): ASTWithSource {
     this._checkNoInterpolation(input, location);
-    var tokens = this._lexer.tokenize(input);
+    var tokens = this._lexer.tokenize(this._stripComments(input));
     var ast = new _ParseAST(input, location, tokens, true).parseChain();
     return new ASTWithSource(ast, input, location);
   }
@@ -96,7 +98,7 @@ export class Parser {
     }
 
     this._checkNoInterpolation(input, location);
-    var tokens = this._lexer.tokenize(input);
+    var tokens = this._lexer.tokenize(this._stripComments(input));
     return new _ParseAST(input, location, tokens, false).parseChain();
   }
 
@@ -122,7 +124,7 @@ export class Parser {
     let expressions = [];
 
     for (let i = 0; i < split.expressions.length; ++i) {
-      var tokens = this._lexer.tokenize(split.expressions[i]);
+      var tokens = this._lexer.tokenize(this._stripComments(split.expressions[i]));
       var ast = new _ParseAST(input, location, tokens, false).parseChain();
       expressions.push(ast);
     }
@@ -158,6 +160,28 @@ export class Parser {
     return new ASTWithSource(new LiteralPrimitive(input), input, location);
   }
 
+  private _stripComments(input: string): string {
+    let i = this._commentStart(input);
+    return isPresent(i) ? input.substring(0, i).trim() : input;
+  }
+
+  private _commentStart(input: string): number {
+    var outerQuote = null;
+    for (var i = 0; i < input.length - 1; i++) {
+      let char = StringWrapper.charCodeAt(input, i);
+      let nextChar = StringWrapper.charCodeAt(input, i + 1);
+
+      if (char === $SLASH && nextChar == $SLASH && isBlank(outerQuote)) return i;
+
+      if (outerQuote === char) {
+        outerQuote = null;
+      } else if (isBlank(outerQuote) && isQuote(char)) {
+        outerQuote = char;
+      }
+    }
+    return null;
+  }
+
   private _checkNoInterpolation(input: string, location: any): void {
     var parts = StringWrapper.split(input, INTERPOLATION_REGEXP);
     if (parts.length > 1) {

diff --git a/modules/angular2/src/i18n/i18n_html_parser.ts b/modules/angular2/src/i18n/i18n_html_parser.ts
@@ -22,14 +22,16 @@ import {
   partition,
   Part,
   stringifyNodes,
-  meaning
+  meaning,
+  getPhNameFromBinding,
+  dedupePhName
 } from './shared';
 
 const _I18N_ATTR = "i18n";
 const _PLACEHOLDER_ELEMENT = "ph";
 const _NAME_ATTR = "name";
 const _I18N_ATTR_PREFIX = "i18n-";
-let _PLACEHOLDER_EXPANDED_REGEXP = RegExpWrapper.create(`\\<ph(\\s)+name=("(\\d)+")\\>\\<\\/ph\\>`);
+let _PLACEHOLDER_EXPANDED_REGEXP = RegExpWrapper.create(`\\<ph(\\s)+name=("(\\w)+")\\>\\<\\/ph\\>`);
 
 /**
  * Creates an i18n-ed version of the parsed template.
@@ -313,19 +315,31 @@ export class I18nHtmlParser implements HtmlParser {
 
   private _replacePlaceholdersWithExpressions(message: string, exps: string[],
                                               sourceSpan: ParseSourceSpan): string {
+    let expMap = this._buildExprMap(exps);
     return RegExpWrapper.replaceAll(_PLACEHOLDER_EXPANDED_REGEXP, message, (match) => {
       let nameWithQuotes = match[2];
       let name = nameWithQuotes.substring(1, nameWithQuotes.length - 1);
-      let index = NumberWrapper.parseInt(name, 10);
-      return this._convertIntoExpression(index, exps, sourceSpan);
+      return this._convertIntoExpression(name, expMap, sourceSpan);
     });
   }
 
-  private _convertIntoExpression(index: number, exps: string[], sourceSpan: ParseSourceSpan) {
-    if (index >= 0 && index < exps.length) {
-      return `{{${exps[index]}}}`;
+  private _buildExprMap(exps: string[]): Map<string, string> {
+    let expMap = new Map<string, string>();
+    let usedNames = new Map<string, number>();
+
+    for (var i = 0; i < exps.length; i++) {
+      let phName = getPhNameFromBinding(exps[i], i);
+      expMap.set(dedupePhName(usedNames, phName), exps[i]);
+    }
+    return expMap;
+  }
+
+  private _convertIntoExpression(name: string, expMap: Map<string, string>,
+                                 sourceSpan: ParseSourceSpan) {
+    if (expMap.has(name)) {
+      return `{{${expMap.get(name)}}}`;
     } else {
-      throw new I18nError(sourceSpan, `Invalid interpolation index '${index}'`);
+      throw new I18nError(sourceSpan, `Invalid interpolation name '${name}'`);
     }
   }
 }
@@ -347,4 +361,4 @@ class _CreateNodeMapping implements HtmlAstVisitor {
   }
 
   visitComment(ast: HtmlCommentAst, context: any): any { return ""; }
-}
+}
diff --git a/modules/angular2/src/i18n/shared.ts b/modules/angular2/src/i18n/shared.ts
@@ -8,12 +8,13 @@ import {
   HtmlCommentAst,
   htmlVisitAll
 } from 'angular2/src/compiler/html_ast';
-import {isPresent, isBlank} from 'angular2/src/facade/lang';
+import {isPresent, isBlank, StringWrapper} from 'angular2/src/facade/lang';
 import {Message} from './message';
 import {Parser} from 'angular2/src/compiler/expression_parser/parser';
 
 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;
 
 /**
  * An i18n error.
@@ -113,12 +114,15 @@ export function removeInterpolation(value: string, source: ParseSourceSpan,
                                     parser: Parser): string {
   try {
     let parsed = parser.splitInterpolation(value, source.toString());
+    let 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) {
-          res += `<ph name="${i}"/>`;
+          let customPhName = getPhNameFromBinding(parsed.expressions[i], i);
+          customPhName = dedupePhName(usedNames, customPhName);
+          res += `<ph name="${customPhName}"/>`;
         }
       }
       return res;
@@ -130,6 +134,22 @@ export function removeInterpolation(value: string, source: ParseSourceSpan,
   }
 }
 
+export function getPhNameFromBinding(input: string, index: number): string {
+  let customPhMatch = StringWrapper.split(input, CUSTOM_PH_EXP);
+  return customPhMatch.length > 1 ? customPhMatch[1] : `${index}`;
+}
+
+export function dedupePhName(usedNames: Map<string, number>, name: string): string {
+  let duplicateNameCount = usedNames.get(name);
+  if (isPresent(duplicateNameCount)) {
+    usedNames.set(name, duplicateNameCount + 1);
+    return `${name}_${duplicateNameCount}`;
+  } else {
+    usedNames.set(name, 1);
+    return name;
+  }
+}
+
 export function stringifyNodes(nodes: HtmlAst[], parser: Parser): string {
   let visitor = new _StringifyVisitor(parser);
   return htmlVisitAll(visitor, nodes).join("");

diff --git a/modules/angular2/test/compiler/expression_parser/parser_spec.ts b/modules/angular2/test/compiler/expression_parser/parser_spec.ts
@@ -103,6 +103,11 @@ export function main() {
 
       it('should parse grouped expressions', () => { checkAction("(1 + 2) * 3", "1 + 2 * 3"); });
 
+      it('should ignore comments in expressions', () => { checkAction('a //comment', 'a'); });
+
+      it('should retain // in string literals',
+         () => { checkAction(`"http://www.google.com"`, `"http://www.google.com"`); });
+
       it('should parse an empty string', () => { checkAction(''); });
 
       describe("literals", () => {
@@ -269,6 +274,14 @@ export function main() {
       });
 
       it('should parse conditional expression', () => { checkBinding('a < b ? a : b'); });
+
+      it('should ignore comments in bindings', () => { checkBinding('a //comment', 'a'); });
+
+      it('should retain // in string literals',
+         () => { checkBinding(`"http://www.google.com"`, `"http://www.google.com"`); });
+
+      it('should retain // in : microsyntax', () => { checkBinding('one:a//b', 'one:a//b'); });
+
     });
 
     describe('parseTemplateBindings', () => {
@@ -424,6 +437,31 @@ export function main() {
       it('should parse expression with newline characters', () => {
         checkInterpolation(`{{ 'foo' +\n 'bar' +\r 'baz' }}`, `{{ "foo" + "bar" + "baz" }}`);
       });
+
+      describe("comments", () => {
+        it('should ignore comments in interpolation expressions',
+           () => { checkInterpolation('{{a //comment}}', '{{ a }}'); });
+
+        it('should retain // in single quote strings', () => {
+          checkInterpolation(`{{ 'http://www.google.com' }}`, `{{ "http://www.google.com" }}`);
+        });
+
+        it('should retain // in double quote strings', () => {
+          checkInterpolation(`{{ "http://www.google.com" }}`, `{{ "http://www.google.com" }}`);
+        });
+
+        it('should ignore comments after string literals',
+           () => { checkInterpolation(`{{ "a//b" //comment }}`, `{{ "a//b" }}`); });
+
+        it('should retain // in complex strings', () => {
+          checkInterpolation(`{{"//a\'//b\`//c\`//d\'//e" //comment}}`, `{{ "//a\'//b\`//c\`//d\'//e" }}`);
+        });
+
+        it('should retain // in nested, unterminated strings', () => {
+          checkInterpolation(`{{ "a\'b\`" //comment}}`, `{{ "a\'b\`" }}`);
+        });
+      });
+
     });
 
     describe("parseSimpleBinding", () => {

diff --git a/modules/angular2/test/i18n/i18n_html_parser_spec.ts b/modules/angular2/test/i18n/i18n_html_parser_spec.ts
@@ -76,6 +76,36 @@ export function main() {
           .toEqual([[HtmlElementAst, 'div', 0], [HtmlAttrAst, 'value', '{{b}} or {{a}}']]);
     });
 
+    it('should handle interpolation with custom placeholder names', () => {
+      let translations: {[key: string]: string} = {};
+      translations[id(new Message('<ph name="FIRST"/> and <ph name="SECOND"/>', null, null))] =
+          '<ph name="SECOND"/> or <ph name="FIRST"/>';
+
+      expect(
+          humanizeDom(parse(
+              `<div value='{{a //i18n(ph="FIRST")}} and {{b //i18n(ph="SECOND")}}' i18n-value></div>`,
+              translations)))
+          .toEqual([
+            [HtmlElementAst, 'div', 0],
+            [HtmlAttrAst, 'value', '{{b //i18n(ph="SECOND")}} or {{a //i18n(ph="FIRST")}}']
+          ]);
+    });
+
+    it('should handle interpolation with duplicate placeholder names', () => {
+      let translations: {[key: string]: string} = {};
+      translations[id(new Message('<ph name="FIRST"/> and <ph name="FIRST_1"/>', null, null))] =
+          '<ph name="FIRST_1"/> or <ph name="FIRST"/>';
+
+      expect(
+          humanizeDom(parse(
+              `<div value='{{a //i18n(ph="FIRST")}} and {{b //i18n(ph="FIRST")}}' i18n-value></div>`,
+              translations)))
+          .toEqual([
+            [HtmlElementAst, 'div', 0],
+            [HtmlAttrAst, 'value', '{{b //i18n(ph="FIRST")}} or {{a //i18n(ph="FIRST")}}']
+          ]);
+    });
+
     it("should handle nested html", () => {
       let translations: {[key: string]: string} = {};
       translations[id(new Message('<ph name="e0">a</ph><ph name="e2">b</ph>', null, null))] =
@@ -198,7 +228,7 @@ export function main() {
 
         expect(
             humanizeErrors(parse("<div value='hi {{a}}' i18n-value></div>", translations).errors))
-            .toEqual(["Invalid interpolation index '99'"]);
+            .toEqual(["Invalid interpolation name '99'"]);
       });
 
     });
@@ -207,4 +237,4 @@ export function main() {
 
 function humanizeErrors(errors: ParseError[]): string[] {
   return errors.map(error => error.msg);
-}
+}
diff --git a/modules/angular2/test/i18n/message_extractor_spec.ts b/modules/angular2/test/i18n/message_extractor_spec.ts
@@ -93,6 +93,47 @@ export function main() {
           .toEqual([new Message('Hi <ph name="0"/> and <ph name="1"/>', null, null)]);
     });
 
+    it('should replace interpolation with named placeholders if provided (text nodes)', () => {
+      let res = extractor.extract(`
+        <div i18n>Hi {{one //i18n(ph="FIRST")}} and {{two //i18n(ph="SECOND")}}</div>`,
+                                  'someurl');
+      expect(res.messages)
+          .toEqual([
+            new Message('<ph name="t0">Hi <ph name="FIRST"/> and <ph name="SECOND"/></ph>', null,
+                        null)
+          ]);
+    });
+
+    it('should replace interpolation with named placeholders if provided (attributes)', () => {
+      let res = extractor.extract(`
+      <div title='Hi {{one //i18n(ph="FIRST")}} and {{two //i18n(ph="SECOND")}}'
+        i18n-title></div>`,
+                                  'someurl');
+      expect(res.messages)
+          .toEqual([new Message('Hi <ph name="FIRST"/> and <ph name="SECOND"/>', null, null)]);
+    });
+
+    it('should match named placeholders with extra spacing', () => {
+      let res = extractor.extract(`
+      <div title='Hi {{one // i18n ( ph = "FIRST" )}} and {{two // i18n ( ph = "SECOND" )}}'
+        i18n-title></div>`,
+                                  'someurl');
+      expect(res.messages)
+          .toEqual([new Message('Hi <ph name="FIRST"/> and <ph name="SECOND"/>', null, null)]);
+    });
+
+    it('should suffix duplicate placeholder names with numbers', () => {
+      let res = extractor.extract(`
+      <div title='Hi {{one //i18n(ph="FIRST")}} and {{two //i18n(ph="FIRST")}} and {{three //i18n(ph="FIRST")}}'
+        i18n-title></div>`,
+                                  'someurl');
+      expect(res.messages)
+          .toEqual([
+            new Message('Hi <ph name="FIRST"/> and <ph name="FIRST_1"/> and <ph name="FIRST_2"/>',
+                        null, null)
+          ]);
+    });
+
     it("should handle html content", () => {
       let res = extractor.extract(
           '<div i18n><div attr="value">zero<div>one</div></div><div>two</div></div>', "someurl");