Support multi provide modules with jsPathToModuleName implementation

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/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;

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');
+}

test/tsickle_test.ts

@@ -13,14 +13,14 @@ import {assertAbsolute} from '../src/cli_support';
 import * as tsickle from '../src/tsickle';
 
 import * as testSupport from './test_support';
+import {outdent} from './test_support';
 
 describe('emitWithTsickle', () => {
   function emitWithTsickle(
       tsSources: {[fileName: string]: string},
       tsConfigOverride: Partial<ts.CompilerOptions> = {},
       tsickleHostOverride: Partial<tsickle.TsickleHost> = {},
-      customTransformers?: tsickle.EmitTransformers):
-      {[fileName: string]: string} {
+      customTransformers?: tsickle.EmitTransformers) {
     const tsCompilerOptions: ts.CompilerOptions = {
       ...testSupport.compilerOptions,
       target: ts.ScriptTarget.ES5,
@@ -55,21 +55,21 @@ describe('emitWithTsickle', () => {
         return importPath.replace(/\/|\\/g, '.');
       },
       fileNameToModuleId: (fileName) => fileName.replace(/^\.\//, ''),
-      ...tsickleHostOverride,
       options: tsCompilerOptions,
       rootDirsRelative: testSupport.relativeToTsickleRoot,
-      transformDynamicImport: 'closure'
+      transformDynamicImport: 'closure',
+      ...tsickleHostOverride,
     };
     const jsSources: {[fileName: string]: string} = {};
-    tsickle.emit(
+    const {diagnostics} = tsickle.emit(
         program, tsickleHost,
         (fileName: string, data: string) => {
           jsSources[path.relative(tsCompilerOptions.rootDir!, fileName)] = data;
         },
         /* sourceFile */ undefined,
         /* cancellationToken */ undefined, /* emitOnlyDtsFiles */ undefined,
         customTransformers);
-    return jsSources;
+    return {jsSources, diagnostics};
   }
 
 
@@ -91,7 +91,7 @@ describe('emitWithTsickle', () => {
        const tsSources = {
          'a.ts': `export const x = 1;`,
        };
-       const jsSources = emitWithTsickle(
+       const {jsSources} = emitWithTsickle(
            tsSources, undefined, {
              shouldSkipTsickleProcessing: () => true,
            },
@@ -106,12 +106,10 @@ describe('emitWithTsickle', () => {
       'b.ts': `export * from './a';`,
     };
 
-    const jsSources = emitWithTsickle(
-        tsSources, {
-          preserveConstEnums: true,
-          module: ts.ModuleKind.ES2015,
-        },
-        {googmodule: false});
+    const {jsSources} = emitWithTsickle(tsSources, {
+      preserveConstEnums: true,
+      module: ts.ModuleKind.ES2015,
+    });
 
     expect(jsSources['b.js']).toContain(`export { Foo } from './a';`);
   });
@@ -121,16 +119,62 @@ describe('emitWithTsickle', () => {
       'a.ts': `export function f() : typeof f { return f; }`,
     };
 
-    const jsSources = emitWithTsickle(tsSources, {
+    const {jsSources} = emitWithTsickle(tsSources, {
       module: ts.ModuleKind.ES2015,
     });
 
-    expect(jsSources['a.js']).toContain(`
-/**
- * @return {function(): ?}
- */
-export function f() { return f; }
-`);
+    expect(jsSources['a.js']).toContain(outdent(`
+      /**
+       * @return {function(): ?}
+       */
+      export function f() { return f; }
+    `));
+  });
+
+  it('reports multi-provides error with jsPathToModuleName impl', () => {
+    const tsSources = {
+      'a.ts': `import {} from 'google3/multi/provide';`,
+      'clutz.d.ts': `declare module 'google3/multi/provide' { export {}; }`,
+    };
+    const {diagnostics} =
+        emitWithTsickle(
+            tsSources, /* tsConfigOverride= */ undefined,
+            /* tsickleHostOverride= */ {
+              jsPathToModuleName(importPath: string) {
+                if (importPath === 'google3/multi/provide') {
+                  return {
+                    name: 'multi.provide',
+                    multipleProvides: true,
+                  };
+                }
+                return undefined;
+              }
+            });
+    expect(testSupport.formatDiagnostics(diagnostics))
+        .toContain(
+            'referenced JavaScript module google3/multi/provide provides multiple namespaces and cannot be imported by path');
+  });
+
+  it('allows side-effect import of multi-provides module', () => {
+    const tsSources = {
+      'a.ts': `import 'google3/multi/provide';`,
+      'clutz.d.ts': `declare module 'google3/multi/provide' { export {}; }`,
+    };
+    const {jsSources} = emitWithTsickle(
+        tsSources, /* tsConfigOverride= */ undefined,
+        /* tsickleHostOverride= */ {
+          googmodule: true,
+          jsPathToModuleName(importPath: string) {
+            if (importPath === 'google3/multi/provide') {
+              return {
+                name: 'multi.provide',
+                multipleProvides: true,
+              };
+            }
+            return undefined;
+          },
+        });
+    expect(jsSources['a.js']).toContain(`goog.require('multi.provide');`);
   });
 
   describe('regressions', () => {
@@ -140,16 +184,15 @@ export function f() { return f; }
            'a.ts': `export const x = 1;`,
            'b.ts': `export * from './a';\n`,
          };
-         const jsSources = emitWithTsickle(
-             tsSources, {
-               declaration: true,
-               module: ts.ModuleKind.ES2015,
-             },
-             {googmodule: false});
-
-         expect(jsSources['b.d.ts'])
-             .toEqual(`//!! generated by tsickle from b.ts
-export * from './a';\n`);
+         const {jsSources} = emitWithTsickle(tsSources, {
+           declaration: true,
+           module: ts.ModuleKind.ES2015,
+         });
+
+         expect(jsSources['b.d.ts']).toEqual(outdent(`
+           //!! generated by tsickle from b.ts
+           export * from './a';
+         `));
        });
   });
 });