Escape illegal JSDoc on enum declarations in unoptimized namespaces

PiperOrigin-RevId: 604603048

Author: frigus02

demo/package.json

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

package.json

@@ -11,7 +11,7 @@
     "out/src/*"
   ],
   "peerDependencies": {
-    "typescript": "~5.1.5"
+    "typescript": "~5.3.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.3.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. */