refactor(compiler): Rework how ICU placeholders are handled (#53643)

The way we were handling ICU placeholders was not compatible with using
interpolations on attributes of elements inside the ICU. This change
refactors the handling of ICU placeholders and unifies the way
expression and tag placeholders work inside ICUs.

The new approach modifies the ingest logic to add the placeholder on to
the TextOp rather than the TextInterpolationOp. This is because, in
ICUs, we may need multiple i18n expressions created from the
interpolation expressions to roll up into the same placeholder. ICUs
essentially do the interpolation at compile time, combining the static
strings with special placeholder strings that represent the expression
values.

PR Close #53643

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/GOLDEN_PARTIAL.js

@@ -961,3 +961,43 @@ export declare class MyComponent {
     static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "my-component", never, {}, {}, never, never, true, never>;
 }
 
+/****************************************************************************************************
+ * PARTIAL FILE: attribute_interpolation.js
+ ****************************************************************************************************/
+import { Component } from '@angular/core';
+import * as i0 from "@angular/core";
+export class MyComponent {
+}
+MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
+MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "my-comp", ngImport: i0, template: `
+  <div i18n>
+    <span title="{{foo}}-{{foo}}"></span>
+    <span>{foo, select, other {<span title="{{foo}}-{{foo}}">foo</span>}}</span>
+    <span>{foo, select, other {{{foo}}-{{foo}}}}</span>
+  </div>
+  `, isInline: true });
+i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
+            type: Component,
+            args: [{
+                    selector: 'my-comp',
+                    standalone: true,
+                    template: `
+  <div i18n>
+    <span title="{{foo}}-{{foo}}"></span>
+    <span>{foo, select, other {<span title="{{foo}}-{{foo}}">foo</span>}}</span>
+    <span>{foo, select, other {{{foo}}-{{foo}}}}</span>
+  </div>
+  `
+                }]
+        }] });
+
+/****************************************************************************************************
+ * PARTIAL FILE: attribute_interpolation.d.ts
+ ****************************************************************************************************/
+import * as i0 from "@angular/core";
+export declare class MyComponent {
+    foo: any;
+    static ɵfac: i0.ɵɵFactoryDeclaration<MyComponent, never>;
+    static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "my-comp", never, {}, {}, never, never, true, never>;
+}
+

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json

@@ -245,6 +245,20 @@
           ]
         }
       ]
+    },
+    {
+      "description": "should handles ICUs with html content that has interpolated attributes",
+      "inputFiles": [
+        "attribute_interpolation.ts"
+      ],
+      "expectations": [
+        {
+          "extraChecks": [
+            "verifyPlaceholdersIntegrity",
+            "verifyUniqueConsts"
+          ]
+        }
+      ]
     }
   ]
 }

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.js

