angular · JoostK · Sep 30, 2020 · Oct 1, 2020 · Sep 30, 2020
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
+import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstIcu, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {Reference} from '../../imports';
@@ -1333,6 +1333,8 @@ class Scope {
       this.checkAndAppendReferencesOfNode(node);
     } else if (node instanceof TmplAstBoundText) {
       this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, node));
+    } else if (node instanceof TmplAstIcu) {
+      this.appendIcuExpressions(node);
     }
   }
 
@@ -1459,6 +1461,17 @@ class Scope {
       this.appendDeepSchemaChecks(node.children);
     }
   }
+
+  private appendIcuExpressions(node: TmplAstIcu): void {
+    for (const variable of Object.values(node.vars)) {
+      this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, variable));
+    }
+    for (const placeholder of Object.values(node.placeholders)) {
+      if (placeholder instanceof TmplAstBoundText) {
+        this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, placeholder));
+      }
+    }
+  }
 }
 
 interface TcbBoundInput {

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts
@@ -176,6 +176,21 @@ runInEachFileSystem(() => {
       ]);
     });
 
+    it('checks expressions in ICUs', () => {
+      const messages = diagnose(
+          `<span i18n>{switch, plural, other { {{interpolation}}
+            {nestedSwitch, plural, other { {{nestedInterpolation}} }}
+          }}</span>`,
+          `class TestComponent {}`);
+
+      expect(messages.sort()).toEqual([
+        `TestComponent.html(1, 13): Property 'switch' does not exist on type 'TestComponent'.`,
+        `TestComponent.html(1, 39): Property 'interpolation' does not exist on type 'TestComponent'.`,
+        `TestComponent.html(2, 14): Property 'nestedSwitch' does not exist on type 'TestComponent'.`,
+        `TestComponent.html(2, 46): Property 'nestedInterpolation' does not exist on type 'TestComponent'.`,
+      ]);
+    });
+
     it('produces diagnostics for pipes', () => {
       const messages = diagnose(
           `<div>{{ person.name | pipe:person.age:1 }}</div>`, `

diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts
@@ -92,7 +92,7 @@ export {getParseErrors, isSyntaxError, syntaxError, Version} from './util';
 export {SourceMap} from './output/source_map';
 export * from './injectable_compiler_2';
 export * from './render3/view/api';
-export {BoundAttribute as TmplAstBoundAttribute, BoundEvent as TmplAstBoundEvent, BoundText as TmplAstBoundText, Content as TmplAstContent, Element as TmplAstElement, Node as TmplAstNode, RecursiveVisitor as TmplAstRecursiveVisitor, Reference as TmplAstReference, Template as TmplAstTemplate, Text as TmplAstText, TextAttribute as TmplAstTextAttribute, Variable as TmplAstVariable,} from './render3/r3_ast';
+export {BoundAttribute as TmplAstBoundAttribute, BoundEvent as TmplAstBoundEvent, BoundText as TmplAstBoundText, Content as TmplAstContent, Element as TmplAstElement, Icu as TmplAstIcu, Node as TmplAstNode, RecursiveVisitor as TmplAstRecursiveVisitor, Reference as TmplAstReference, Template as TmplAstTemplate, Text as TmplAstText, TextAttribute as TmplAstTextAttribute, Variable as TmplAstVariable} from './render3/r3_ast';
 export * from './render3/view/t2_api';
 export * from './render3/view/t2_binder';
 export {Identifiers as R3Identifiers} from './render3/r3_identifiers';

diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts
@@ -179,10 +179,33 @@ export class Parser {
       expressions.push(ast);
     }
 
-    const span = new ParseSpan(0, input == null ? 0 : input.length);
-    return new ASTWithSource(
-        new Interpolation(span, span.toAbsolute(absoluteOffset), split.strings, expressions), input,
-        location, absoluteOffset, this.errors);
+    return this.createInterpolationAst(split.strings, expressions, input, location, absoluteOffset);
+  }
+
+  /**
+   * Similar to `parseInterpolation`, but treats the provided string as a single expression
+   * element that would normally appear within the interpolation prefix and suffix (`{{` and `}}`).
+   * This is used for parsing the switch expression in ICUs.
+   */
+  parseInterpolationExpression(expression: string, location: any, absoluteOffset: number):
+      ASTWithSource {
+    const sourceToLex = this._stripComments(expression);
+    const tokens = this._lexer.tokenize(sourceToLex);
+    const ast = new _ParseAST(
+                    expression, location, absoluteOffset, tokens, sourceToLex.length,
+                    /* parseAction */ false, this.errors, 0)
+                    .parseChain();
+    const strings = ['', ''];  // The prefix and suffix strings are both empty
+    return this.createInterpolationAst(strings, [ast], expression, location, absoluteOffset);
+  }
+
+  private createInterpolationAst(
+      strings: string[], expressions: AST[], input: string, location: string,
+      absoluteOffset: number): ASTWithSource {
+    const span = new ParseSpan(0, input.length);
+    const interpolation =
+        new Interpolation(span, span.toAbsolute(absoluteOffset), strings, expressions);
+    return new ASTWithSource(interpolation, input, location, absoluteOffset, this.errors);
   }
 
   /**

diff --git a/packages/compiler/src/i18n/i18n_ast.ts b/packages/compiler/src/i18n/i18n_ast.ts
@@ -8,6 +8,18 @@
 
 import {ParseSourceSpan} from '../parse_util';
 
+/**
+ * Describes the text contents of a placeholder as it appears in an ICU expression, including its
+ * source span information.
+ */
+export interface MessagePlaceholder {
+  /** The text contents of the placeholder */
+  text: string;
+
+  /** The source span of the placeholder */
+  sourceSpan: ParseSourceSpan;
+}
+
 export class Message {
   sources: MessageSpan[];
   id: string = this.customId;
@@ -16,14 +28,14 @@ export class Message {
 
   /**
    * @param nodes message AST
-   * @param placeholders maps placeholder names to static content
+   * @param placeholders maps placeholder names to static content and their source spans
    * @param placeholderToMessage maps placeholder names to messages (used for nested ICU messages)
    * @param meaning
    * @param description
    * @param customId
    */
   constructor(
-      public nodes: Node[], public placeholders: {[phName: string]: string},
+      public nodes: Node[], public placeholders: {[phName: string]: MessagePlaceholder},
       public placeholderToMessage: {[phName: string]: Message}, public meaning: string,
       public description: string, public customId: string) {
     if (nodes.length) {

diff --git a/packages/compiler/src/i18n/i18n_parser.ts b/packages/compiler/src/i18n/i18n_parser.ts
@@ -39,7 +39,7 @@ interface I18nMessageVisitorContext {
   isIcu: boolean;
   icuDepth: number;
   placeholderRegistry: PlaceholderRegistry;
-  placeholderToContent: {[phName: string]: string};
+  placeholderToContent: {[phName: string]: i18n.MessagePlaceholder};
   placeholderToMessage: {[phName: string]: i18n.Message};
   visitNodeFn: VisitNodeFn;
 }
@@ -83,13 +83,19 @@ class _I18nVisitor implements html.Visitor {
     const isVoid: boolean = getHtmlTagDefinition(el.name).isVoid;
     const startPhName =
         context.placeholderRegistry.getStartTagPlaceholderName(el.name, attrs, isVoid);
-    context.placeholderToContent[startPhName] = el.startSourceSpan.toString();
+    context.placeholderToContent[startPhName] = {
+      text: el.startSourceSpan.toString(),
+      sourceSpan: el.startSourceSpan,
+    };
 
     let closePhName = '';
 
     if (!isVoid) {
       closePhName = context.placeholderRegistry.getCloseTagPlaceholderName(el.name);
-      context.placeholderToContent[closePhName] = `</${el.name}>`;
+      context.placeholderToContent[closePhName] = {
+        text: `</${el.name}>`,
+        sourceSpan: el.endSourceSpan ?? el.sourceSpan,
+      };
     }
 
     const node = new i18n.TagPlaceholder(
@@ -128,7 +134,10 @@ class _I18nVisitor implements html.Visitor {
       // - the ICU message is nested.
       const expPh = context.placeholderRegistry.getUniquePlaceholder(`VAR_${icu.type}`);
       i18nIcu.expressionPlaceholder = expPh;
-      context.placeholderToContent[expPh] = icu.switchValue;
+      context.placeholderToContent[expPh] = {
+        text: icu.switchValue,
+        sourceSpan: icu.switchValueSourceSpan,
+      };
       return context.visitNodeFn(icu, i18nIcu);
     }
 
@@ -175,7 +184,10 @@ class _I18nVisitor implements html.Visitor {
       const expressionSpan =
           getOffsetSourceSpan(sourceSpan, splitInterpolation.expressionsSpans[i]);
       nodes.push(new i18n.Placeholder(expression, phName, expressionSpan));
-      context.placeholderToContent[phName] = sDelimiter + expression + eDelimiter;
+      context.placeholderToContent[phName] = {
+        text: sDelimiter + expression + eDelimiter,
+        sourceSpan: expressionSpan,
+      };
     }
 
     // The last index contains no expression

diff --git a/packages/compiler/src/i18n/translation_bundle.ts b/packages/compiler/src/i18n/translation_bundle.ts
@@ -109,7 +109,7 @@ class I18nToHtmlVisitor implements i18n.Visitor {
     // TODO(vicb): Once all format switch to using expression placeholders
     // we should throw when the placeholder is not in the source message
     const exp = this._srcMsg.placeholders.hasOwnProperty(icu.expression) ?
-        this._srcMsg.placeholders[icu.expression] :
+        this._srcMsg.placeholders[icu.expression].text :
         icu.expression;
 
     return `{${exp}, ${icu.type}, ${cases.join(' ')}}`;
@@ -118,7 +118,7 @@ class I18nToHtmlVisitor implements i18n.Visitor {
   visitPlaceholder(ph: i18n.Placeholder, context?: any): string {
     const phName = this._mapper(ph.name);
     if (this._srcMsg.placeholders.hasOwnProperty(phName)) {
-      return this._srcMsg.placeholders[phName];
+      return this._srcMsg.placeholders[phName].text;
     }
 
     if (this._srcMsg.placeholderToMessage.hasOwnProperty(phName)) {

diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts
@@ -266,23 +266,18 @@ class HtmlAstToIvyAst implements html.Visitor {
     Object.keys(message.placeholders).forEach(key => {
       const value = message.placeholders[key];
       if (key.startsWith(I18N_ICU_VAR_PREFIX)) {
-        const config = this.bindingParser.interpolationConfig;
-
-        // ICU expression is a plain string, not wrapped into start
-        // and end tags, so we wrap it before passing to binding parser
-        const wrapped = `${config.start}${value}${config.end}`;
-
         // Currently when the `plural` or `select` keywords in an ICU contain trailing spaces (e.g.
         // `{count, select , ...}`), these spaces are also included into the key names in ICU vars
         // (e.g. "VAR_SELECT "). These trailing spaces are not desirable, since they will later be
         // converted into `_` symbols while normalizing placeholder names, which might lead to
         // mismatches at runtime (i.e. placeholder will not be replaced with the correct value).
         const formattedKey = key.trim();
 
-        vars[formattedKey] =
-            this._visitTextWithInterpolation(wrapped, expansion.sourceSpan) as t.BoundText;
+        const ast = this.bindingParser.parseInterpolationExpression(value.text, value.sourceSpan);
+
+        vars[formattedKey] = new t.BoundText(ast, value.sourceSpan);
       } else {
-        placeholders[key] = this._visitTextWithInterpolation(value, expansion.sourceSpan);
+        placeholders[key] = this._visitTextWithInterpolation(value.text, value.sourceSpan);
       }
     });
     return new t.Icu(vars, placeholders, expansion.sourceSpan, message);

diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts
@@ -127,6 +127,26 @@ export class BindingParser {
     }
   }
 
+  /**
+   * Similar to `parseInterpolation`, but treats the provided string as a single expression
+   * element that would normally appear within the interpolation prefix and suffix (`{{` and `}}`).
+   * This is used for parsing the switch expression in ICUs.
+   */
+  parseInterpolationExpression(expression: string, sourceSpan: ParseSourceSpan): ASTWithSource {
+    const sourceInfo = sourceSpan.start.toString();
+
+    try {
+      const ast = this._exprParser.parseInterpolationExpression(
+          expression, sourceInfo, sourceSpan.start.offset);
+      if (ast) this._reportExpressionParserErrors(ast.errors, sourceSpan);
+      this._checkPipes(ast, sourceSpan);
+      return ast;
+    } catch (e) {
+      this._reportError(`${e}`, sourceSpan);
+      return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, sourceSpan.start.offset);
+    }
+  }
+
   /**
    * Parses the bindings in a microsyntax expression, and converts them to
    * `ParsedProperty` or `ParsedVariable`.

diff --git a/packages/compiler/test/i18n/i18n_parser_spec.ts b/packages/compiler/test/i18n/i18n_parser_spec.ts
@@ -324,7 +324,7 @@ function _humanizePlaceholders(
   // clang-format off
   // https://github.com/angular/clang-format/issues/35
   return _extractMessages(html, implicitTags, implicitAttrs).map(
-    msg => Object.keys(msg.placeholders).map((name) => `${name}=${msg.placeholders[name]}`).join(', '));
+    msg => Object.keys(msg.placeholders).map((name) => `${name}=${msg.placeholders[name].text}`).join(', '));
   // clang-format on
 }
 

diff --git a/packages/compiler/test/i18n/translation_bundle_spec.ts b/packages/compiler/test/i18n/translation_bundle_spec.ts
@@ -50,7 +50,7 @@ import {_extractMessages} from './i18n_parser_spec';
         ]
       };
       const phMap = {
-        ph1: '*phContent*',
+        ph1: createPlaceholder('*phContent*'),
       };
       const tb = new TranslationBundle(msgMap, null, (_) => 'foo');
       const msg = new i18n.Message([srcNode], phMap, {}, 'm', 'd', 'i');
@@ -160,7 +160,7 @@ import {_extractMessages} from './i18n_parser_spec';
           ]
         };
         const phMap = {
-          ph1: '</b>',
+          ph1: createPlaceholder('</b>'),
         };
         const tb = new TranslationBundle(msgMap, null, (_) => 'foo');
         const msg = new i18n.Message([srcNode], phMap, {}, 'm', 'd', 'i');
@@ -169,3 +169,13 @@ import {_extractMessages} from './i18n_parser_spec';
     });
   });
 }
+
+function createPlaceholder(text: string): i18n.MessagePlaceholder {
+  const file = new ParseSourceFile(text, 'file://test');
+  const start = new ParseLocation(file, 0, 0, 0);
+  const end = new ParseLocation(file, text.length, 0, text.length);
+  return {
+    text,
+    sourceSpan: new ParseSourceSpan(start, end),
+  };
+}
diff --git a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts
@@ -339,4 +339,25 @@ describe('expression AST absolute source spans', () => {
               [['As', new AbsoluteSourceSpan(22, 24)], ['Bs', new AbsoluteSourceSpan(35, 37)]]));
     });
   });
+
+  describe('ICU expressions', () => {
+    it('is correct for variables and placeholders', () => {
+      const spans = humanizeExpressionSource(
+          parse('<span i18n>{item.var, plural, other { {{item.placeholder}} items } }</span>')
+              .nodes);
+      expect(spans).toContain(['item.var', new AbsoluteSourceSpan(12, 20)]);
+      expect(spans).toContain(['item.placeholder', new AbsoluteSourceSpan(40, 56)]);
+    });
+
+    it('is correct for variables and placeholders', () => {
+      const spans = humanizeExpressionSource(
+          parse(
+              '<span i18n>{item.var, plural, other { {{item.placeholder}} {nestedVar, plural, other { {{nestedPlaceholder}} }}} }</span>')
+              .nodes);
+      expect(spans).toContain(['item.var', new AbsoluteSourceSpan(12, 20)]);
+      expect(spans).toContain(['item.placeholder', new AbsoluteSourceSpan(40, 56)]);
+      expect(spans).toContain(['nestedVar', new AbsoluteSourceSpan(60, 69)]);
+      expect(spans).toContain(['nestedPlaceholder', new AbsoluteSourceSpan(89, 106)]);
+    });
+  });
 });
diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts
@@ -89,7 +89,13 @@ class R3AstSourceSpans implements t.Visitor<void> {
   }
 
   visitIcu(icu: t.Icu) {
-    return null;
+    this.result.push(['Icu', humanizeSpan(icu.sourceSpan)]);
+    for (const key of Object.keys(icu.vars)) {
+      this.result.push(['Icu:Var', humanizeSpan(icu.vars[key].sourceSpan)]);
+    }
+    for (const key of Object.keys(icu.placeholders)) {
+      this.result.push(['Icu:Placeholder', humanizeSpan(icu.placeholders[key].sourceSpan)]);
+    }
   }
 
   private visitAll(nodes: t.Node[][]) {
@@ -420,4 +426,40 @@ describe('R3 AST source spans', () => {
       ]);
     });
   });
+
+  describe('ICU expressions', () => {
+    it('is correct for variables and placeholders', () => {
+      expectFromHtml('<span i18n>{item.var, plural, other { {{item.placeholder}} items } }</span>')
+          .toEqual([
+            [
+              'Element',
+              '<span i18n>{item.var, plural, other { {{item.placeholder}} items } }</span>',
+              '<span i18n>', '</span>'
+            ],
+            ['Icu', '{item.var, plural, other { {{item.placeholder}} items } }'],
+            ['Icu:Var', 'item.var'],
+            ['Icu:Placeholder', '{{item.placeholder}}'],
+          ]);
+    });
+
+    it('is correct for nested ICUs', () => {
+      expectFromHtml(
+          '<span i18n>{item.var, plural, other { {{item.placeholder}} {nestedVar, plural, other { {{nestedPlaceholder}} }}} }</span>')
+          .toEqual([
+            [
+              'Element',
+              '<span i18n>{item.var, plural, other { {{item.placeholder}} {nestedVar, plural, other { {{nestedPlaceholder}} }}} }</span>',
+              '<span i18n>', '</span>'
+            ],
+            [
+              'Icu',
+              '{item.var, plural, other { {{item.placeholder}} {nestedVar, plural, other { {{nestedPlaceholder}} }}} }'
+            ],
+            ['Icu:Var', 'nestedVar'],
+            ['Icu:Var', 'item.var'],
+            ['Icu:Placeholder', '{{item.placeholder}}'],
+            ['Icu:Placeholder', '{{nestedPlaceholder}}'],
+          ]);
+    });
+  });
 });