feat(I18N): generate error on unknown cases

fixes angular#9094
vicb · Jun 10, 2016 · 3f6e107 · 3f6e107
1 parent 5c21225
commit 3f6e107
Show file tree

Hide file tree

Showing 8 changed files with 206 additions and 233 deletions.
diff --git a/modules/@angular/compiler/src/html_lexer.ts b/modules/@angular/compiler/src/html_lexer.ts
@@ -155,18 +155,21 @@ class _HtmlTokenizer {
           } else {
             this._consumeTagOpen(start);
           }
-        } else if (_isExpansionFormStart(this._peek, this._nextPeek) && this.tokenizeExpansionForms) {
+        } else if (
+            _isExpansionFormStart(this._peek, this._nextPeek) && this.tokenizeExpansionForms) {
           this._consumeExpansionFormStart();
 
-        } else if ((this._peek === $EQ || (isAsciiLetter(this._peek) && this._isInExpansionForm()) && this.tokenizeExpansionForms)) {
+        } else if ((this._peek === $EQ ||
+                    (isAsciiLetter(this._peek) && this._isInExpansionForm()) &&
+                        this.tokenizeExpansionForms)) {
           this._consumeExpansionCaseStart();
 
-        } else if (this._peek === $RBRACE && this._isInExpansionCase() &&
-                   this.tokenizeExpansionForms) {
+        } else if (
+            this._peek === $RBRACE && this._isInExpansionCase() && this.tokenizeExpansionForms) {
           this._consumeExpansionCaseEnd();
 
-        } else if (this._peek === $RBRACE && this._isInExpansionForm() &&
-                   this.tokenizeExpansionForms) {
+        } else if (
+            this._peek === $RBRACE && this._isInExpansionForm() && this.tokenizeExpansionForms) {
           this._consumeExpansionFormEnd();
 
         } else {
@@ -211,8 +214,8 @@ class _HtmlTokenizer {
     if (isBlank(end)) {
       end = this._getLocation();
     }
-    var token = new HtmlToken(this._currentTokenType, parts,
-                              new ParseSourceSpan(this._currentTokenStart, end));
+    var token = new HtmlToken(
+        this._currentTokenType, parts, new ParseSourceSpan(this._currentTokenStart, end));
     this.tokens.push(token);
     this._currentTokenStart = null;
     this._currentTokenType = null;
@@ -237,9 +240,11 @@ class _HtmlTokenizer {
       this._column++;
     }
     this._index++;
-    this._peek = this._index >= this._length ? $EOF : StringWrapper.charCodeAt(this._input, this._index);
-    this._nextPeek =
-        this._index + 1 >= this._length ? $EOF : StringWrapper.charCodeAt(this._input, this._index + 1);
+    this._peek =
+        this._index >= this._length ? $EOF : StringWrapper.charCodeAt(this._input, this._index);
+    this._nextPeek = this._index + 1 >= this._length ?
+        $EOF :
+        StringWrapper.charCodeAt(this._input, this._index + 1);
   }
 
   private _attemptCharCode(charCode: number): boolean {
@@ -261,8 +266,8 @@ class _HtmlTokenizer {
   private _requireCharCode(charCode: number) {
     var location = this._getLocation();
     if (!this._attemptCharCode(charCode)) {
-      throw this._createError(unexpectedCharacterErrorMsg(this._peek),
-                              this._getSpan(location, location));
+      throw this._createError(
+          unexpectedCharacterErrorMsg(this._peek), this._getSpan(location, location));
     }
   }
 
@@ -653,14 +658,14 @@ class _HtmlTokenizer {
 
   private _isInExpansionCase(): boolean {
     return this._expansionCaseStack.length > 0 &&
-           this._expansionCaseStack[this._expansionCaseStack.length - 1] ===
-               HtmlTokenType.EXPANSION_CASE_EXP_START;
+        this._expansionCaseStack[this._expansionCaseStack.length - 1] ===
+        HtmlTokenType.EXPANSION_CASE_EXP_START;
   }
 
   private _isInExpansionForm(): boolean {
     return this._expansionCaseStack.length > 0 &&
-           this._expansionCaseStack[this._expansionCaseStack.length - 1] ===
-               HtmlTokenType.EXPANSION_FORM_START;
+        this._expansionCaseStack[this._expansionCaseStack.length - 1] ===
+        HtmlTokenType.EXPANSION_FORM_START;
   }
 }
 

diff --git a/modules/@angular/compiler/src/i18n/expander.ts b/modules/@angular/compiler/src/i18n/expander.ts
@@ -1,7 +1,10 @@
 import {BaseException} from '../facade/exceptions';
 import {HtmlAst, HtmlAstVisitor, HtmlAttrAst, HtmlCommentAst, HtmlElementAst, HtmlExpansionAst, HtmlExpansionCaseAst, HtmlTextAst, htmlVisitAll} from '../html_ast';
+import {ParseError} from '../parse_util';
+import {I18nError} from './shared';
 
-
+// http://cldr.unicode.org/index/cldr-spec/plural-rules
+const PLURAL_CASES: string[] = ['zero', 'one', 'two', 'few', 'many', 'other'];
 
 /**
  * Expands special forms into elements.
@@ -20,25 +23,25 @@ import {HtmlAst, HtmlAstVisitor, HtmlAttrAst, HtmlCommentAst, HtmlElementAst, Ht
  *
  * ```
  * <ul [ngPlural]="messages.length">
- *   <template [ngPluralCase]="0"><li i18n="plural_0">zero</li></template>
- *   <template [ngPluralCase]="1"><li i18n="plural_1">one</li></template>
- *   <template [ngPluralCase]="other"><li i18n="plural_other">more than one</li></template>
+ *   <template [ngPluralCase]="'=0'"><li i18n="plural_=0">zero</li></template>
+ *   <template [ngPluralCase]="'=1'"><li i18n="plural_=1">one</li></template>
+ *   <template [ngPluralCase]="'other'"><li i18n="plural_other">more than one</li></template>
  * </ul>
  * ```
  */
 export function expandNodes(nodes: HtmlAst[]): ExpansionResult {
   let e = new _Expander();
   let n = htmlVisitAll(e, nodes);
-  return new ExpansionResult(n, e.expanded);
+  return new ExpansionResult(n, e.expanded, e.errors);
 }
 
 export class ExpansionResult {
-  constructor(public nodes: HtmlAst[], public expanded: boolean) {}
+  constructor(public nodes: HtmlAst[], public expanded: boolean, public errors: ParseError[]) {}
 }
 
 class _Expander implements HtmlAstVisitor {
   expanded: boolean = false;
-  constructor() {}
+  errors: ParseError[] = [];
 
   visitElement(ast: HtmlElementAst, context: any): any {
     return new HtmlElementAst(
@@ -54,17 +57,23 @@ class _Expander implements HtmlAstVisitor {
 
   visitExpansion(ast: HtmlExpansionAst, context: any): any {
     this.expanded = true;
-    return ast.type == 'plural' ? _expandPluralForm(ast) : _expandDefaultForm(ast);
+    return ast.type == 'plural' ? _expandPluralForm(ast, this.errors) : _expandDefaultForm(ast);
   }
 
   visitExpansionCase(ast: HtmlExpansionCaseAst, context: any): any {
     throw new BaseException('Should not be reached');
   }
 }
 
-function _expandPluralForm(ast: HtmlExpansionAst): HtmlElementAst {
+function _expandPluralForm(ast: HtmlExpansionAst, errors: ParseError[]): HtmlElementAst {
   let children = ast.cases.map(c => {
+    if (PLURAL_CASES.indexOf(c.value) == -1 && !c.value.match(/^=\d+$/)) {
+      errors.push(new I18nError(
+          c.valueSourceSpan,
+          `Plural cases should be "=<number>" or one of ${PLURAL_CASES.join(", ")}`));
+    }
     let expansionResult = expandNodes(c.expression);
+    expansionResult.errors.forEach(e => errors.push(e));
     let i18nAttrs = expansionResult.expanded ?
         [] :
         [new HtmlAttrAst('i18n', `${ast.type}_${c.value}`, c.valueSourceSpan)];

diff --git a/modules/@angular/compiler/src/i18n/i18n_html_parser.ts b/modules/@angular/compiler/src/i18n/i18n_html_parser.ts
@@ -111,7 +111,10 @@ export class I18nHtmlParser implements HtmlParser {
     if (res.errors.length > 0) {
       return res;
     } else {
-      let nodes = this._recurse(expandNodes(res.rootNodes).nodes);
+      let expanded = expandNodes(res.rootNodes);
+      let nodes = this._recurse(expanded.nodes);
+      this.errors = this.errors.concat(expanded.errors);
+
       return this.errors.length > 0 ? new HtmlParseTreeResult([], this.errors) :
                                       new HtmlParseTreeResult(nodes, []);
     }

diff --git a/modules/@angular/compiler/src/i18n/message_extractor.ts b/modules/@angular/compiler/src/i18n/message_extractor.ts
@@ -19,14 +19,6 @@ export class ExtractionResult {
 
 /**
  * Removes duplicate messages.
- *
- * E.g.
- *
- * ```
- *  var m = [new Message("message", "meaning", "desc1"), new Message("message", "meaning",
- * "desc2")];
- *  expect(removeDuplicates(m)).toEqual([new Message("message", "meaning", "desc1")]);
- * ```
  */
 export function removeDuplicates(messages: Message[]): Message[] {
   let uniq: {[key: string]: Message} = {};
@@ -113,8 +105,9 @@ export class MessageExtractor {
     if (res.errors.length > 0) {
       return new ExtractionResult([], res.errors);
     } else {
-      this._recurse(expandNodes(res.rootNodes).nodes);
-      return new ExtractionResult(this.messages, this.errors);
+      let expanded = expandNodes(res.rootNodes);
+      this._recurse(expanded.nodes);
+      return new ExtractionResult(this.messages, this.errors.concat(expanded.errors));
     }
   }
 

diff --git a/modules/@angular/compiler/test/html_lexer_spec.ts b/modules/@angular/compiler/test/html_lexer_spec.ts
@@ -485,98 +485,65 @@ export function main() {
 
     });
 
-    describe("expansion forms", () => {
-      it("should parse an expansion form", () => {
+    describe('expansion forms', () => {
+      it('should parse an expansion form', () => {
         expect(tokenizeAndHumanizeParts('{one.two, three, =4 {four} =5 {five} foo {bar} }', true))
             .toEqual([
-              [HtmlTokenType.EXPANSION_FORM_START],
-              [HtmlTokenType.RAW_TEXT, 'one.two'],
-              [HtmlTokenType.RAW_TEXT, 'three'],
-              [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_START],
-              [HtmlTokenType.TEXT, 'four'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_END],
-              [HtmlTokenType.EXPANSION_CASE_VALUE, '=5'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_START],
-              [HtmlTokenType.TEXT, 'five'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_END],
-              [HtmlTokenType.EXPANSION_CASE_VALUE, 'foo'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_START],
-              [HtmlTokenType.TEXT, 'bar'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_END],
-              [HtmlTokenType.EXPANSION_FORM_END],
+              [HtmlTokenType.EXPANSION_FORM_START], [HtmlTokenType.RAW_TEXT, 'one.two'],
+              [HtmlTokenType.RAW_TEXT, 'three'], [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
+              [HtmlTokenType.EXPANSION_CASE_EXP_START], [HtmlTokenType.TEXT, 'four'],
+              [HtmlTokenType.EXPANSION_CASE_EXP_END], [HtmlTokenType.EXPANSION_CASE_VALUE, '=5'],
+              [HtmlTokenType.EXPANSION_CASE_EXP_START], [HtmlTokenType.TEXT, 'five'],
+              [HtmlTokenType.EXPANSION_CASE_EXP_END], [HtmlTokenType.EXPANSION_CASE_VALUE, 'foo'],
+              [HtmlTokenType.EXPANSION_CASE_EXP_START], [HtmlTokenType.TEXT, 'bar'],
+              [HtmlTokenType.EXPANSION_CASE_EXP_END], [HtmlTokenType.EXPANSION_FORM_END],
               [HtmlTokenType.EOF]
             ]);
       });
 
-      it("should parse an expansion form with text elements surrounding it", () => {
-        expect(tokenizeAndHumanizeParts('before{one.two, three, =4 {four}}after', true))
-            .toEqual([
-              [HtmlTokenType.TEXT, "before"],
-              [HtmlTokenType.EXPANSION_FORM_START],
-              [HtmlTokenType.RAW_TEXT, 'one.two'],
-              [HtmlTokenType.RAW_TEXT, 'three'],
-              [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_START],
-              [HtmlTokenType.TEXT, 'four'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_END],
-              [HtmlTokenType.EXPANSION_FORM_END],
-              [HtmlTokenType.TEXT, "after"],
-              [HtmlTokenType.EOF]
-            ]);
+      it('should parse an expansion form with text elements surrounding it', () => {
+        expect(tokenizeAndHumanizeParts('before{one.two, three, =4 {four}}after', true)).toEqual([
+          [HtmlTokenType.TEXT, 'before'], [HtmlTokenType.EXPANSION_FORM_START],
+          [HtmlTokenType.RAW_TEXT, 'one.two'], [HtmlTokenType.RAW_TEXT, 'three'],
+          [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'], [HtmlTokenType.EXPANSION_CASE_EXP_START],
+          [HtmlTokenType.TEXT, 'four'], [HtmlTokenType.EXPANSION_CASE_EXP_END],
+          [HtmlTokenType.EXPANSION_FORM_END], [HtmlTokenType.TEXT, 'after'], [HtmlTokenType.EOF]
+        ]);
       });
 
-      it("should parse an expansion forms with elements in it", () => {
-        expect(tokenizeAndHumanizeParts('{one.two, three, =4 {four <b>a</b>}}', true))
-            .toEqual([
-              [HtmlTokenType.EXPANSION_FORM_START],
-              [HtmlTokenType.RAW_TEXT, 'one.two'],
-              [HtmlTokenType.RAW_TEXT, 'three'],
-              [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_START],
-              [HtmlTokenType.TEXT, 'four '],
-              [HtmlTokenType.TAG_OPEN_START, null, 'b'],
-              [HtmlTokenType.TAG_OPEN_END],
-              [HtmlTokenType.TEXT, 'a'],
-              [HtmlTokenType.TAG_CLOSE, null, 'b'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_END],
-              [HtmlTokenType.EXPANSION_FORM_END],
-              [HtmlTokenType.EOF]
-            ]);
+      it('should parse an expansion forms with elements in it', () => {
+        expect(tokenizeAndHumanizeParts('{one.two, three, =4 {four <b>a</b>}}', true)).toEqual([
+          [HtmlTokenType.EXPANSION_FORM_START], [HtmlTokenType.RAW_TEXT, 'one.two'],
+          [HtmlTokenType.RAW_TEXT, 'three'], [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
+          [HtmlTokenType.EXPANSION_CASE_EXP_START], [HtmlTokenType.TEXT, 'four '],
+          [HtmlTokenType.TAG_OPEN_START, null, 'b'], [HtmlTokenType.TAG_OPEN_END],
+          [HtmlTokenType.TEXT, 'a'], [HtmlTokenType.TAG_CLOSE, null, 'b'],
+          [HtmlTokenType.EXPANSION_CASE_EXP_END], [HtmlTokenType.EXPANSION_FORM_END],
+          [HtmlTokenType.EOF]
+        ]);
       });
 
-      it("should parse an expansion forms with interpolation in it", () => {
-        expect(tokenizeAndHumanizeParts('{one.two, three, =4 {four {{a}}}}', true))
-            .toEqual([
-              [HtmlTokenType.EXPANSION_FORM_START],
-              [HtmlTokenType.RAW_TEXT, 'one.two'],
-              [HtmlTokenType.RAW_TEXT, 'three'],
-              [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_START],
-              [HtmlTokenType.TEXT, 'four {{a}}'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_END],
-              [HtmlTokenType.EXPANSION_FORM_END],
-              [HtmlTokenType.EOF]
-            ]);
+      it('should parse an expansion forms with interpolation in it', () => {
+        expect(tokenizeAndHumanizeParts('{one.two, three, =4 {four {{a}}}}', true)).toEqual([
+          [HtmlTokenType.EXPANSION_FORM_START], [HtmlTokenType.RAW_TEXT, 'one.two'],
+          [HtmlTokenType.RAW_TEXT, 'three'], [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
+          [HtmlTokenType.EXPANSION_CASE_EXP_START], [HtmlTokenType.TEXT, 'four {{a}}'],
+          [HtmlTokenType.EXPANSION_CASE_EXP_END], [HtmlTokenType.EXPANSION_FORM_END],
+          [HtmlTokenType.EOF]
+        ]);
       });
 
-      it("should parse nested expansion forms", () => {
+      it('should parse nested expansion forms', () => {
         expect(tokenizeAndHumanizeParts(`{one.two, three, =4 { {xx, yy, =x {one}} }}`, true))
             .toEqual([
-              [HtmlTokenType.EXPANSION_FORM_START],
-              [HtmlTokenType.RAW_TEXT, 'one.two'],
-              [HtmlTokenType.RAW_TEXT, 'three'],
-              [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
+              [HtmlTokenType.EXPANSION_FORM_START], [HtmlTokenType.RAW_TEXT, 'one.two'],
+              [HtmlTokenType.RAW_TEXT, 'three'], [HtmlTokenType.EXPANSION_CASE_VALUE, '=4'],
               [HtmlTokenType.EXPANSION_CASE_EXP_START],
 
-              [HtmlTokenType.EXPANSION_FORM_START],
-              [HtmlTokenType.RAW_TEXT, 'xx'],
-              [HtmlTokenType.RAW_TEXT, 'yy'],
-              [HtmlTokenType.EXPANSION_CASE_VALUE, '=x'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_START],
-              [HtmlTokenType.TEXT, 'one'],
-              [HtmlTokenType.EXPANSION_CASE_EXP_END],
-              [HtmlTokenType.EXPANSION_FORM_END],
+              [HtmlTokenType.EXPANSION_FORM_START], [HtmlTokenType.RAW_TEXT, 'xx'],
+              [HtmlTokenType.RAW_TEXT, 'yy'], [HtmlTokenType.EXPANSION_CASE_VALUE, '=x'],
+              [HtmlTokenType.EXPANSION_CASE_EXP_START], [HtmlTokenType.TEXT, 'one'],
+              [HtmlTokenType.EXPANSION_CASE_EXP_END], [HtmlTokenType.EXPANSION_FORM_END],
               [HtmlTokenType.TEXT, ' '],
 
               [HtmlTokenType.EXPANSION_CASE_EXP_END], [HtmlTokenType.EXPANSION_FORM_END],