refactor(compiler-cli): properly preserve file overview comments (#54819)

devversion · alxhub · commit d01576b104fb · 2024-03-15T15:09:41.000-07:00
This commit updates the logic for preserving file overview comments to be more reliable and less dependent on previous transforms. Previously, with the old import manager, we had a utility called `addImport` that always separated import statements and non-import statements. This meant that the non-emitted statement from Tsickle for the synthetic file-overview comments no longer lived at the beginning of the file. `addImports` tried to overcome this by adding another new non-emitted statement *before* all imports. This then was later used by the transform (or was assumed!) to attach the synthetic file overview comments if the original tsickle AST Node is no longer at the top. This logic can be improved, because the import manager shouldn't need to bother about this fileoverview non-emitted statement, and the logic for re-attaching the fileoverview comment should be local. This commit fixes this and makes it a local transform. PR Close #54819
diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts
@@ -341,7 +341,7 @@ function transformIvySourceFile(
   sf = importManager.transformTsFile(context, sf, constants);
 
   if (fileOverviewMeta !== null) {
-    setFileOverviewComment(sf, fileOverviewMeta);
+    sf = insertFileOverviewComment(sf, fileOverviewMeta);
   }
 
   return sf;
@@ -382,7 +382,8 @@ function getFileOverviewComment(statements: ts.NodeArray<ts.Statement>): FileOve
   return null;
 }
 
-function setFileOverviewComment(sf: ts.SourceFile, fileoverview: FileOverviewMeta): void {
+function insertFileOverviewComment(
+    sf: ts.SourceFile, fileoverview: FileOverviewMeta): ts.SourceFile {
   const {comments, host, trailing} = fileoverview;
   // If host statement is no longer the first one, it means that extra statements were added at the
   // very beginning, so we need to relocate @fileoverview comment and cleanup the original statement
@@ -393,8 +394,17 @@ function setFileOverviewComment(sf: ts.SourceFile, fileoverview: FileOverviewMet
     } else {
       ts.setSyntheticLeadingComments(host, undefined);
     }
-    ts.setSyntheticLeadingComments(sf.statements[0], comments);
+
+    // Note: Do not use the first statement as it may be elided at runtime.
+    // E.g. an import declaration that is type only.
+    const commentNode = ts.factory.createNotEmittedStatement(sf);
+    ts.setSyntheticLeadingComments(commentNode, comments);
+
+    return ts.factory.updateSourceFile(
+        sf, [commentNode, ...sf.statements], sf.isDeclarationFile, sf.referencedFiles,
+        sf.typeReferenceDirectives, sf.hasNoDefaultLib, sf.libReferenceDirectives);
   }
+  return sf;
 }
 
 function maybeFilterDecorator(
diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -6,11 +6,14 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {NgtscProgram} from '@angular/compiler-cli/src/ngtsc/program';
+import {CompilerOptions} from '@angular/compiler-cli/src/transformers/api';
+import {createCompilerHost} from '@angular/compiler-cli/src/transformers/compiler_host';
 import {platform} from 'os';
 import ts from 'typescript';
 
 import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics';
-import {absoluteFrom} from '../../src/ngtsc/file_system';
+import {absoluteFrom, NgtscCompilerHost} from '../../src/ngtsc/file_system';
 import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
 import {loadStandardTestFiles} from '../../src/ngtsc/testing';
 import {restoreTypeScriptVersionForTesting, setTypeScriptVersionForTesting} from '../../src/typescript_support';
@@ -9294,6 +9297,60 @@ function allTests(os: string) {
             .toContain(
                 '{ i0.ɵɵtwoWayBindingSet(ctx.a, $event) || (ctx.a = $event); return $event; }');
       });
+
+      describe('tsickle compatibility', () => {
+        it('should preserve fileoverview comments', () => {
+          env.write('test.ts', `
+            // type-only import that will be elided.
+            import {SimpleChanges} from '@angular/core';
+
+            export class X {
+              p: SimpleChanges = null!;
+            }
+          `);
+
+          const options: CompilerOptions = {
+            strict: true,
+            strictTemplates: true,
+            target: ts.ScriptTarget.Latest,
+            module: ts.ModuleKind.ESNext,
+            annotateForClosureCompiler: true,
+          };
+
+          const program = new NgtscProgram(['/test.ts'], options, createCompilerHost({options}));
+          const transformers = program.compiler.prepareEmit().transformers;
+
+          // Add a "fake tsickle" transform before Angular's transform.
+          transformers.before!.unshift(ctx => (sf: ts.SourceFile) => {
+            const tsickleFileOverview = ctx.factory.createNotEmittedStatement(sf);
+            ts.setSyntheticLeadingComments(tsickleFileOverview, [
+              {
+                kind: ts.SyntaxKind.MultiLineCommentTrivia,
+                text: `*
+                    * @fileoverview Closure comment
+                    * @supress bla1
+                    * @supress bla2
+                  `,
+                pos: -1,
+                end: -1,
+                hasTrailingNewLine: true,
+              },
+            ]);
+            return ctx.factory.updateSourceFile(
+                sf, [tsickleFileOverview, ...sf.statements], sf.isDeclarationFile,
+                sf.referencedFiles, sf.typeReferenceDirectives, sf.hasNoDefaultLib,
+                sf.libReferenceDirectives);
+          });
+
+          const {diagnostics, emitSkipped} =
+              program.getTsProgram().emit(undefined, undefined, undefined, undefined, transformers);
+
+          expect(diagnostics.length).toBe(0);
+          expect(emitSkipped).toBe(false);
+
+          expect(env.getContents('/test.js')).toContain(`* @fileoverview Closure comment`);
+        });
+      });
     });
   });
 