@@ -0,0 +1,75 @@
+consts: () => {
+  let $i18n_1$;
+  if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
+    /**
+     * @suppress {msgDescriptions}
+     */
+    const $MSG_EXTERNAL_6301050568345677976__ATTRIBUTE_INTERPOLATION_TS_2$ = goog.getMsg("{VAR_SELECT, select, other {{START_TAG_SPAN}foo{CLOSE_TAG_SPAN}}}");
+    $i18n_1$ = $MSG_EXTERNAL_6301050568345677976__ATTRIBUTE_INTERPOLATION_TS_2$;
+  } else {
+    $i18n_1$ = $localize`{VAR_SELECT, select, other {{START_TAG_SPAN}foo{CLOSE_TAG_SPAN}}}`;
+  }
+  $i18n_1$ = i0.ɵɵi18nPostprocess($i18n_1$, {
+    "CLOSE_TAG_SPAN": "</span>",
+    "START_TAG_SPAN": "<span title=\"\uFFFD1\uFFFD-\uFFFD2\uFFFD\">",
+    "VAR_SELECT": "\uFFFD0\uFFFD"
+  });
+  let $i18n_3$;
+  if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
+    /**
+     * @suppress {msgDescriptions}
+     */
+    const $MSG_EXTERNAL_369205108016154659__ATTRIBUTE_INTERPOLATION_TS_4$ = goog.getMsg("{VAR_SELECT, select, other {{INTERPOLATION}-{INTERPOLATION}}}");
+    $i18n_3$ = $MSG_EXTERNAL_369205108016154659__ATTRIBUTE_INTERPOLATION_TS_4$;
+  } else {
+    $i18n_3$ = $localize`{VAR_SELECT, select, other {{INTERPOLATION}-{INTERPOLATION}}}`;
+  }
+  $i18n_3$ = i0.ɵɵi18nPostprocess($i18n_3$, {
+    "INTERPOLATION": "\uFFFD4\uFFFD",
+    "VAR_SELECT": "\uFFFD3\uFFFD"
+  });
+  let $i18n_0$;
+  if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
+    /**
+     * @suppress {msgDescriptions}
+     */
+    const $MSG_EXTERNAL_6009429127580785009__ATTRIBUTE_INTERPOLATION_TS_5$ = goog.getMsg(
+        "{$startTagSpan}{$closeTagSpan}{$startTagSpan_1}{$icu}{$closeTagSpan}{$startTagSpan_1}{$icu_1}{$closeTagSpan}",
+        {
+          "closeTagSpan": "[\uFFFD/#2\uFFFD|\uFFFD/#3\uFFFD|\uFFFD/#4\uFFFD]",
+          "icu": $i18n_1$,
+          "icu_1": $i18n_3$,
+          "startTagSpan": "\uFFFD#2\uFFFD",
+          "startTagSpan_1": "[\uFFFD#3\uFFFD|\uFFFD#4\uFFFD]"
+        }, {
+          original_code: {
+            "closeTagSpan": "</span>",
+            "icu": "{foo, select, other {<span title=\"{{foo}}-{{foo}}\">foo</span>}}",
+            "icu_1": "{foo, select, other {{{foo}}-{{foo}}}}",
+            "startTagSpan": "<span title=\"{{foo}}-{{foo}}\">",
+            "startTagSpan_1": "<span>"
+          }
+        });
+    $i18n_0$ = $MSG_EXTERNAL_6009429127580785009__ATTRIBUTE_INTERPOLATION_TS_5$;
+  } else {
+    $i18n_0$ = $localize`${"\uFFFD#2\uFFFD"}:START_TAG_SPAN:${"[\uFFFD/#2\uFFFD|\uFFFD/#3\uFFFD|\uFFFD/#4\uFFFD]"}:CLOSE_TAG_SPAN:${"[\uFFFD#3\uFFFD|\uFFFD#4\uFFFD]"}:START_TAG_SPAN_1:${$i18n_1$}:ICU@@6051755734147382484:${"[\uFFFD/#2\uFFFD|\uFFFD/#3\uFFFD|\uFFFD/#4\uFFFD]"}:CLOSE_TAG_SPAN:${"[\uFFFD#3\uFFFD|\uFFFD#4\uFFFD]"}:START_TAG_SPAN_1:${$i18n_3$}:ICU_1@@7593934392904803263:${"[\uFFFD/#2\uFFFD|\uFFFD/#3\uFFFD|\uFFFD/#4\uFFFD]"}:CLOSE_TAG_SPAN:`;
+  }
+  $i18n_0$ = i0.ɵɵi18nPostprocess($i18n_0$);
+  return [$i18n_0$, [3, "title"]];
+},
+template: function MyComponent_Template(rf, ctx) {
+  if (rf & 1) {
+    i0.ɵɵelementStart(0, "div");
+    i0.ɵɵi18nStart(1, 0);
+    i0.ɵɵelement(2, "span", 1)(3, "span")(4, "span");
+    i0.ɵɵi18nEnd();
+    i0.ɵɵelementEnd();
+  }
+  if (rf & 2) {
+    i0.ɵɵadvance(2);
+    i0.ɵɵpropertyInterpolate2("title", "", ctx.foo, "-", ctx.foo, "");
+    i0.ɵɵadvance(2);
+    i0.ɵɵi18nExp(ctx.foo)(ctx.foo)(ctx.foo)(ctx.foo)(ctx.foo);
+    i0.ɵɵi18nApply(1);
+  }
+}

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.ts

