Fix enum declaration merging in unoptimized namespaces

In build targets with --allow_unoptimized_namesapces, emit enum declarations as `var E = {...}` instead of const. That fixes compilation errors when a namespace is merged with the enum.

PiperOrigin-RevId: 587866111

package.json

@@ -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/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/googmodule.ts

@@ -29,11 +29,9 @@ export interface GoogModuleProcessorHost {
    * Takes the import URL of an ES6 import and returns the googmodule module
    * name for the imported module, iff the module is an original closure
    * JavaScript file.
-   *
-   * Warning: If this function is present, GoogModule won't produce diagnostics
-   * for multiple provides.
    */
-  jsPathToModuleName?(importPath: string): string|undefined;
+  jsPathToModuleName?
+      (importPath: string): {name: string, multipleProvides: boolean}|undefined;
   /**
    * Takes the import URL of an ES6 import and returns the property name that
    * should be stripped from the usage.
@@ -89,7 +87,8 @@ export function jsPathToNamespace(
     host: GoogModuleProcessorHost, context: ts.Node,
     diagnostics: ts.Diagnostic[], importPath: string,
     getModuleSymbol: () => ts.Symbol | undefined): string|undefined {
-  const namespace = localJsPathToNamespace(host, importPath);
+  const namespace =
+      localJsPathToNamespace(host, context, diagnostics, importPath);
   if (namespace) return namespace;
 
   const moduleSymbol = getModuleSymbol();
@@ -105,15 +104,21 @@ export function jsPathToNamespace(
  * Forwards to `jsPathToModuleName` on the host if present.
  */
 export function localJsPathToNamespace(
-    host: GoogModuleProcessorHost, importPath: string): string|undefined {
+    host: GoogModuleProcessorHost, context: ts.Node|undefined,
+    diagnostics: ts.Diagnostic[], importPath: string): string|undefined {
   if (importPath.match(/^goog:/)) {
     // This is a namespace import, of the form "goog:foo.bar".
     // Fix it to just "foo.bar".
     return importPath.substring('goog:'.length);
   }
 
   if (host.jsPathToModuleName) {
-    return host.jsPathToModuleName(importPath);
+    const module = host.jsPathToModuleName(importPath);
+    if (!module) return undefined;
+    if (module.multipleProvides) {
+      reportMultipleProvidesError(context, diagnostics, importPath);
+    }
+    return module.name;
   }
 
   return undefined;
@@ -394,10 +399,7 @@ function getGoogNamespaceFromClutzComments(
       findLocalInDeclarations(moduleSymbol, '__clutz_multiple_provides');
   if (hasMultipleProvides) {
     // Report an error...
-    reportDiagnostic(
-        tsickleDiagnostics, context,
-        `referenced JavaScript module ${
-            tsImport} provides multiple namespaces and cannot be imported by path.`);
+    reportMultipleProvidesError(context, tsickleDiagnostics, tsImport);
     // ... but continue producing an emit that effectively references the first
     // provided symbol (to continue finding any additional errors).
   }
@@ -411,6 +413,15 @@ function getGoogNamespaceFromClutzComments(
   return actualNamespace;
 }
 
+function reportMultipleProvidesError(
+    context: ts.Node|undefined, diagnostics: ts.Diagnostic[],
+    importPath: string) {
+  reportDiagnostic(
+      diagnostics, context,
+      `referenced JavaScript module ${
+          importPath} provides multiple namespaces and cannot be imported by path.`);
+}
+
 /**
  * Converts a TS/ES module './import/path' into a goog.module compatible
  * namespace, handling regular imports and `goog:` namespace imports.

src/jsdoc_transformer.ts

@@ -846,10 +846,9 @@ export function jsdocTransformer(
               continue;
             }
           }
-          const newDecl =
-              // TODO: go/ts50upgrade - Remove after upgrade.
-              // tslint:disable-next-line:no-unnecessary-type-assertion
-              ts.visitNode(decl, visitor, ts.isVariableDeclaration)!;
+          const newDecl = ts.setEmitFlags(
+              ts.visitNode(decl, visitor, ts.isVariableDeclaration)!,
+              ts.EmitFlags.NoComments);
           const newStmt = ts.factory.createVariableStatement(
               varStmt.modifiers,
               ts.factory.createVariableDeclarationList([newDecl], flags));

src/ns_transformer.ts

@@ -72,8 +72,8 @@ export function namespaceTransformer(
       // transformation fails.
       function transformNamespace(
           ns: ts.ModuleDeclaration,
-          mergedDecl: ts.ClassDeclaration|
-          ts.InterfaceDeclaration): ts.Statement[] {
+          mergedDecl: ts.ClassDeclaration|ts.InterfaceDeclaration|
+          ts.EnumDeclaration): ts.Statement[] {
         if (!ns.body || !ts.isModuleBlock(ns.body)) {
           if (ts.isModuleDeclaration(ns)) {
             error(
@@ -83,10 +83,15 @@ export function namespaceTransformer(
           return [ns];
         }
         const nsName = getIdentifierText(ns.name as ts.Identifier);
+        const mergingWithEnum = ts.isEnumDeclaration(mergedDecl);
         const transformedNsStmts: ts.Statement[] = [];
         for (const stmt of ns.body.statements) {
           if (ts.isEmptyStatement(stmt)) continue;
           if (ts.isClassDeclaration(stmt)) {
+            if (mergingWithEnum) {
+              errorNotAllowed(stmt, 'class');
+              continue;
+            }
             transformInnerDeclaration(
                 stmt, (classDecl, notExported, hoistedIdent) => {
                   return ts.factory.updateClassDeclaration(
@@ -95,12 +100,20 @@ export function namespaceTransformer(
                       classDecl.members);
                 });
           } else if (ts.isEnumDeclaration(stmt)) {
+            if (mergingWithEnum) {
+              errorNotAllowed(stmt, 'enum');
+              continue;
+            }
             transformInnerDeclaration(
                 stmt, (enumDecl, notExported, hoistedIdent) => {
                   return ts.factory.updateEnumDeclaration(
                       enumDecl, notExported, hoistedIdent, enumDecl.members);
                 });
           } else if (ts.isInterfaceDeclaration(stmt)) {
+            if (mergingWithEnum) {
+              errorNotAllowed(stmt, 'interface');
+              continue;
+            }
             transformInnerDeclaration(
                 stmt, (interfDecl, notExported, hoistedIdent) => {
                   return ts.factory.updateInterfaceDeclaration(
@@ -109,20 +122,39 @@ export function namespaceTransformer(
                       interfDecl.members);
                 });
           } else if (ts.isTypeAliasDeclaration(stmt)) {
+            if (mergingWithEnum) {
+              errorNotAllowed(stmt, 'type alias');
+              continue;
+            }
             transformTypeAliasDeclaration(stmt);
           } else if (ts.isVariableStatement(stmt)) {
             if ((ts.getCombinedNodeFlags(stmt.declarationList) &
                  ts.NodeFlags.Const) === 0) {
               error(
                   stmt,
                   'non-const values are not supported. (go/ts-merged-namespaces)');
+              continue;
             }
             if (!ts.isInterfaceDeclaration(mergedDecl)) {
               error(
                   stmt,
                   'const declaration only allowed when merging with an interface (go/ts-merged-namespaces)');
+              continue;
             }
             transformConstDeclaration(stmt);
+          } else if (ts.isFunctionDeclaration(stmt)) {
+            if (!ts.isEnumDeclaration(mergedDecl)) {
+              error(
+                  stmt,
+                  'function declaration only allowed when merging with an enum (go/ts-merged-namespaces)');
+            }
+            transformInnerDeclaration(
+                stmt, (funcDecl, notExported, hoistedIdent) => {
+                  return ts.factory.updateFunctionDeclaration(
+                      funcDecl, notExported, funcDecl.asteriskToken,
+                      hoistedIdent, funcDecl.typeParameters,
+                      funcDecl.parameters, funcDecl.type, funcDecl.body);
+                });
           } else {
             error(
                 stmt,
@@ -145,6 +177,12 @@ export function namespaceTransformer(
 
         // Local functions follow.
 
+        function errorNotAllowed(stmt: ts.Statement, declKind: string) {
+          error(
+              stmt,
+              `${declKind} cannot be merged with enum declaration. (go/ts-merged-namespaces)`);
+        }
+
         type DeclarationStatement = ts.Declaration&ts.DeclarationStatement;
 
         function transformConstDeclaration(varDecl: ts.VariableStatement) {
@@ -365,12 +403,13 @@ export function namespaceTransformer(
         }
 
         if (!ts.isInterfaceDeclaration(mergedDecl) &&
-            !ts.isClassDeclaration(mergedDecl)) {
-          // The previous declaration is not a class or interface.
+            !ts.isClassDeclaration(mergedDecl) &&
+            !ts.isEnumDeclaration(mergedDecl)) {
+          // The previous declaration is not a class, enum, or interface.
           transformedStmts.push(ns);  // Nothing to do here.
           error(
               ns.name,
-              'merged declaration must be local class or interface. (go/ts-merged-namespaces)');
+              'merged declaration must be local class, enum, or interface. (go/ts-merged-namespaces)');
           return;
         }
 

src/summary.ts

@@ -54,6 +54,7 @@ export class FileSummary {
   modName: string|undefined;
   autochunk = false;
   enhanceable = false;
+  legacyNamespace = false;
   moduleType = ModuleType.UNKNOWN;
 
   private stringify(symbol: Symbol): string {

src/transformer_util.ts

@@ -227,28 +227,30 @@ export function reportDebugWarning(
  * @param textRange pass to overrride the text range from the node with a more specific range.
  */
 export function reportDiagnostic(
-    diagnostics: ts.Diagnostic[], node: ts.Node, messageText: string, textRange?: ts.TextRange,
-    category = ts.DiagnosticCategory.Error) {
+    diagnostics: ts.Diagnostic[], node: ts.Node|undefined, messageText: string,
+    textRange?: ts.TextRange, category = ts.DiagnosticCategory.Error) {
   diagnostics.push(createDiagnostic(node, messageText, textRange, category));
 }
 
 function createDiagnostic(
-    node: ts.Node, messageText: string, textRange: ts.TextRange|undefined,
+    node: ts.Node|undefined, messageText: string,
+    textRange: ts.TextRange|undefined,
     category: ts.DiagnosticCategory): ts.Diagnostic {
-  let start, length: number;
+  let start: number|undefined;
+  let length: number|undefined;
   // getStart on a synthesized node can crash (due to not finding an associated
   // source file). Make sure to use the original node.
   node = ts.getOriginalNode(node);
   if (textRange) {
     start = textRange.pos;
     length = textRange.end - textRange.pos;
-  } else {
+  } else if (node) {
     // Only use getStart if node has a valid pos, as it might be synthesized.
     start = node.pos >= 0 ? node.getStart() : 0;
     length = node.end - node.pos;
   }
   return {
-    file: node.getSourceFile(),
+    file: node?.getSourceFile(),
     start,
     length,
     messageText,
@@ -431,4 +433,4 @@ export function getPreviousDeclaration(
     }
   }
   return null;
-}
+}

src/ts_migration_exports_shim.ts

@@ -457,6 +457,9 @@ class Generator {
     fileSummary.addStrongRequire({type: Type.CLOSURE, name: 'goog'});
     fileSummary.addStrongRequire(
         {type: Type.CLOSURE, name: this.srcIds.googModuleId});
+    if (maybeDeclareLegacyNameCall) {
+      fileSummary.legacyNamespace = true;
+    }
 
     fileSummary.autochunk = isAutoChunk;
     fileSummary.moduleType = ModuleType.GOOG_MODULE;

src/tsickle.ts

@@ -222,7 +222,7 @@ export function emit(
     }
     tsickleSourceTransformers.push(
         jsdocTransformer(host, tsOptions, typeChecker, tsickleDiagnostics));
-    tsickleSourceTransformers.push(enumTransformer(typeChecker));
+    tsickleSourceTransformers.push(enumTransformer(host, typeChecker));
   }
   if (host.transformDecorators) {
     tsickleSourceTransformers.push(

test/googmodule_test.ts

@@ -13,6 +13,7 @@ import * as googmodule from '../src/googmodule';
 import {ModulesManifest} from '../src/modules_manifest';
 
 import * as testSupport from './test_support';
+import {outdent} from './test_support';
 
 interface ResolvedNamespace {
   name: string;
@@ -42,8 +43,14 @@ function processES5(
     transformDynamicImport: 'closure',
   };
   if (pathToNamespaceMap) {
-    host.jsPathToModuleName = (importPath: string) =>
-        pathToNamespaceMap.get(importPath)?.name;
+    host.jsPathToModuleName = (importPath: string) => {
+      const module = pathToNamespaceMap.get(importPath);
+      if (!module) return undefined;
+      return {
+        name: module.name,
+        multipleProvides: false,
+      };
+    };
     host.jsPathToStripProperty = (importPath: string) =>
         pathToNamespaceMap.get(importPath)?.stripProperty;
   }
@@ -71,21 +78,6 @@ function processES5(
   return {output, manifest, rootDir};
 }
 
-/**
- * Remove the first line (if empty) and unindents the all other lines by the
- * amount of leading whitespace in the second line.
- */
-function outdent(str: string) {
-  const lines = str.split('\n');
-  if (lines.length < 2) return str;
-  if (lines.shift() !== '') return str;
-  const indent = lines[0].match(/^ */)![0].length;
-  for (let i = 0; i < lines.length; i++) {
-    lines[i] = lines[i].substring(indent);
-  }
-  return lines.join('\n');
-}
-
 describe('convertCommonJsToGoogModule', () => {
   beforeEach(() => {
     testSupport.addDiffMatchers();

test/test_support.ts

@@ -453,3 +453,18 @@ export function pathToModuleName(
   if (fileName === tslibPath()) return 'tslib';
   return cliSupport.pathToModuleName(rootModulePath, context, fileName);
 }
+
+/**
+ * Remove the first line (if empty) and unindents the all other lines by the
+ * amount of leading whitespace in the second line.
+ */
+export function outdent(str: string) {
+  const lines = str.split('\n');
+  if (lines.length < 2) return str;
+  if (lines.shift() !== '') return str;
+  const indent = lines[0].match(/^ */)![0].length;
+  for (let i = 0; i < lines.length; i++) {
+    lines[i] = lines[i].substring(indent);
+  }
+  return lines.join('\n');
+}