fix(compiler): include leading whitespace in source-spans of i18n messages (#43132)

Previously, the way templates were tokenized meant that we lost information
about the location of interpolations if the template contained encoded HTML
entities. This meant that the mapping back to the source interpolated strings
could be offset incorrectly.

Also, the source-span assigned to an i18n message did not include leading
whitespace. This confused the output source-mappings so that the first text
nodes of the message stopped at the first non-whitespace character.

This commit makes use of the previous refactorings, where more fine grain
information was provided in text tokens, to enable the parser to identify
the location of the interpolations in the original source more accurately.

Fixes #41034

PR Close #43132

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

@@ -7,14 +7,28 @@ 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: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: `
-  <div i18n>Some Message</div>
+  <div i18n-title title="Some &amp; attribute"></div>
+  <div i18n>Some &amp; message</div>
+  <div i18n-title title="Some &amp; {{'interpolated'}} attribute"></div>
+  <div i18n>Some &amp; {{'interpolated' }} message</div>
+  <div i18n>&amp;</div>
+  <div i18n>&amp;&quot;</div>
+  <div i18n-title title="&quot;"></div>
+  <div i18n-title title="&quot;&quot;"></div>
 `, isInline: true });
 i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
             type: Component,
             args: [{
                     selector: 'my-component',
                     template: `
-  <div i18n>Some Message</div>
+  <div i18n-title title="Some &amp; attribute"></div>
+  <div i18n>Some &amp; message</div>
+  <div i18n-title title="Some &amp; {{'interpolated'}} attribute"></div>
+  <div i18n>Some &amp; {{'interpolated' }} message</div>
+  <div i18n>&amp;</div>
+  <div i18n>&amp;&quot;</div>
+  <div i18n-title title="&quot;"></div>
+  <div i18n-title title="&quot;&quot;"></div>
 `
                 }]
         }] });

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/localize_legacy_message_ids/legacy_enabled.js

@@ -1,5 +1,11 @@
-let $I18N_0$;
-if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { … }
-else {
-    $I18N_0$ = $localize `:␟ec93160d6d6a8822214060dd7938bf821c22b226␟6795333002533525253:Some Message`;
-}
+$localize `:␟82ec661067f503a3357ecc159b2128325e9208cd␟2908931752694090721:Some & attribute`;
+…
+$localize `:␟10adaf0ad7b8ba40200cd3c0e7c8d0f13280d522␟4700340487900776701:Some & message`;
+…
+$localize `:␟57ebd20267116c04cc1dbd7be0b73bf56484f45d␟2334195497629636162:Some & ${"\uFFFD0\uFFFD"}:INTERPOLATION: attribute`;
+…
+$localize `:␟28d558ca32556f1da67a333e3dada321a97212cd␟3204054277547499090:Some & ${"\uFFFD0\uFFFD"}:INTERPOLATION: message`;
+…
+$localize `:␟0b3dff7b9382b6217ac97c99f9b04df04381bfdd␟2406634758623728945:&`;
+…
+$localize `:␟25b7cbf210e59a931423097cb7f2e1b72991a687␟4156372478368653226:&"`;

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/localize_legacy_message_ids/legacy_enabled.ts