@@ -0,0 +1,16 @@
+import {Component} from '@angular/core';
+
+@Component({
+  selector: 'my-comp',
+  standalone: true,
+  template: `
+  <div i18n>
+    <span title="{{foo}}-{{foo}}"></span>
+    <span>{foo, select, other {<span title="{{foo}}-{{foo}}">foo</span>}}</span>
+    <span>{foo, select, other {{{foo}}-{{foo}}}}</span>
+  </div>
+  `
+})
+export class MyComponent {
+  foo: any;
+}

packages/compiler/src/template/pipeline/ir/src/enums.ts

@@ -230,6 +230,11 @@ export enum OpKind {
    */
   IcuEnd,
 
+  /**
+   * An instruction representing a placeholder in an ICU expression.
+   */
+  IcuPlaceholder,
+
   /**
    * An i18n context containing information needed to generate an i18n message.
    */

packages/compiler/src/template/pipeline/ir/src/expression.ts

@@ -1004,6 +1004,7 @@ export function transformExpressionsInOp(
     case OpKind.Template:
     case OpKind.Text:
     case OpKind.I18nAttributes:
+    case OpKind.IcuPlaceholder:
       // These operations contain no expressions.
       break;
     default:

packages/compiler/src/template/pipeline/ir/src/ops/create.ts

@@ -27,7 +27,7 @@ export type CreateOp = ListEndOp<CreateOp>|StatementOp<CreateOp>|ElementOp|Eleme
     ElementEndOp|ContainerOp|ContainerStartOp|ContainerEndOp|TemplateOp|EnableBindingsOp|
     DisableBindingsOp|TextOp|ListenerOp|PipeOp|VariableOp<CreateOp>|NamespaceOp|ProjectionDefOp|
     ProjectionOp|ExtractedAttributeOp|DeferOp|DeferOnOp|RepeaterCreateOp|I18nMessageOp|I18nOp|
-    I18nStartOp|I18nEndOp|IcuStartOp|IcuEndOp|I18nContextOp|I18nAttributesOp;
+    I18nStartOp|I18nEndOp|IcuStartOp|IcuEndOp|IcuPlaceholderOp|I18nContextOp|I18nAttributesOp;
 
 /**
  * An operation representing the creation of an element or container.
@@ -465,19 +465,27 @@ export interface TextOp extends Op<CreateOp>, ConsumesSlotOpTrait {
    */
   initialValue: string;
 
+  /**
+   * The placeholder for this text in its parent ICU. If this text is not part of an ICU, the
+   * placeholder is null.
+   */
+  icuPlaceholder: string|null;
+
   sourceSpan: ParseSourceSpan|null;
 }
 
 /**
  * Create a `TextOp`.
  */
 export function createTextOp(
-    xref: XrefId, initialValue: string, sourceSpan: ParseSourceSpan|null): TextOp {
+    xref: XrefId, initialValue: string, icuPlaceholder: string|null,
+    sourceSpan: ParseSourceSpan|null): TextOp {
   return {
     kind: OpKind.Text,
     xref,
     handle: new SlotHandle(),
     initialValue,
+    icuPlaceholder,
     sourceSpan,
     ...TRAIT_CONSUMES_SLOT,
     ...NEW_OP,
@@ -1167,6 +1175,51 @@ export function createIcuEndOp(xref: XrefId): IcuEndOp {
   };
 }
 
+/**
+ * An op that represents a placeholder in an ICU expression.
+ */
+export interface IcuPlaceholderOp extends Op<CreateOp> {
+  kind: OpKind.IcuPlaceholder;
+
+  /**
+   * The ID of the ICU placeholder.
+   */
+  xref: XrefId;
+
+  /**
+   * The name of the placeholder in the ICU expression.
+   */
+  name: string;
+
+  /**
+   * The static strings to be combined with dynamic expression values to form the text. This works
+   * like interpolation, but the strings are combined at compile time, using special placeholders
+   * for the dynamic expressions, and put into the translated message.
+   */
+  strings: string[];
+
+  /**
+   * Placeholder values for the i18n expressions to be combined with the static strings to form the
+   * full placeholder value.
+   */
+  expressionPlaceholders: I18nParamValue[];
+}
+
+/**
+ * Creates an ICU placeholder op.
+ */
+export function createIcuPlaceholderOp(
+    xref: XrefId, name: string, strings: string[]): IcuPlaceholderOp {
+  return {
+    kind: OpKind.IcuPlaceholder,
+    xref,
+    name,
+    strings,
+    expressionPlaceholders: [],
+    ...NEW_OP,
+  };
+}
+
 /**
  * An i18n context that is used to generate a translated i18n message. A separate context is created
  * for three different scenarios:

packages/compiler/src/template/pipeline/ir/src/ops/update.ts

@@ -687,10 +687,15 @@ export interface I18nExpressionOp extends Op<UpdateOp>, ConsumesVarsTrait,
    */
   expression: o.Expression;
 
+  icuPlaceholder: XrefId|null;
+
   /**
-   * The i18n placeholder associated with this expression.
+   * The i18n placeholder associated with this expression. This can be null if the expression is
+   * part of an ICU placeholder. In this case it gets combined with the string literal value and
+   * other expressions in the ICU placeholder and assigned to the translated message under the ICU
+   * placeholder name.
    */
-  i18nPlaceholder: string;
+  i18nPlaceholder: string|null;
 
   /**
    * The time that this expression is resolved.
@@ -716,15 +721,17 @@ export interface I18nExpressionOp extends Op<UpdateOp>, ConsumesVarsTrait,
  */
 export function createI18nExpressionOp(
     context: XrefId, target: XrefId, i18nOwner: XrefId, handle: SlotHandle,
-    expression: o.Expression, i18nPlaceholder: string, resolutionTime: I18nParamResolutionTime,
-    usage: I18nExpressionFor, name: string, sourceSpan: ParseSourceSpan): I18nExpressionOp {
+    expression: o.Expression, icuPlaceholder: XrefId|null, i18nPlaceholder: string|null,
+    resolutionTime: I18nParamResolutionTime, usage: I18nExpressionFor, name: string,
+    sourceSpan: ParseSourceSpan): I18nExpressionOp {
   return {
     kind: OpKind.I18nExpression,
     context,
     target,
     i18nOwner,
     handle,
     expression,
+    icuPlaceholder,
     i18nPlaceholder,
     resolutionTime,
     usage,

packages/compiler/src/template/pipeline/src/emit.ts

@@ -62,7 +62,6 @@ import {resolveContexts} from './phases/resolve_contexts';
 import {resolveDollarEvent} from './phases/resolve_dollar_event';
 import {resolveI18nElementPlaceholders} from './phases/resolve_i18n_element_placeholders';
 import {resolveI18nExpressionPlaceholders} from './phases/resolve_i18n_expression_placeholders';
-import {resolveI18nIcuPlaceholders} from './phases/resolve_i18n_icu_placeholders';
 import {resolveNames} from './phases/resolve_names';
 import {resolveSanitizers} from './phases/resolve_sanitizers';
 import {saveAndRestoreView} from './phases/save_restore_view';
@@ -132,7 +131,6 @@ const phases: Phase[] = [
   {kind: Kind.Tmpl, fn: createDeferDepsFns},
   {kind: Kind.Tmpl, fn: resolveI18nElementPlaceholders},
   {kind: Kind.Tmpl, fn: resolveI18nExpressionPlaceholders},
-  {kind: Kind.Tmpl, fn: resolveI18nIcuPlaceholders},
   {kind: Kind.Tmpl, fn: extractI18nMessages},
   {kind: Kind.Tmpl, fn: generateTrackFns},
   {kind: Kind.Tmpl, fn: collectI18nConsts},

packages/compiler/src/template/pipeline/src/ingest.ts

@@ -145,9 +145,9 @@ function ingestNodes(unit: ViewCompilationUnit, template: t.Node[]): void {
     } else if (node instanceof t.Content) {
       ingestContent(unit, node);
     } else if (node instanceof t.Text) {
-      ingestText(unit, node);
+      ingestText(unit, node, null);
     } else if (node instanceof t.BoundText) {
-      ingestBoundText(unit, node);
+      ingestBoundText(unit, node, null);
     } else if (node instanceof t.IfBlock) {
       ingestIfBlock(unit, node);
     } else if (node instanceof t.SwitchBlock) {
@@ -282,15 +282,16 @@ function ingestContent(unit: ViewCompilationUnit, content: t.Content): void {
 /**
  * Ingest a literal text node from the AST into the given `ViewCompilation`.
  */
-function ingestText(unit: ViewCompilationUnit, text: t.Text): void {
-  unit.create.push(ir.createTextOp(unit.job.allocateXrefId(), text.value, text.sourceSpan));
+function ingestText(unit: ViewCompilationUnit, text: t.Text, icuPlaceholder: string|null): void {
+  unit.create.push(
+      ir.createTextOp(unit.job.allocateXrefId(), text.value, icuPlaceholder, text.sourceSpan));
 }
 
 /**
  * Ingest an interpolated text node from the AST into the given `ViewCompilation`.
  */
 function ingestBoundText(
-    unit: ViewCompilationUnit, text: t.BoundText, i18nPlaceholders?: string[]): void {
+    unit: ViewCompilationUnit, text: t.BoundText, icuPlaceholder: string|null): void {
   let value = text.value;
   if (value instanceof e.ASTWithSource) {
     value = value.ast;
@@ -304,21 +305,18 @@ function ingestBoundText(
         `Unhandled i18n metadata type for text interpolation: ${text.i18n?.constructor.name}`);
   }
 
-  if (i18nPlaceholders === undefined) {
-    // TODO: We probably can just use the placeholders field, instead of walking the AST.
-    i18nPlaceholders = text.i18n instanceof i18n.Container ?
-        text.i18n.children
-            .filter((node): node is i18n.Placeholder => node instanceof i18n.Placeholder)
-            .map(placeholder => placeholder.name) :
-        [];
-  }
+  const i18nPlaceholders = text.i18n instanceof i18n.Container ?
+      text.i18n.children
+          .filter((node): node is i18n.Placeholder => node instanceof i18n.Placeholder)
+          .map(placeholder => placeholder.name) :
+      [];
   if (i18nPlaceholders.length > 0 && i18nPlaceholders.length !== value.expressions.length) {
     throw Error(`Unexpected number of i18n placeholders (${
         value.expressions.length}) for BoundText with ${value.expressions.length} expressions`);
   }
 
   const textXref = unit.job.allocateXrefId();
-  unit.create.push(ir.createTextOp(textXref, '', text.sourceSpan));
+  unit.create.push(ir.createTextOp(textXref, '', icuPlaceholder, text.sourceSpan));
   // TemplateDefinitionBuilder does not generate source maps for sub-expressions inside an
   // interpolation. We copy that behavior in compatibility mode.
   // TODO: is it actually correct to generate these extra maps in modern mode?
@@ -564,9 +562,9 @@ function ingestIcu(unit: ViewCompilationUnit, icu: t.Icu) {
     unit.create.push(ir.createIcuStartOp(xref, icu.i18n, icuFromI18nMessage(icu.i18n).name, null!));
     for (const [placeholder, text] of Object.entries({...icu.vars, ...icu.placeholders})) {
       if (text instanceof t.BoundText) {
-        ingestBoundText(unit, text, [placeholder]);
+        ingestBoundText(unit, text, placeholder);
       } else {
-        ingestText(unit, text);
+        ingestText(unit, text, placeholder);
       }
     }
     unit.create.push(ir.createIcuEndOp(xref));