refactor(compiler): associate accurate source spans with ICU expressions (#39072)

Prior to this change, expressions within ICUs would have a source span
corresponding with the whole ICU. This commit narrows down the source
spans of these expressions to the exact location in the source file, as
a prerequisite for reporting type check errors within these expressions.

PR Close #39072

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 this.createInterpolationAst(split.strings, expressions, input, location, absoluteOffset);
-    return new ASTWithSource(
+  }
-        new Interpolation(span, span.toAbsolute(absoluteOffset), split.strings, expressions), input,
+
-        location, absoluteOffset, this.errors);
+  /**
+   * 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);
   }
 
   /**

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) {

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

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)) {

packages/compiler/src/render3/r3_template_transform.ts

@@ -266,12 +266,6 @@ 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
@@ -279,10 +273,11 @@ class HtmlAstToIvyAst implements html.Visitor {
         // mismatches at runtime (i.e. placeholder will not be replaced with the correct value).
         const formattedKey = key.trim();
 
-        vars[formattedKey] =
+        const ast = this.bindingParser.parseInterpolationExpression(value.text, value.sourceSpan);
-            this._visitTextWithInterpolation(wrapped, expansion.sourceSpan) as t.BoundText;
+
+        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);

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`.

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
 }
 

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),
+  };
+}

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)]);
+    });
+  });
 });

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}}'],
+          ]);
+    });
+  });
 });

packages/compiler/test/render3/util/expression.ts

@@ -141,7 +141,14 @@ class ExpressionSourceHumanizer extends e.RecursiveAstVisitor implements t.Visit
   }
   visitContent(ast: t.Content) {}
   visitText(ast: t.Text) {}
-  visitIcu(ast: t.Icu) {}
+  visitIcu(ast: t.Icu) {
+    for (const key of Object.keys(ast.vars)) {
+      ast.vars[key].visit(this);
+    }
+    for (const key of Object.keys(ast.placeholders)) {
+      ast.placeholders[key].visit(this);
+    }
+  }
 }
 
 /**