@@ -3,7 +3,14 @@ import {Component, NgModule} from '@angular/core';
 @Component({
   selector: 'my-component',
   template: `
-  <div i18n>Some Message</div>
+  <div i18n-title title="Some &amp; attribute"></div>
+  <div i18n>Some &amp; message</div>
+  <div i18n-title title="Some &amp; {{'interpolated'}} attribute"></div>
+  <div i18n>Some &amp; {{'interpolated' }} message</div>
+  <div i18n>&amp;</div>
+  <div i18n>&amp;&quot;</div>
+  <div i18n-title title="&quot;"></div>
+  <div i18n-title title="&quot;&quot;"></div>
 `
 })
 export class MyComponent {
@@ -11,4 +18,4 @@ export class MyComponent {
 
 @NgModule({declarations: [MyComponent]})
 export class MyModule {
-}
+}

packages/compiler-cli/test/compliance/test_cases/source_mapping/inline_templates/i18n_message_element_whitespace.js

@@ -1,19 +1,19 @@
-` pre-p ${ // SOURCE: "/i18n_message_element_whitespace.ts" "pre-p\\n  "
+` pre-p ${ // SOURCE: "/i18n_message_element_whitespace.ts" "\\n  pre-p\\n  "
 …
-"\uFFFD#2\uFFFD" // SOURCE: "/i18n_message_element_whitespace.ts" "<p>\\n    "
+"\uFFFD#2\uFFFD" // SOURCE: "/i18n_message_element_whitespace.ts" "<p>"
 …
-}:START_PARAGRAPH: in-p ${ // SOURCE: "/i18n_message_element_whitespace.ts" "in-p\\n  "
+}:START_PARAGRAPH: in-p ${ // SOURCE: "/i18n_message_element_whitespace.ts" "\\n    in-p\\n  "
 …
 "\uFFFD/#2\uFFFD" // SOURCE: "/i18n_message_element_whitespace.ts" "</p>"
 …
-}:CLOSE_PARAGRAPH: post-p\n` // SOURCE: "/i18n_message_element_whitespace.ts" "post-p\\n"
+}:CLOSE_PARAGRAPH: post-p\n` // SOURCE: "/i18n_message_element_whitespace.ts" "\\n  post-p\\n"
 …
 
 i0.ɵɵelementStart(0, "div") // SOURCE: "/i18n_message_element_whitespace.ts" "<div i18n>"
 …
 i0.ɵɵi18nStart(1, 0) // SOURCE: "/i18n_message_element_whitespace.ts" "<div i18n>"
 …
-i0.ɵɵelement(2, "p") // SOURCE: "/i18n_message_element_whitespace.ts" "<p>\\n    "
+i0.ɵɵelement(2, "p") // SOURCE: "/i18n_message_element_whitespace.ts" "<p>"
 …
 i0.ɵɵi18nEnd() // SOURCE: "/i18n_message_element_whitespace.ts" "</div>"
 …

packages/compiler-cli/test/compliance/test_cases/source_mapping/inline_templates/i18n_message_element_whitespace_partial.js

@@ -1,19 +1,19 @@
-$localize` pre-p ${ // SOURCE: "/i18n_message_element_whitespace.ts" "pre-p\\n  "
+$localize` pre-p ${ // SOURCE: "/i18n_message_element_whitespace.ts" "\\n  pre-p\\n  "
 …
-"\uFFFD#2\uFFFD" // SOURCE: "/i18n_message_element_whitespace.ts" "<p>\\n    "
+"\uFFFD#2\uFFFD" // SOURCE: "/i18n_message_element_whitespace.ts" "<p>"
 …
-}:START_PARAGRAPH: in-p ${ // SOURCE: "/i18n_message_element_whitespace.ts" "in-p\\n  "
+}:START_PARAGRAPH: in-p ${ // SOURCE: "/i18n_message_element_whitespace.ts" "\\n    in-p\\n  "
 …
-"\uFFFD/#2\uFFFD" // SOURCE: "/i18n_message_element_whitespace.ts" "</p>\\n  "
+"\uFFFD/#2\uFFFD" // SOURCE: "/i18n_message_element_whitespace.ts" "</p>"
 …
-}:CLOSE_PARAGRAPH: post-p\n` // SOURCE: "/i18n_message_element_whitespace.ts" "post-p\\n"
+}:CLOSE_PARAGRAPH: post-p\n` // SOURCE: "/i18n_message_element_whitespace.ts" "\\n  post-p\\n"
 …
 
-.ɵɵelementStart(0, "div") // SOURCE: "/i18n_message_element_whitespace.ts" "<div i18n>\\n  "
+.ɵɵelementStart(0, "div") // SOURCE: "/i18n_message_element_whitespace.ts" "<div i18n>"
 …
-.ɵɵi18nStart(1, 0) // SOURCE: "/i18n_message_element_whitespace.ts" "<div i18n>\\n  "
+.ɵɵi18nStart(1, 0) // SOURCE: "/i18n_message_element_whitespace.ts" "<div i18n>"
 …
-.ɵɵelement(2, "p") // SOURCE: "/i18n_message_element_whitespace.ts" "<p>\\n    "
+.ɵɵelement(2, "p") // SOURCE: "/i18n_message_element_whitespace.ts" "<p>"
 …
 .ɵɵi18nEnd() // SOURCE: "/i18n_message_element_whitespace.ts" "</div>'"
 …

packages/compiler-cli/test/ngtsc/template_mapping_spec.ts

@@ -464,27 +464,27 @@ runInEachFileSystem((os) => {
              // $localize expressions
              expectMapping(mappings, {
                sourceUrl: '../test.ts',
-               source: 'pre-p\n  ',
+               source: '\n  pre-p\n  ',
                generated: '` pre-p ${',
              });
              expectMapping(mappings, {
                sourceUrl: '../test.ts',
-               source: '<p>\n    ',
+               source: '<p>',
                generated: '"\\uFFFD#2\\uFFFD"',
              });
              expectMapping(mappings, {
                sourceUrl: '../test.ts',
-               source: 'in-p\n  ',
+               source: '\n    in-p\n  ',
                generated: '}:START_PARAGRAPH: in-p ${',
              });
              expectMapping(mappings, {
                sourceUrl: '../test.ts',
-               source: '</p>\n  ',
+               source: '</p>',
                generated: '"\\uFFFD/#2\\uFFFD"',
              });
              expectMapping(mappings, {
                sourceUrl: '../test.ts',
-               source: 'post-p\n',
+               source: '\n  post-p\n',
                generated: '}:CLOSE_PARAGRAPH: post-p\n`',
              });
              // ivy instructions

packages/compiler/src/i18n/i18n_parser.ts

@@ -7,10 +7,11 @@
  */
 
 import {Lexer as ExpressionLexer} from '../expression_parser/lexer';
-import {InterpolationPiece, Parser as ExpressionParser} from '../expression_parser/parser';
+import {Parser as ExpressionParser} from '../expression_parser/parser';
 import * as html from '../ml_parser/ast';
 import {getHtmlTagDefinition} from '../ml_parser/html_tags';
 import {InterpolationConfig} from '../ml_parser/interpolation_config';
+import {Token, TokenType} from '../ml_parser/lexer';
 import {ParseSourceSpan} from '../parse_util';
 
 import * as i18n from './i18n_ast';
@@ -105,13 +106,18 @@ class _I18nVisitor implements html.Visitor {
   }
 
   visitAttribute(attribute: html.Attribute, context: I18nMessageVisitorContext): i18n.Node {
-    const node = this._visitTextWithInterpolation(
-        attribute.value, attribute.valueSpan || attribute.sourceSpan, context, attribute.i18n);
+    const node = attribute.valueTokens === undefined || attribute.valueTokens.length === 1 ?
+        new i18n.Text(attribute.value, attribute.valueSpan || attribute.sourceSpan) :
+        this._visitTextWithInterpolation(
+            attribute.valueTokens, attribute.valueSpan || attribute.sourceSpan, context,
+            attribute.i18n);
     return context.visitNodeFn(attribute, node);
   }
 
   visitText(text: html.Text, context: I18nMessageVisitorContext): i18n.Node {
-    const node = this._visitTextWithInterpolation(text.value, text.sourceSpan, context, text.i18n);
+    const node = text.tokens.length === 1 ?
+        new i18n.Text(text.value, text.sourceSpan) :
+        this._visitTextWithInterpolation(text.tokens, text.sourceSpan, context, text.i18n);
     return context.visitNodeFn(text, node);
   }
 
@@ -165,66 +171,54 @@ class _I18nVisitor implements html.Visitor {
    * @param previousI18n Any i18n metadata associated with this `text` from a previous pass.
    */
   private _visitTextWithInterpolation(
-      text: string, sourceSpan: ParseSourceSpan, context: I18nMessageVisitorContext,
+      tokens: Token[], sourceSpan: ParseSourceSpan, context: I18nMessageVisitorContext,
       previousI18n: i18n.I18nMeta|undefined): i18n.Node {
-    const {strings, expressions} = this._expressionParser.splitInterpolation(
-        text, sourceSpan.start.toString(), this._interpolationConfig);
-
-    // No expressions, return a single text.
-    if (expressions.length === 0) {
-      return new i18n.Text(text, sourceSpan);
-    }
-
     // Return a sequence of `Text` and `Placeholder` nodes grouped in a `Container`.
     const nodes: i18n.Node[] = [];
-    for (let i = 0; i < strings.length - 1; i++) {
-      this._addText(nodes, strings[i], sourceSpan);
-      this._addPlaceholder(nodes, context, expressions[i], sourceSpan);
+    // We will only create a container if there are actually interpolations,
+    // so this flag tracks that.
+    let hasInterpolation = false;
+    for (const token of tokens) {
+      switch (token.type) {
+        case TokenType.INTERPOLATION:
+        case TokenType.ATTR_VALUE_INTERPOLATION:
+          hasInterpolation = true;
+          const expression = token.parts[1];
+          const baseName = extractPlaceholderName(expression) || 'INTERPOLATION';
+          const phName = context.placeholderRegistry.getPlaceholderName(baseName, expression);
+          context.placeholderToContent[phName] = {
+            text: token.parts.join(''),
+            sourceSpan: token.sourceSpan
+          };
+          nodes.push(new i18n.Placeholder(expression, phName, token.sourceSpan));
+          break;
+        default:
+          if (token.parts[0].length > 0) {
+            // This token is text or an encoded entity.
+            // If it is following on from a previous text node then merge it into that node
+            // Otherwise, if it is following an interpolation, then add a new node.
+            const previous = nodes[nodes.length - 1];
+            if (previous instanceof i18n.Text) {
+              previous.value += token.parts[0];
+              previous.sourceSpan = new ParseSourceSpan(
+                  previous.sourceSpan.start, token.sourceSpan.end, previous.sourceSpan.fullStart,
+                  previous.sourceSpan.details);
+            } else {
+              nodes.push(new i18n.Text(token.parts[0], token.sourceSpan));
+            }
+          }
+          break;
+      }
     }
-    // The last index contains no expression
-    this._addText(nodes, strings[strings.length - 1], sourceSpan);
 
-    // Whitespace removal may have invalidated the interpolation source-spans.
-    reusePreviousSourceSpans(nodes, previousI18n);
-
-    return new i18n.Container(nodes, sourceSpan);
-  }
-
-  /**
-   * Create a new `Text` node from the `textPiece` and add it to the `nodes` collection.
-   *
-   * @param nodes The nodes to which the created `Text` node should be added.
-   * @param textPiece The text and relative span information for this `Text` node.
-   * @param interpolationSpan The span of the whole interpolated text.
-   */
-  private _addText(
-      nodes: i18n.Node[], textPiece: InterpolationPiece, interpolationSpan: ParseSourceSpan): void {
-    if (textPiece.text.length > 0) {
-      // No need to add empty strings
-      const stringSpan = getOffsetSourceSpan(interpolationSpan, textPiece);
-      nodes.push(new i18n.Text(textPiece.text, stringSpan));
+    if (hasInterpolation) {
+      // Whitespace removal may have invalidated the interpolation source-spans.
+      reusePreviousSourceSpans(nodes, previousI18n);
+      return new i18n.Container(nodes, sourceSpan);
+    } else {
+      return nodes[0];
     }
   }
-
-  /**
-   * Create a new `Placeholder` node from the `expression` and add it to the `nodes` collection.
-   *
-   * @param nodes The nodes to which the created `Text` node should be added.
-   * @param context The current context of the visitor, used to compute and store placeholders.
-   * @param expression The expression text and relative span information for this `Placeholder`
-   *     node.
-   * @param interpolationSpan The span of the whole interpolated text.
-   */
-  private _addPlaceholder(
-      nodes: i18n.Node[], context: I18nMessageVisitorContext, expression: InterpolationPiece,
-      interpolationSpan: ParseSourceSpan): void {
-    const sourceSpan = getOffsetSourceSpan(interpolationSpan, expression);
-    const baseName = extractPlaceholderName(expression.text) || 'INTERPOLATION';
-    const phName = context.placeholderRegistry.getPlaceholderName(baseName, expression.text);
-    const text = this._interpolationConfig.start + expression.text + this._interpolationConfig.end;
-    context.placeholderToContent[phName] = {text, sourceSpan};
-    nodes.push(new i18n.Placeholder(expression.text, phName, sourceSpan));
-  }
 }
 
 /**
@@ -247,7 +241,7 @@ function reusePreviousSourceSpans(nodes: i18n.Node[], previousI18n: i18n.I18nMet
 
   if (previousI18n instanceof i18n.Container) {
     // The `previousI18n` is a `Container`, which means that this is a second i18n extraction pass
-    // after whitespace has been removed from the AST ndoes.
+    // after whitespace has been removed from the AST nodes.
     assertEquivalentNodes(previousI18n.children, nodes);
 
     // Reuse the source-spans from the first pass.
@@ -282,14 +276,6 @@ function assertEquivalentNodes(previousNodes: i18n.Node[], nodes: i18n.Node[]):
   }
 }
 
-/**
- * Create a new `ParseSourceSpan` from the `sourceSpan`, offset by the `start` and `end` values.
- */
-function getOffsetSourceSpan(
-    sourceSpan: ParseSourceSpan, {start, end}: InterpolationPiece): ParseSourceSpan {
-  return new ParseSourceSpan(sourceSpan.fullStart.moveBy(start), sourceSpan.fullStart.moveBy(end));
-}
-
 const _CUSTOM_PH_EXP =
     /\/\/[\s\S]*i18n[\s\S]*\([\s\S]*ph[\s\S]*=[\s\S]*("|')([\s\S]*?)\1[\s\S]*\)/g;
 

packages/compiler/src/render3/view/i18n/localize_utils.ts

@@ -35,7 +35,10 @@ class LocalizeSerializerVisitor implements i18n.Visitor {
       // Two literal pieces in a row means that there was some comment node in-between.
       context[context.length - 1].text += text.value;
     } else {
-      context.push(new o.LiteralPiece(text.value, text.sourceSpan));
+      const sourceSpan = new ParseSourceSpan(
+          text.sourceSpan.fullStart, text.sourceSpan.end, text.sourceSpan.fullStart,
+          text.sourceSpan.details);
+      context.push(new o.LiteralPiece(text.value, sourceSpan));
     }
   }
 
@@ -90,7 +93,7 @@ function getSourceSpan(message: i18n.Message): ParseSourceSpan {
   const startNode = message.nodes[0];
   const endNode = message.nodes[message.nodes.length - 1];
   return new ParseSourceSpan(
-      startNode.sourceSpan.start, endNode.sourceSpan.end, startNode.sourceSpan.fullStart,
+      startNode.sourceSpan.fullStart, endNode.sourceSpan.end, startNode.sourceSpan.fullStart,
       startNode.sourceSpan.details);
 }
 

packages/compiler/test/render3/view/i18n_spec.ts

@@ -478,7 +478,7 @@ describe('serializeI18nMessageForLocalize', () => {
        expect(messageParts[3].text).toEqual('');
        expect(messageParts[3].sourceSpan.toString()).toEqual('');
        expect(messageParts[4].text).toEqual(' D');
-       expect(messageParts[4].sourceSpan.toString()).toEqual('D');
+       expect(messageParts[4].sourceSpan.toString()).toEqual(' D');
 
        expect(placeHolders[0].text).toEqual('START_TAG_SPAN');
        expect(placeHolders[0].sourceSpan.toString()).toEqual('<span>');