refactor(compiler-cli): `attachComments()` now expects a defined `leadingComments` array (#39076)

Previously the value passed to `AstFactory.attachComments()` could be
`undefined` which is counterintuitive, since why attach something that
doesn't exist? Now it expects there to be a defined array. Further it no
longer returns a statement. Both these aspects of the interface were designed
to make the usage simpler but has the result of complicating the implemenation.

The `ExpressionTranslatorVisitor` now has a helper function (`attachComments()`)
to handle `leadingComments` being undefined and also returning the statement.
This keeps the usage in the translator simple, while ensuring that the `AstFactory`
API is not influenced by how it is used.

PR Close #39076

packages/compiler-cli/linker/src/ast/babel/babel_ast_factory.ts

@@ -10,17 +10,16 @@ import * as t from '@babel/types';
 import {AstFactory, BinaryOperator, LeadingComment, ObjectLiteralProperty, SourceMapRange, TemplateLiteral, VariableDeclarationType} from '../../../../src/ngtsc/translator';
 import {assert} from '../utils';
 
+/**
+ * A Babel flavored implementation of the AstFactory.
+ */
 export class BabelAstFactory implements AstFactory<t.Statement, t.Expression> {
-  attachComments(statement: t.Statement, leadingComments: LeadingComment[]|undefined): t.Statement {
-    if (leadingComments === undefined) {
-      return statement;
-    }
+  attachComments(statement: t.Statement, leadingComments: LeadingComment[]): void {
     // We must process the comments in reverse because `t.addComment()` will add new ones in front.
     for (let i = leadingComments.length - 1; i >= 0; i--) {
       const comment = leadingComments[i];
       t.addComment(statement, 'leading', comment.toString(), !comment.multiline);
     }
-    return statement;
   }
 
   createArrayLiteral = t.arrayExpression;

packages/compiler-cli/src/ngtsc/translator/src/api/ast_factory.ts

@@ -17,11 +17,10 @@ export interface AstFactory<TStatement, TExpression> {
   /**
    * Attach the `leadingComments` to the given `statement` node.
    *
-   * @param statement the statement where the comment is to be attached.
+   * @param statement the statement where the comments are to be attached.
    * @param leadingComments the comments to attach.
-   * @returns the node passed in as `statement` with the comments attached.
    */
-  attachComments(statement: TStatement, leadingComments: LeadingComment[]|undefined): TStatement;
+  attachComments(statement: TStatement, leadingComments: LeadingComment[]): void;
 
   /**
    * Create a literal array expresion (e.g. `[expr1, expr2]`).

packages/compiler-cli/src/ngtsc/translator/src/translator.ts

@@ -61,14 +61,14 @@ export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.E
     const varType = this.downlevelVariableDeclarations ?
         'var' :
         stmt.hasModifier(o.StmtModifier.Final) ? 'const' : 'let';
-    return this.factory.attachComments(
+    return this.attachComments(
         this.factory.createVariableDeclaration(
             stmt.name, stmt.value?.visitExpression(this, context.withExpressionMode), varType),
         stmt.leadingComments);
   }
 
   visitDeclareFunctionStmt(stmt: o.DeclareFunctionStmt, context: Context): TStatement {
-    return this.factory.attachComments(
+    return this.attachComments(
         this.factory.createFunctionDeclaration(
             stmt.name, stmt.params.map(param => param.name),
             this.factory.createBlock(
@@ -77,14 +77,14 @@ export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.E
   }
 
   visitExpressionStmt(stmt: o.ExpressionStatement, context: Context): TStatement {
-    return this.factory.attachComments(
+    return this.attachComments(
         this.factory.createExpressionStatement(
             stmt.expr.visitExpression(this, context.withStatementMode)),
         stmt.leadingComments);
   }
 
   visitReturnStmt(stmt: o.ReturnStatement, context: Context): TStatement {
-    return this.factory.attachComments(
+    return this.attachComments(
         this.factory.createReturnStatement(
             stmt.value.visitExpression(this, context.withExpressionMode)),
         stmt.leadingComments);
@@ -95,7 +95,7 @@ export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.E
   }
 
   visitIfStmt(stmt: o.IfStmt, context: Context): TStatement {
-    return this.factory.attachComments(
+    return this.attachComments(
         this.factory.createIfStatement(
             stmt.condition.visitExpression(this, context),
             this.factory.createBlock(
@@ -111,7 +111,7 @@ export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.E
   }
 
   visitThrowStmt(stmt: o.ThrowStmt, context: Context): TStatement {
-    return this.factory.attachComments(
+    return this.attachComments(
         this.factory.createThrowStatement(
             stmt.error.visitExpression(this, context.withExpressionMode)),
         stmt.leadingComments);
@@ -387,6 +387,14 @@ export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.E
       T {
     return this.factory.setSourceMapRange(ast, createRange(span));
   }
+
+  private attachComments(statement: TStatement, leadingComments: o.LeadingComment[]|undefined):
+      TStatement {
+    if (leadingComments !== undefined) {
+      this.factory.attachComments(statement, leadingComments);
+    }
+    return statement;
+  }
 }
 
 /**

packages/compiler-cli/src/ngtsc/translator/src/typescript_ast_factory.ts

@@ -234,12 +234,7 @@ export function createTemplateTail(cooked: string, raw: string): ts.TemplateTail
  * @param statement The statement that will have comments attached.
  * @param leadingComments The comments to attach to the statement.
  */
-export function attachComments<T extends ts.Statement>(
-    statement: T, leadingComments: LeadingComment[]|undefined): T {
-  if (leadingComments === undefined) {
-    return statement;
-  }
-
+export function attachComments(statement: ts.Statement, leadingComments: LeadingComment[]): void {
   for (const comment of leadingComments) {
     const commentKind = comment.multiline ? ts.SyntaxKind.MultiLineCommentTrivia :
                                             ts.SyntaxKind.SingleLineCommentTrivia;
@@ -252,5 +247,4 @@ export function attachComments<T extends ts.Statement>(
       }
     }
   }
-  return statement;
 }

packages/compiler-cli/src/transformers/node_emitter.ts

@@ -287,7 +287,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
     if (tsNode && !this._nodeMap.has(tsNode)) {
       this._nodeMap.set(tsNode, ngNode);
     }
-    if (tsNode !== null && ngNode instanceof Statement) {
+    if (tsNode !== null && ngNode instanceof Statement && ngNode.leadingComments !== undefined) {
       attachComments(tsNode as unknown as ts.Statement, ngNode.leadingComments);
     }
     return tsNode as RecordedNode<T>;