Add test cases for @ngInject escaping in tsickle

PiperOrigin-RevId: 629412411

Author: lauraharker

demo/package.json

@@ -8,7 +8,7 @@
   "dependencies": {
     "minimist": "^1.2.3",
     "tsickle": "file:../",
-    "typescript": "5.2.2"
+    "typescript": "5.4.2"
   },
   "devDependencies": {
     "@types/minimist": "1.2.0",

package.json

@@ -11,7 +11,7 @@
     "out/src/*"
   ],
   "peerDependencies": {
-    "typescript": "~5.1.5"
+    "typescript": "~5.4.2"
   },
   "devDependencies": {
     "@types/diff-match-patch": "^1.0.32",
@@ -28,7 +28,7 @@
     "source-map-support": "^0.5.19",
     "tslib": "^2.2.0",
     "tslint": "^6.1.3",
-    "typescript": "5.2.2"
+    "typescript": "5.4.2"
   },
   "scripts": {
     "build": "tsc",

src/clutz.ts

@@ -21,15 +21,19 @@ import * as googmodule from './googmodule';
 import * as path from './path';
 import {isDeclaredInClutzDts} from './type_translator';
 
+interface ClutzHost {
+    /** See compiler_host.ts */
+    rootDirsRelative(fileName: string): string;
+}
+
 /**
  * Constructs a ts.CustomTransformerFactory that postprocesses the .d.ts
  * that are generated by ordinary TypeScript compilations to add some
  * Clutz-specific logic.  See generateClutzAliases.
  */
 export function makeDeclarationTransformerFactory(
     typeChecker: ts.TypeChecker,
-    googmoduleHost: googmodule.GoogModuleProcessorHost):
-    ts.CustomTransformerFactory {
+    host: ClutzHost&googmodule.GoogModuleProcessorHost): ts.CustomTransformerFactory {
   return (context: ts.TransformationContext): ts.CustomTransformer => {
     return {
       transformBundle(): ts.Bundle {
@@ -49,8 +53,7 @@ export function makeDeclarationTransformerFactory(
         //   import 'path/to/the/js_file';
         // so to for that import to resolve, you need to first import the clutz
         // d.ts that defines that declared module.
-        const imports =
-            gatherNecessaryClutzImports(googmoduleHost, typeChecker, file);
+        const imports = gatherNecessaryClutzImports(host, typeChecker, file);
         let importStmts: ts.Statement[]|undefined;
         if (imports.length > 0) {
           importStmts = imports.map(fileName => {
@@ -66,22 +69,56 @@ export function makeDeclarationTransformerFactory(
         // Construct `declare global {}` in the Clutz namespace for symbols
         // Clutz might use.
         const globalBlock = generateClutzAliases(
-            file, googmoduleHost.pathToModuleName('', file.fileName),
-            typeChecker, options);
+            file, host.pathToModuleName('', file.fileName), typeChecker,
+            options);
 
         // Only need to transform file if we needed one of the above additions.
         if (!importStmts && !globalBlock) return file;
 
-        return ts.factory.updateSourceFile(file, [
-          ...(importStmts ?? []),
-          ...file.statements,
-          ...(globalBlock ? [globalBlock] : []),
-        ]);
+        return ts.factory.updateSourceFile(
+            file,
+            ts.setTextRange(
+                ts.factory.createNodeArray([
+                  ...(importStmts ?? []),
+                  ...file.statements,
+                  ...(globalBlock ? [globalBlock] : []),
+                ]),
+                file.statements),
+            file.isDeclarationFile,
+            file.referencedFiles.map(
+                f => fixRelativeReference(f, file, options, host)),
+            // /// <reference types="x" /> directives are ignored under bazel.
+            /*typeReferences=*/[]);
       }
     };
   };
 }
 
+/**
+ * Fixes a relative reference from an output file with respect to multiple
+ * rootDirs. See https://github.com/Microsoft/TypeScript/issues/8245 for
+ * details.
+ */
+function fixRelativeReference(
+    reference: ts.FileReference, origin: ts.SourceFile,
+    options: ts.CompilerOptions, host: ClutzHost): ts.FileReference {
+  if (!options.outDir || !options.rootDir) {
+    return reference;
+  }
+  const originDir = path.dirname(origin.fileName);
+  // Where TypeScript expects the output to be.
+  const expectedOutDir =
+      path.join(options.outDir, path.relative(options.rootDir, originDir));
+  const referencedFile = path.join(expectedOutDir, reference.fileName);
+  // Where the output is actually emitted.
+  const actualOutDir =
+      path.join(options.outDir, host.rootDirsRelative(originDir));
+  const fixedReference = path.relative(actualOutDir, referencedFile);
+
+  reference.fileName = fixedReference;
+  return reference;
+}
+
 /** Compares two strings and returns a number suitable for use in sort(). */
 function stringCompare(a: string, b: string): number {
   if (a < b) return -1;

src/decorators.ts

@@ -98,7 +98,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics:
   return (context: ts.TransformationContext) => {
     const result: ts.Transformer<ts.SourceFile> = (sourceFile: ts.SourceFile) => {
       let nodeNeedingGoogReflect: undefined|ts.Node = undefined;
-      const visitor: ts.Visitor = (node) => {
+      const visitor = (node: ts.Node) => {
         const replacementNode = rewriteDecorator(node);
         if (replacementNode) {
           nodeNeedingGoogReflect = node;
@@ -107,9 +107,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics:
         return ts.visitEachChild(node, visitor, context);
       };
       let updatedSourceFile =
-          // TODO: go/ts50upgrade - Remove after upgrade.
-          // tslint:disable-next-line:no-unnecessary-type-assertion
-          ts.visitNode(sourceFile, visitor, ts.isSourceFile)!;
+          ts.visitNode(sourceFile, visitor, ts.isSourceFile);
       if (nodeNeedingGoogReflect !== undefined) {
         const statements = [...updatedSourceFile.statements];
         const googModuleIndex = statements.findIndex(isGoogModuleStatement);

src/enum_transformer.ts

@@ -19,6 +19,7 @@
  * type resolve ("@type {Foo}").
  */
 
+import {TsickleHost} from 'tsickle';
 import * as ts from 'typescript';
 
 import * as jsdoc from './jsdoc';
@@ -95,7 +96,7 @@ export function getEnumType(typeChecker: ts.TypeChecker, enumDecl: ts.EnumDeclar
 /**
  * Transformer factory for the enum transformer. See fileoverview for details.
  */
-export function enumTransformer(typeChecker: ts.TypeChecker):
+export function enumTransformer(host: TsickleHost, typeChecker: ts.TypeChecker):
     (context: ts.TransformationContext) => ts.Transformer<ts.SourceFile> {
   return (context: ts.TransformationContext) => {
     function visitor<T extends ts.Node>(node: T): T|ts.Node[] {
@@ -180,7 +181,11 @@ export function enumTransformer(typeChecker: ts.TypeChecker):
                   /* modifiers */ undefined,
                   ts.factory.createVariableDeclarationList(
                       [varDecl],
-                      /* create a const var */ ts.NodeFlags.Const)),
+                      /* When using unoptimized namespaces, create a var
+                         declaration, otherwise create a const var. See b/157460535 */
+                      host.useDeclarationMergingTransformation ?
+                          ts.NodeFlags.Const :
+                          undefined)),
               node),
           node);
 

src/externs.ts

@@ -295,24 +295,13 @@ export function generateExterns(
    *   interface Foo { x: number; }
    *   interface Foo { y: number; }
    * we only want to emit the "\@record" for Foo on the first one.
-   *
-   * The exception are variable declarations, which - in externs - do not assign a value:
-   *   /.. \@type {...} ./
-   *   var someVariable;
-   *   /.. \@type {...} ./
-   *   someNamespace.someVariable;
-   * If a later declaration wants to add additional properties on someVariable, tsickle must still
-   * emit an assignment into the object, as it's otherwise absent.
    */
   function isFirstValueDeclaration(decl: ts.DeclarationStatement): boolean {
     if (!decl.name) return true;
     const sym = typeChecker.getSymbolAtLocation(decl.name)!;
     if (!sym.declarations || sym.declarations.length < 2) return true;
     const earlierDecls = sym.declarations.slice(0, sym.declarations.indexOf(decl));
-    // Either there are no earlier declarations, or all of them are variables (see above). tsickle
-    // emits a value for all other declaration kinds (function for functions, classes, interfaces,
-    // {} object for namespaces).
-    return earlierDecls.length === 0 || earlierDecls.every(ts.isVariableDeclaration);
+    return earlierDecls.length === 0 || earlierDecls.every(d => ts.isVariableDeclaration(d) && d.getSourceFile() !== decl.getSourceFile());
   }
 
   /** Writes the actual variable statement of a Closure variable declaration. */

src/fileoverview_comment_transformer.ts

@@ -10,7 +10,7 @@ import * as ts from 'typescript';
 
 import * as jsdoc from './jsdoc';
 import * as path from './path';
-import {createNotEmittedStatement, reportDiagnostic, synthesizeCommentRanges, updateSourceFileNode} from './transformer_util';
+import {reportDiagnostic, synthesizeCommentRanges, updateSourceFileNode} from './transformer_util';
 
 /**
  * A set of JSDoc tags that mark a comment as a fileoverview comment. These are
@@ -65,6 +65,9 @@ function augmentFileoverviewComments(
     // * Suppress uselessCode.  We emit an "if (false)" around type
     // declarations, which is flagged as unused code unless we suppress it.
       'uselessCode',
+    // suspiciousCode errors flag patterns that are suspicious if human-written
+    // but not inherently wrong. See also b/323580655.
+      'suspiciousCode',
     // * Suppress some checks for user errors that TS already checks.
       'missingReturn',
       'unusedPrivateMembers',
@@ -152,35 +155,34 @@ export function transformFileoverviewCommentFactory(
           // they do not get lost later on.
           const synthesizedComments =
               jsdoc.synthesizeLeadingComments(firstStatement);
-          const notEmitted = ts.factory.createNotEmittedStatement(sourceFile);
           // Modify the comments on the firstStatement in place by removing the
           // file-level comments.
           fileComments = synthesizedComments.splice(0, i + 1);
-          // Move the fileComments onto notEmitted.
-          ts.setSyntheticLeadingComments(notEmitted, fileComments);
-          sourceFile =
-              updateSourceFileNode(sourceFile, ts.factory.createNodeArray([
-                notEmitted, firstStatement, ...sourceFile.statements.slice(1)
-              ]));
           break;
         }
+      }
 
-
-        // Now walk every top level statement and escape/drop any @fileoverview
-        // comments found. Closure ignores all @fileoverview comments but the
-        // last, so tsickle must make sure not to emit duplicated ones.
-        for (let i = 0; i < sourceFile.statements.length; i++) {
-          const stmt = sourceFile.statements[i];
-          // Accept the NotEmittedStatement inserted above.
-          if (i === 0 && stmt.kind === ts.SyntaxKind.NotEmittedStatement) {
-            continue;
-          }
-          const comments = jsdoc.synthesizeLeadingComments(stmt);
-          checkNoFileoverviewComments(
-              stmt, comments,
-              `file comments must be at the top of the file, ` +
-                  `separated from the file body by an empty line.`);
+      // Move the fileComments onto notEmitted.
+      const notEmitted = ts.factory.createNotEmittedStatement(sourceFile);
+      ts.setSyntheticLeadingComments(notEmitted, fileComments);
+      sourceFile = updateSourceFileNode(
+          sourceFile,
+          ts.factory.createNodeArray([notEmitted, ...sourceFile.statements]));
+
+      // Now walk every top level statement and escape/drop any @fileoverview
+      // comments found. Closure ignores all @fileoverview comments but the
+      // last, so tsickle must make sure not to emit duplicated ones.
+      for (let i = 0; i < sourceFile.statements.length; i++) {
+        const stmt = sourceFile.statements[i];
+        // Accept the NotEmittedStatement inserted above.
+        if (i === 0 && stmt.kind === ts.SyntaxKind.NotEmittedStatement) {
+          continue;
         }
+        const comments = jsdoc.synthesizeLeadingComments(stmt);
+        checkNoFileoverviewComments(
+            stmt, comments,
+            `file comments must be at the top of the file, ` +
+                `separated from the file body by an empty line.`);
       }
 
       // Closure Compiler considers the *last* comment with @fileoverview (or
@@ -192,14 +194,17 @@ export function transformFileoverviewCommentFactory(
       let fileoverviewIdx = -1;
       let tags: jsdoc.Tag[] = [];
       for (let i = fileComments.length - 1; i >= 0; i--) {
-        const parse = jsdoc.parseContents(fileComments[i].text);
-        if (parse !== null &&
-            parse.tags.some(t => FILEOVERVIEW_COMMENT_MARKERS.has(t.tagName))) {
+        const parsed = jsdoc.parse(fileComments[i]);
+        if (parsed !== null &&
+            parsed.tags.some(
+                t => FILEOVERVIEW_COMMENT_MARKERS.has(t.tagName))) {
           fileoverviewIdx = i;
-          tags = parse.tags;
+          tags = parsed.tags;
           break;
         }
       }
+      const mutableJsDoc = new jsdoc.MutableJSDoc(
+          notEmitted, fileComments, fileoverviewIdx, tags);
 
       if (fileoverviewIdx !== -1) {
         checkNoFileoverviewComments(
@@ -208,28 +213,11 @@ export function transformFileoverviewCommentFactory(
             `duplicate file level comment`);
       }
 
-      augmentFileoverviewComments(options, sourceFile, tags, generateExtraSuppressions);
-      const commentText = jsdoc.toStringWithoutStartEnd(tags);
-
-      if (fileoverviewIdx < 0) {
-        // No existing comment to merge with, just emit a new one.
-        return addNewFileoverviewComment(sourceFile, commentText);
-      }
+      augmentFileoverviewComments(
+          options, sourceFile, mutableJsDoc.tags, generateExtraSuppressions);
+      mutableJsDoc.updateComment();
 
-      fileComments[fileoverviewIdx].text = commentText;
-      // sf does not need to be updated, synthesized comments are mutable.
       return sourceFile;
     };
   };
 }
-
-function addNewFileoverviewComment(
-    sf: ts.SourceFile, commentText: string): ts.SourceFile {
-  let syntheticFirstStatement = createNotEmittedStatement(sf);
-  syntheticFirstStatement = ts.addSyntheticTrailingComment(
-      syntheticFirstStatement, ts.SyntaxKind.MultiLineCommentTrivia,
-      commentText, true);
-  return updateSourceFileNode(
-      sf,
-      ts.factory.createNodeArray([syntheticFirstStatement, ...sf.statements]));
-}