refactor(compiler): make I18MetaVisitor stateless (#33318)

This commit cleans up the I18MetaVisitor code by moving all the
state of the visitor into a `context` object that gets passed along
as the nodes are being visited. This is in keeping with how visitors
are designed but also makes it easy to remove the
[definite assignment assertions](https://mariusschulz.com/blog/strict-property-initialization-in-typescript#solution-4-definite-assignment-assertion)
from the class properties.

Also, a `I18nMessageFactory` named type is exported to make it
clearer to consumers of the `createI18nMessageFactory()` function.

PR Close #33318

packages/compiler/src/i18n/extractor_merger.ts

@@ -10,7 +10,7 @@ import * as html from '../ml_parser/ast';
 import {InterpolationConfig} from '../ml_parser/interpolation_config';
 import {ParseTreeResult} from '../ml_parser/parser';
 import * as i18n from './i18n_ast';
-import {createI18nMessageFactory} from './i18n_parser';
+import {I18nMessageFactory, createI18nMessageFactory} from './i18n_parser';
 import {I18nError} from './parse_util';
 import {TranslationBundle} from './translation_bundle';
 
@@ -93,8 +93,7 @@ class _Visitor implements html.Visitor {
   // TODO(issue/24571): remove '!'.
   private _translations !: TranslationBundle;
   // TODO(issue/24571): remove '!'.
-  private _createI18nMessage !: (
+  private _createI18nMessage !: I18nMessageFactory;
-      msg: html.Node[], meaning: string, description: string, id: string) => i18n.Message;
 
 
   constructor(private _implicitTags: string[], private _implicitAttrs: {[k: string]: string[]}) {}

packages/compiler/src/i18n/i18n_parser.ts

@@ -18,63 +18,62 @@ import {PlaceholderRegistry} from './serializers/placeholder';
 
 const _expParser = new ExpressionParser(new ExpressionLexer());
 
-type VisitNodeFn = (html: html.Node, i18n: i18n.Node) => void;
+export type VisitNodeFn = (html: html.Node, i18n: i18n.Node) => i18n.Node;
+
+export interface I18nMessageFactory {
+  (nodes: html.Node[], meaning: string, description: string, id: string,
+   visitNodeFn?: VisitNodeFn): i18n.Message;
+}
 
 /**
  * Returns a function converting html nodes to an i18n Message given an interpolationConfig
  */
-export function createI18nMessageFactory(interpolationConfig: InterpolationConfig): (
+export function createI18nMessageFactory(interpolationConfig: InterpolationConfig):
-    nodes: html.Node[], meaning: string, description: string, id: string,
+    I18nMessageFactory {
-    visitNodeFn?: VisitNodeFn) => i18n.Message {
   const visitor = new _I18nVisitor(_expParser, interpolationConfig);
-
+  return (nodes, meaning, description, id, visitNodeFn) =>
-  return (nodes: html.Node[], meaning: string, description: string, id: string,
-          visitNodeFn?: VisitNodeFn) =>
              visitor.toI18nMessage(nodes, meaning, description, id, visitNodeFn);
 }
 
-class _I18nVisitor implements html.Visitor {
+interface I18nMessageVisitorContext {
-  // TODO(issue/24571): remove '!'.
+  isIcu: boolean;
-  private _isIcu !: boolean;
+  icuDepth: number;
-  // TODO(issue/24571): remove '!'.
+  placeholderRegistry: PlaceholderRegistry;
-  private _icuDepth !: number;
+  placeholderToContent: {[phName: string]: string};
-  // TODO(issue/24571): remove '!'.
+  placeholderToMessage: {[phName: string]: i18n.Message};
-  private _placeholderRegistry !: PlaceholderRegistry;
+  visitNodeFn: VisitNodeFn;
-  // TODO(issue/24571): remove '!'.
+}
-  private _placeholderToContent !: {[phName: string]: string};
-  // TODO(issue/24571): remove '!'.
-  private _placeholderToMessage !: {[phName: string]: i18n.Message};
-  private _visitNodeFn: VisitNodeFn|undefined;
 
+function noopVisitNodeFn(_html: html.Node, i18n: i18n.Node): i18n.Node {
+  return i18n;
+}
+
+class _I18nVisitor implements html.Visitor {
   constructor(
       private _expressionParser: ExpressionParser,
       private _interpolationConfig: InterpolationConfig) {}
 
   public toI18nMessage(
       nodes: html.Node[], meaning: string, description: string, id: string,
-      visitNodeFn?: VisitNodeFn): i18n.Message {
+      visitNodeFn: VisitNodeFn|undefined): i18n.Message {
-    this._isIcu = nodes.length == 1 && nodes[0] instanceof html.Expansion;
+    const context: I18nMessageVisitorContext = {
-    this._icuDepth = 0;
+      isIcu: nodes.length == 1 && nodes[0] instanceof html.Expansion,
-    this._placeholderRegistry = new PlaceholderRegistry();
+      icuDepth: 0,
-    this._placeholderToContent = {};
+      placeholderRegistry: new PlaceholderRegistry(),
-    this._placeholderToMessage = {};
+      placeholderToContent: {},
-    this._visitNodeFn = visitNodeFn;
+      placeholderToMessage: {},
+      visitNodeFn: visitNodeFn || noopVisitNodeFn,
+    };
 
-    const i18nodes: i18n.Node[] = html.visitAll(this, nodes, {});
+    const i18nodes: i18n.Node[] = html.visitAll(this, nodes, context);
 
     return new i18n.Message(
-        i18nodes, this._placeholderToContent, this._placeholderToMessage, meaning, description, id);
+        i18nodes, context.placeholderToContent, context.placeholderToMessage, meaning, description,
+        id);
   }
 
-  private _visitNode(html: html.Node, i18n: i18n.Node): i18n.Node {
+  visitElement(el: html.Element, context: I18nMessageVisitorContext): i18n.Node {
-    if (this._visitNodeFn) {
+    const children = html.visitAll(this, el.children, context);
-      this._visitNodeFn(html, i18n);
-    }
-    return i18n;
-  }
-
-  visitElement(el: html.Element, context: any): i18n.Node {
-    const children = html.visitAll(this, el.children);
     const attrs: {[k: string]: string} = {};
     el.attrs.forEach(attr => {
       // Do not visit the attributes, translatable ones are top-level ASTs
@@ -83,70 +82,71 @@ class _I18nVisitor implements html.Visitor {
 
     const isVoid: boolean = getHtmlTagDefinition(el.name).isVoid;
     const startPhName =
-        this._placeholderRegistry.getStartTagPlaceholderName(el.name, attrs, isVoid);
+        context.placeholderRegistry.getStartTagPlaceholderName(el.name, attrs, isVoid);
-    this._placeholderToContent[startPhName] = el.sourceSpan !.toString();
+    context.placeholderToContent[startPhName] = el.sourceSpan !.toString();
 
     let closePhName = '';
 
     if (!isVoid) {
-      closePhName = this._placeholderRegistry.getCloseTagPlaceholderName(el.name);
+      closePhName = context.placeholderRegistry.getCloseTagPlaceholderName(el.name);
-      this._placeholderToContent[closePhName] = `</${el.name}>`;
+      context.placeholderToContent[closePhName] = `</${el.name}>`;
     }
 
     const node = new i18n.TagPlaceholder(
         el.name, attrs, startPhName, closePhName, children, isVoid, el.sourceSpan !);
-    return this._visitNode(el, node);
+    return context.visitNodeFn(el, node);
   }
 
-  visitAttribute(attribute: html.Attribute, context: any): i18n.Node {
+  visitAttribute(attribute: html.Attribute, context: I18nMessageVisitorContext): i18n.Node {
-    const node = this._visitTextWithInterpolation(attribute.value, attribute.sourceSpan);
+    const node = this._visitTextWithInterpolation(attribute.value, attribute.sourceSpan, context);
-    return this._visitNode(attribute, node);
+    return context.visitNodeFn(attribute, node);
   }
 
-  visitText(text: html.Text, context: any): i18n.Node {
+  visitText(text: html.Text, context: I18nMessageVisitorContext): i18n.Node {
-    const node = this._visitTextWithInterpolation(text.value, text.sourceSpan !);
+    const node = this._visitTextWithInterpolation(text.value, text.sourceSpan !, context);
-    return this._visitNode(text, node);
+    return context.visitNodeFn(text, node);
   }
 
-  visitComment(comment: html.Comment, context: any): i18n.Node|null { return null; }
+  visitComment(comment: html.Comment, context: I18nMessageVisitorContext): i18n.Node|null {
+    return null;
+  }
 
-  visitExpansion(icu: html.Expansion, context: any): i18n.Node {
+  visitExpansion(icu: html.Expansion, context: I18nMessageVisitorContext): i18n.Node {
-    this._icuDepth++;
+    context.icuDepth++;
     const i18nIcuCases: {[k: string]: i18n.Node} = {};
     const i18nIcu = new i18n.Icu(icu.switchValue, icu.type, i18nIcuCases, icu.sourceSpan);
     icu.cases.forEach((caze): void => {
       i18nIcuCases[caze.value] = new i18n.Container(
-          caze.expression.map((node) => node.visit(this, {})), caze.expSourceSpan);
+          caze.expression.map((node) => node.visit(this, context)), caze.expSourceSpan);
     });
-    this._icuDepth--;
+    context.icuDepth--;
 
-    if (this._isIcu || this._icuDepth > 0) {
+    if (context.isIcu || context.icuDepth > 0) {
       // Returns an ICU node when:
       // - the message (vs a part of the message) is an ICU message, or
       // - the ICU message is nested.
-      const expPh = this._placeholderRegistry.getUniquePlaceholder(`VAR_${icu.type}`);
+      const expPh = context.placeholderRegistry.getUniquePlaceholder(`VAR_${icu.type}`);
       i18nIcu.expressionPlaceholder = expPh;
-      this._placeholderToContent[expPh] = icu.switchValue;
+      context.placeholderToContent[expPh] = icu.switchValue;
-      return this._visitNode(icu, i18nIcu);
+      return context.visitNodeFn(icu, i18nIcu);
     }
 
     // Else returns a placeholder
     // ICU placeholders should not be replaced with their original content but with the their
-    // translations. We need to create a new visitor (they are not re-entrant) to compute the
+    // translations.
-    // message id.
     // TODO(vicb): add a html.Node -> i18n.Message cache to avoid having to re-create the msg
-    const phName = this._placeholderRegistry.getPlaceholderName('ICU', icu.sourceSpan.toString());
+    const phName = context.placeholderRegistry.getPlaceholderName('ICU', icu.sourceSpan.toString());
-    const visitor = new _I18nVisitor(this._expressionParser, this._interpolationConfig);
+    context.placeholderToMessage[phName] = this.toI18nMessage([icu], '', '', '', undefined);
-    this._placeholderToMessage[phName] = visitor.toI18nMessage([icu], '', '', '');
     const node = new i18n.IcuPlaceholder(i18nIcu, phName, icu.sourceSpan);
-    return this._visitNode(icu, node);
+    return context.visitNodeFn(icu, node);
   }
 
-  visitExpansionCase(icuCase: html.ExpansionCase, context: any): i18n.Node {
+  visitExpansionCase(_icuCase: html.ExpansionCase, _context: I18nMessageVisitorContext): i18n.Node {
     throw new Error('Unreachable code');
   }
 
-  private _visitTextWithInterpolation(text: string, sourceSpan: ParseSourceSpan): i18n.Node {
+  private _visitTextWithInterpolation(
+      text: string, sourceSpan: ParseSourceSpan, context: I18nMessageVisitorContext): i18n.Node {
     const splitInterpolation = this._expressionParser.splitInterpolation(
         text, sourceSpan.start.toString(), this._interpolationConfig);
 
@@ -163,7 +163,7 @@ class _I18nVisitor implements html.Visitor {
     for (let i = 0; i < splitInterpolation.strings.length - 1; i++) {
       const expression = splitInterpolation.expressions[i];
       const baseName = _extractPlaceholderName(expression) || 'INTERPOLATION';
-      const phName = this._placeholderRegistry.getPlaceholderName(baseName, expression);
+      const phName = context.placeholderRegistry.getPlaceholderName(baseName, expression);
 
       if (splitInterpolation.strings[i].length) {
         // No need to add empty strings
@@ -171,7 +171,7 @@ class _I18nVisitor implements html.Visitor {
       }
 
       nodes.push(new i18n.Placeholder(expression, phName, sourceSpan));
-      this._placeholderToContent[phName] = sDelimiter + expression + eDelimiter;
+      context.placeholderToContent[phName] = sDelimiter + expression + eDelimiter;
     }
 
     // The last index contains no expression

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

@@ -8,7 +8,7 @@
 
 import {computeDecimalDigest, computeDigest, decimalDigest} from '../../../i18n/digest';
 import * as i18n from '../../../i18n/i18n_ast';
-import {createI18nMessageFactory} from '../../../i18n/i18n_parser';
+import {VisitNodeFn, createI18nMessageFactory} from '../../../i18n/i18n_parser';
 import * as html from '../../../ml_parser/ast';
 import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../ml_parser/interpolation_config';
 import * as o from '../../../output/output_ast';
@@ -23,8 +23,9 @@ export type I18nMeta = {
   meaning?: string
 };
 
-function setI18nRefs(html: html.Node & {i18n?: i18n.AST}, i18n: i18n.Node) {
+function setI18nRefs(html: html.Node & {i18n?: i18n.AST}, i18n: i18n.Node): i18n.Node {
   html.i18n = i18n;
+  return i18n;
 }
 
 /**
@@ -41,8 +42,7 @@ export class I18nMetaVisitor implements html.Visitor {
       private keepI18nAttrs: boolean = false, private i18nLegacyMessageIdFormat: string = '') {}
 
   private _generateI18nMessage(
-      nodes: html.Node[], meta: string|i18n.AST = '',
+      nodes: html.Node[], meta: string|i18n.AST = '', visitNodeFn?: VisitNodeFn): i18n.Message {
-      visitNodeFn?: (html: html.Node, i18n: i18n.Node) => void): i18n.Message {
     const parsed: I18nMeta =
         typeof meta === 'string' ? parseI18nMeta(meta) : metaFromI18nMessage(meta as i18n.Message);
     const message = this._createI18nMessage(