refactor(compiler): replace Comment nodes with leadingComments proper…

…ty (#38811)

Common AST formats such as TS and Babel do not use a separate
node for comments, but instead attach comments to other AST nodes.
Previously this was worked around in TS by creating a `NotEmittedStatement`
AST node to attach the comment to. But Babel does not have this facility,
so it will not be a viable approach for the linker.

This commit refactors the output AST, to remove the `CommentStmt` and
`JSDocCommentStmt` nodes. Instead statements have a collection of
`leadingComments` that are rendered/attached to the final AST nodes
when being translated or printed.

PR Close #38811

packages/compiler-cli/BUILD.bazel

@@ -32,6 +32,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/perf",
         "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/compiler-cli/src/ngtsc/shims",
+        "//packages/compiler-cli/src/ngtsc/translator",
         "//packages/compiler-cli/src/ngtsc/typecheck",
         "@npm//@bazel/typescript",
         "@npm//@types/node",

packages/compiler-cli/ngcc/src/rendering/renderer.ts

@@ -5,7 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {CommentStmt, ConstantPool, Expression, Statement, WrappedNodeExpr, WritePropExpr} from '@angular/compiler';
+import {ConstantPool, Expression, jsDocComment, LeadingComment, Statement, WrappedNodeExpr, WritePropExpr} from '@angular/compiler';
 import MagicString from 'magic-string';
 import * as ts from 'typescript';
 
@@ -166,11 +166,11 @@ export class Renderer {
       sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager,
       annotateForClosureCompiler: boolean): string {
     const name = this.host.getInternalNameOfClass(compiledClass.declaration);
-    const statements: Statement[][] = compiledClass.compilation.map(c => {
-      return createAssignmentStatements(
-          name, c.name, c.initializer, annotateForClosureCompiler ? '* @nocollapse ' : undefined);
-    });
-    return this.renderStatements(sourceFile, Array.prototype.concat.apply([], statements), imports);
+    const leadingComment =
+        annotateForClosureCompiler ? jsDocComment([{tagName: 'nocollapse'}]) : undefined;
+    const statements: Statement[] = compiledClass.compilation.map(
+        c => createAssignmentStatement(name, c.name, c.initializer, leadingComment));
+    return this.renderStatements(sourceFile, statements, imports);
   }
 
   /**
@@ -213,16 +213,16 @@ export function renderConstantPool(
  * compiled decorator to be applied to the class.
  * @param analyzedClass The info about the class whose statement we want to create.
  */
-function createAssignmentStatements(
+function createAssignmentStatement(
     receiverName: ts.DeclarationName, propName: string, initializer: Expression,
-    leadingComment?: string): Statement[] {
+    leadingComment?: LeadingComment): Statement {
   const receiver = new WrappedNodeExpr(receiverName);
-  const statements =
-      [new WritePropExpr(
-           receiver, propName, initializer, /* type */ undefined, /* sourceSpan */ undefined)
-           .toStmt()];
+  const statement =
+      new WritePropExpr(
+          receiver, propName, initializer, /* type */ undefined, /* sourceSpan */ undefined)
+          .toStmt();
   if (leadingComment !== undefined) {
-    statements.unshift(new CommentStmt(leadingComment, true));
+    statement.addLeadingComment(leadingComment);
   }
-  return statements;
+  return statement;
 }

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -1663,12 +1663,12 @@ runInEachFileSystem(() => {
             JSON.stringify({angularCompilerOptions: {annotateForClosureCompiler: true}}));
         mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']});
         const jsContents = fs.readFile(_(`/dist/local-package/index.js`));
-        expect(jsContents).toContain('/** @nocollapse */ \nAppComponent.ɵcmp =');
+        expect(jsContents).toContain('/** @nocollapse */\nAppComponent.ɵcmp =');
       });
       it('should default to not give closure annotated output', () => {
         mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']});
         const jsContents = fs.readFile(_(`/dist/local-package/index.js`));
-        expect(jsContents).not.toContain('/** @nocollapse */');
+        expect(jsContents).not.toContain('@nocollapse');
       });
     });
 

packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts

@@ -103,7 +103,7 @@ export class ModuleWithProvidersScanner {
         this.emitter.emit(ngModule, decl.getSourceFile(), ImportFlags.ForceNewImport);
     const ngModuleType = new ExpressionType(ngModuleExpr);
     const mwpNgType = new ExpressionType(
-        new ExternalExpr(Identifiers.ModuleWithProviders), /* modifiers */ null, [ngModuleType]);
+        new ExternalExpr(Identifiers.ModuleWithProviders), [/* modifiers */], [ngModuleType]);
 
     dts.addTypeReplacement(decl, mwpNgType);
   }

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

@@ -6,4 +6,4 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-export {Import, ImportManager, NamedImport, translateExpression, translateStatement, translateType} from './src/translator';
+export {attachComments, Import, ImportManager, NamedImport, translateExpression, translateStatement, translateType} from './src/translator';

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

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ParseSourceSpan, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeofExpr, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler';
+import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, LeadingComment, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ParseSourceSpan, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeofExpr, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler';
 import {LocalizedString, UnaryOperator, UnaryOperatorExpr} from '@angular/compiler/src/output/output_ast';
 import * as ts from 'typescript';
 
@@ -134,34 +134,45 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
       private scriptTarget: Exclude<ts.ScriptTarget, ts.ScriptTarget.JSON>) {}
 
   visitDeclareVarStmt(stmt: DeclareVarStmt, context: Context): ts.VariableStatement {
-    const nodeFlags =
-        ((this.scriptTarget >= ts.ScriptTarget.ES2015) && stmt.hasModifier(StmtModifier.Final)) ?
-        ts.NodeFlags.Const :
-        ts.NodeFlags.None;
-    return ts.createVariableStatement(
-        undefined,
-        ts.createVariableDeclarationList(
-            [ts.createVariableDeclaration(
-                stmt.name, undefined,
-                stmt.value && stmt.value.visitExpression(this, context.withExpressionMode))],
-            nodeFlags));
+    const isConst =
+        this.scriptTarget >= ts.ScriptTarget.ES2015 && stmt.hasModifier(StmtModifier.Final);
+    const varDeclaration = ts.createVariableDeclaration(
+        /* name */ stmt.name,
+        /* type */ undefined,
+        /* initializer */ stmt.value?.visitExpression(this, context.withExpressionMode));
+    const declarationList = ts.createVariableDeclarationList(
+        /* declarations */[varDeclaration],
+        /* flags */ isConst ? ts.NodeFlags.Const : ts.NodeFlags.None);
+    const varStatement = ts.createVariableStatement(undefined, declarationList);
+    return attachComments(varStatement, stmt.leadingComments);
   }
 
   visitDeclareFunctionStmt(stmt: DeclareFunctionStmt, context: Context): ts.FunctionDeclaration {
-    return ts.createFunctionDeclaration(
-        undefined, undefined, undefined, stmt.name, undefined,
+    const fnDeclaration = ts.createFunctionDeclaration(
+        /* decorators */ undefined,
+        /* modifiers */ undefined,
+        /* asterisk */ undefined,
+        /* name */ stmt.name,
+        /* typeParameters */ undefined,
+        /* parameters */
         stmt.params.map(param => ts.createParameter(undefined, undefined, undefined, param.name)),
-        undefined,
+        /* type */ undefined,
+        /* body */
         ts.createBlock(
             stmt.statements.map(child => child.visitStatement(this, context.withStatementMode))));
+    return attachComments(fnDeclaration, stmt.leadingComments);
   }
 
   visitExpressionStmt(stmt: ExpressionStatement, context: Context): ts.ExpressionStatement {
-    return ts.createStatement(stmt.expr.visitExpression(this, context.withStatementMode));
+    return attachComments(
+        ts.createStatement(stmt.expr.visitExpression(this, context.withStatementMode)),
+        stmt.leadingComments);
   }
 
   visitReturnStmt(stmt: ReturnStatement, context: Context): ts.ReturnStatement {
-    return ts.createReturn(stmt.value.visitExpression(this, context.withExpressionMode));
+    return attachComments(
+        ts.createReturn(stmt.value.visitExpression(this, context.withExpressionMode)),
+        stmt.leadingComments);
   }
 
   visitDeclareClassStmt(stmt: ClassStmt, context: Context) {
@@ -174,40 +185,25 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
   }
 
   visitIfStmt(stmt: IfStmt, context: Context): ts.IfStatement {
-    return ts.createIf(
-        stmt.condition.visitExpression(this, context),
+    const thenBlock = ts.createBlock(
+        stmt.trueCase.map(child => child.visitStatement(this, context.withStatementMode)));
+    const elseBlock = stmt.falseCase.length > 0 ?
         ts.createBlock(
-            stmt.trueCase.map(child => child.visitStatement(this, context.withStatementMode))),
-        stmt.falseCase.length > 0 ?
-            ts.createBlock(stmt.falseCase.map(
-                child => child.visitStatement(this, context.withStatementMode))) :
-            undefined);
+            stmt.falseCase.map(child => child.visitStatement(this, context.withStatementMode))) :
+        undefined;
+    const ifStatement =
+        ts.createIf(stmt.condition.visitExpression(this, context), thenBlock, elseBlock);
+    return attachComments(ifStatement, stmt.leadingComments);
   }
 
   visitTryCatchStmt(stmt: TryCatchStmt, context: Context) {
     throw new Error('Method not implemented.');
   }
 
   visitThrowStmt(stmt: ThrowStmt, context: Context): ts.ThrowStatement {
-    return ts.createThrow(stmt.error.visitExpression(this, context.withExpressionMode));
-  }
-
-  visitCommentStmt(stmt: CommentStmt, context: Context): ts.NotEmittedStatement {
-    const commentStmt = ts.createNotEmittedStatement(ts.createLiteral(''));
-    ts.addSyntheticLeadingComment(
-        commentStmt,
-        stmt.multiline ? ts.SyntaxKind.MultiLineCommentTrivia :
-                         ts.SyntaxKind.SingleLineCommentTrivia,
-        stmt.comment, /** hasTrailingNewLine */ false);
-    return commentStmt;
-  }
-
-  visitJSDocCommentStmt(stmt: JSDocCommentStmt, context: Context): ts.NotEmittedStatement {
-    const commentStmt = ts.createNotEmittedStatement(ts.createLiteral(''));
-    const text = stmt.toString();
-    const kind = ts.SyntaxKind.MultiLineCommentTrivia;
-    ts.setSyntheticLeadingComments(commentStmt, [{kind, text, pos: -1, end: -1}]);
-    return commentStmt;
+    return attachComments(
+        ts.createThrow(stmt.error.visitExpression(this, context.withExpressionMode)),
+        stmt.leadingComments);
   }
 
   visitReadVarExpr(ast: ReadVarExpr, context: Context): ts.Identifier {
@@ -784,4 +780,31 @@ function createTemplateTail(cooked: string, raw: string): ts.TemplateTail {
   const node: ts.TemplateLiteralLikeNode = ts.createTemplateHead(cooked, raw);
   (node.kind as ts.SyntaxKind) = ts.SyntaxKind.TemplateTail;
   return node as ts.TemplateTail;
-}
+}
+
+/**
+ * Attach the given `leadingComments` to the `statement` node.
+ *
+ * @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[]): T {
+  if (leadingComments === undefined) {
+    return statement;
+  }
+
+  for (const comment of leadingComments) {
+    const commentKind = comment.multiline ? ts.SyntaxKind.MultiLineCommentTrivia :
+                                            ts.SyntaxKind.SingleLineCommentTrivia;
+    if (comment.multiline) {
+      ts.addSyntheticLeadingComment(
+          statement, commentKind, comment.toString(), comment.trailingNewline);
+    } else {
+      for (const line of comment.text.split('\n')) {
+        ts.addSyntheticLeadingComment(statement, commentKind, line, comment.trailingNewline);
+      }
+    }
+  }
+  return statement;
+}

packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts

@@ -245,7 +245,8 @@ export class Environment {
    */
   referenceExternalType(moduleName: string, name: string, typeParams?: Type[]): ts.TypeNode {
     const external = new ExternalExpr({moduleName, name});
-    return translateType(new ExpressionType(external, null, typeParams), this.importManager);
+    return translateType(
+        new ExpressionType(external, [/* modifiers */], typeParams), this.importManager);
   }
 
   getPreludeStatements(): ts.Statement[] {