refactor(ngcc): share code between `CommonJsReflectionHost` and `UmdReflectionHost` (#34512)

While different, CommonJS and UMD have a lot in common regarding the
their exports are constructed. Therefore, there was some code
duplication between `CommonJsReflectionHost` and `UmdReflectionHost`.

This commit extracts some of the common bits into a separate file as
helpers to allow reusing the code in both `ReflectionHost`s.

PR Close #34512

packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts

@@ -7,7 +7,7 @@
  */
 import * as ts from 'typescript';
 import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
-import {isRequireCall} from '../host/commonjs_host';
+import {isRequireCall} from '../host/commonjs_umd_utils';
 import {DependencyHostBase} from './dependency_host';
 import {ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver';
 

packages/compiler-cli/ngcc/src/host/commonjs_host.ts

@@ -11,8 +11,9 @@ import {absoluteFrom} from '../../../src/ngtsc/file_system';
 import {Declaration, Import} from '../../../src/ngtsc/reflection';
 import {Logger} from '../logging/logger';
 import {BundleProgram} from '../packages/bundle_program';
-import {isDefined, stripExtension} from '../utils';
+import {getOrDefault, isDefined, stripExtension} from '../utils';
 
+import {ExportDeclaration, ExportStatement, ReexportStatement, RequireCall, findNamespaceOfIdentifier, findRequireCallReference, isExportStatement, isReexportStatement, isRequireCall} from './commonjs_umd_utils';
 import {Esm5ReflectionHost} from './esm5_host';
 import {NgccClassSymbol} from './ngcc_host';
 
@@ -103,7 +104,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
   private computeExportsOfCommonJsModule(sourceFile: ts.SourceFile): Map<string, Declaration> {
     const moduleMap = new Map<string, Declaration>();
     for (const statement of this.getModuleStatements(sourceFile)) {
-      if (isCommonJsExportStatement(statement)) {
+      if (isExportStatement(statement)) {
         const exportDeclaration = this.extractCommonJsExportDeclaration(statement);
         moduleMap.set(exportDeclaration.name, exportDeclaration.declaration);
       } else if (isReexportStatement(statement)) {
@@ -116,8 +117,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
     return moduleMap;
   }
 
-  private extractCommonJsExportDeclaration(statement: CommonJsExportStatement):
-      CommonJsExportDeclaration {
+  private extractCommonJsExportDeclaration(statement: ExportStatement): ExportDeclaration {
     const exportExpression = statement.expression.right;
     const declaration = this.getDeclarationOfExpression(exportExpression);
     const name = statement.expression.left.name.text;
@@ -136,12 +136,12 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
   }
 
   private extractCommonJsReexports(statement: ReexportStatement, containingFile: ts.SourceFile):
-      CommonJsExportDeclaration[] {
+      ExportDeclaration[] {
     const reexportArg = statement.expression.arguments[0];
 
     const requireCall = isRequireCall(reexportArg) ?
         reexportArg :
-        ts.isIdentifier(reexportArg) ? this.findRequireCallReference(reexportArg) : null;
+        ts.isIdentifier(reexportArg) ? findRequireCallReference(reexportArg, this.checker) : null;
     if (requireCall === null) {
       return [];
     }
@@ -158,7 +158,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
     }
 
     const viaModule = stripExtension(importedFile.fileName);
-    const reexports: CommonJsExportDeclaration[] = [];
+    const reexports: ExportDeclaration[] = [];
     importedExports.forEach((decl, name) => {
       if (decl.node !== null) {
         reexports.push({name, declaration: {node: decl.node, viaModule}});
@@ -173,15 +173,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
     // Is `id` a namespaced property access, e.g. `Directive` in `core.Directive`?
     // If so capture the symbol of the namespace, e.g. `core`.
     const nsIdentifier = findNamespaceOfIdentifier(id);
-    return nsIdentifier && this.findRequireCallReference(nsIdentifier);
-  }
-
-  private findRequireCallReference(id: ts.Identifier): RequireCall|null {
-    const symbol = id && this.checker.getSymbolAtLocation(id) || null;
-    const declaration = symbol && symbol.valueDeclaration;
-    const initializer =
-        declaration && ts.isVariableDeclaration(declaration) && declaration.initializer || null;
-    return initializer && isRequireCall(initializer) ? initializer : null;
+    return nsIdentifier && findRequireCallReference(nsIdentifier, this.checker);
   }
 
   private getCommonJsImportedDeclaration(id: ts.Identifier): Declaration|null {
@@ -215,53 +207,3 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
     }
   }
 }
-
-type CommonJsExportStatement = ts.ExpressionStatement & {
-  expression:
-      ts.BinaryExpression & {left: ts.PropertyAccessExpression & {expression: ts.Identifier}}
-};
-export function isCommonJsExportStatement(s: ts.Statement): s is CommonJsExportStatement {
-  return ts.isExpressionStatement(s) && ts.isBinaryExpression(s.expression) &&
-      ts.isPropertyAccessExpression(s.expression.left) &&
-      ts.isIdentifier(s.expression.left.expression) &&
-      s.expression.left.expression.text === 'exports';
-}
-
-interface CommonJsExportDeclaration {
-  name: string;
-  declaration: Declaration;
-}
-
-export type RequireCall = ts.CallExpression & {arguments: [ts.StringLiteral]};
-export function isRequireCall(node: ts.Node): node is RequireCall {
-  return ts.isCallExpression(node) && ts.isIdentifier(node.expression) &&
-      node.expression.text === 'require' && node.arguments.length === 1 &&
-      ts.isStringLiteral(node.arguments[0]);
-}
-
-/**
- * If the identifier `id` is the RHS of a property access of the form `namespace.id`
- * and `namespace` is an identifer then return `namespace`, otherwise `null`.
- * @param id The identifier whose namespace we want to find.
- */
-function findNamespaceOfIdentifier(id: ts.Identifier): ts.Identifier|null {
-  return id.parent && ts.isPropertyAccessExpression(id.parent) &&
-          ts.isIdentifier(id.parent.expression) ?
-      id.parent.expression :
-      null;
-}
-
-type ReexportStatement = ts.ExpressionStatement & {expression: ts.CallExpression};
-function isReexportStatement(statement: ts.Statement): statement is ReexportStatement {
-  return ts.isExpressionStatement(statement) && ts.isCallExpression(statement.expression) &&
-      ts.isIdentifier(statement.expression.expression) &&
-      statement.expression.expression.text === '__export' &&
-      statement.expression.arguments.length === 1;
-}
-
-function getOrDefault<K, V>(map: Map<K, V>, key: K, factory: (key: K) => V): V {
-  if (!map.has(key)) {
-    map.set(key, factory(key));
-  }
-  return map.get(key) !;
-}

packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts

@@ -0,0 +1,85 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import * as ts from 'typescript';
+import {Declaration} from '../../../src/ngtsc/reflection';
+
+
+export interface ExportDeclaration {
+  name: string;
+  declaration: Declaration;
+}
+
+export interface ExportStatement extends ts.ExpressionStatement {
+  expression: ts.BinaryExpression&{left: ts.PropertyAccessExpression & {expression: ts.Identifier}};
+}
+
+export interface ReexportStatement extends ts.ExpressionStatement { expression: ts.CallExpression; }
+
+export interface RequireCall extends ts.CallExpression {
+  arguments: ts.CallExpression['arguments']&[ts.StringLiteral];
+}
+
+
+/**
+ * Return the "namespace" of the specified `ts.Identifier` if the identifier is the RHS of a
+ * property access expression, i.e. an expression of the form `<namespace>.<id>` (in which case a
+ * `ts.Identifier` corresponding to `<namespace>` will be returned). Otherwise return `null`.
+ */
+export function findNamespaceOfIdentifier(id: ts.Identifier): ts.Identifier|null {
+  return id.parent && ts.isPropertyAccessExpression(id.parent) &&
+          ts.isIdentifier(id.parent.expression) ?
+      id.parent.expression :
+      null;
+}
+
+/**
+ * Return the `RequireCall` that is used to initialize the specified `ts.Identifier`, if the
+ * specified indentifier was indeed initialized with a require call in a declaration of the form:
+ * `var <id> = require('...')`
+ */
+export function findRequireCallReference(id: ts.Identifier, checker: ts.TypeChecker): RequireCall|
+    null {
+  const symbol = checker.getSymbolAtLocation(id) || null;
+  const declaration = symbol && symbol.valueDeclaration;
+  const initializer =
+      declaration && ts.isVariableDeclaration(declaration) && declaration.initializer || null;
+  return initializer && isRequireCall(initializer) ? initializer : null;
+}
+
+/**
+ * Check whether the specified `ts.Statement` is an export statement, i.e. an expression statement
+ * of the form: `export.<foo> = <bar>`
+ */
+export function isExportStatement(stmt: ts.Statement): stmt is ExportStatement {
+  return ts.isExpressionStatement(stmt) && ts.isBinaryExpression(stmt.expression) &&
+      (stmt.expression.operatorToken.kind === ts.SyntaxKind.EqualsToken) &&
+      ts.isPropertyAccessExpression(stmt.expression.left) &&
+      ts.isIdentifier(stmt.expression.left.expression) &&
+      stmt.expression.left.expression.text === 'exports';
+}
+
+/**
+ * Check whether the specified `ts.Statement` is a re-export statement, i.e. an expression statement
+ * of the form: `__export(<foo>)`
+ */
+export function isReexportStatement(stmt: ts.Statement): stmt is ReexportStatement {
+  return ts.isExpressionStatement(stmt) && ts.isCallExpression(stmt.expression) &&
+      ts.isIdentifier(stmt.expression.expression) &&
+      stmt.expression.expression.text === '__export' && stmt.expression.arguments.length === 1;
+}
+
+/**
+ * Check whether the specified `ts.Node` represents a `require()` call, i.e. an call expression of
+ * the form: `require('<foo>')`
+ */
+export function isRequireCall(node: ts.Node): node is RequireCall {
+  return ts.isCallExpression(node) && ts.isIdentifier(node.expression) &&
+      node.expression.text === 'require' && node.arguments.length === 1 &&
+      ts.isStringLiteral(node.arguments[0]);
+}

packages/compiler-cli/ngcc/src/host/umd_host.ts

@@ -12,7 +12,8 @@ import {absoluteFrom} from '../../../src/ngtsc/file_system';
 import {Declaration, Import} from '../../../src/ngtsc/reflection';
 import {Logger} from '../logging/logger';
 import {BundleProgram} from '../packages/bundle_program';
-import {stripExtension} from '../utils';
+import {getOrDefault, stripExtension} from '../utils';
+import {ExportDeclaration, ExportStatement, ReexportStatement, findNamespaceOfIdentifier, findRequireCallReference, isExportStatement, isReexportStatement, isRequireCall} from './commonjs_umd_utils';
 import {Esm5ReflectionHost, stripParentheses} from './esm5_host';
 
 export class UmdReflectionHost extends Esm5ReflectionHost {
@@ -54,47 +55,45 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
     if (sourceFile.isDeclarationFile) {
       return null;
     }
-    if (!this.umdModules.has(sourceFile)) {
-      if (sourceFile.statements.length !== 1) {
+
+    return getOrDefault(this.umdModules, sourceFile, (sf: ts.SourceFile) => {
+      if (sf.statements.length !== 1) {
         throw new Error(
-            `Expected UMD module file (${sourceFile.fileName}) to contain exactly one statement, but found ${sourceFile.statements}.`);
+            `Expected UMD module file (${sf.fileName}) to contain exactly one statement, but found ${sf.statements.length}.`);
       }
-      this.umdModules.set(sourceFile, parseStatementForUmdModule(sourceFile.statements[0]));
-    }
-    return this.umdModules.get(sourceFile) !;
+      return parseStatementForUmdModule(sf.statements[0]);
+    });
   }
 
   getUmdImportPath(importParameter: ts.ParameterDeclaration): string|null {
-    if (this.umdImportPaths.has(importParameter)) {
-      return this.umdImportPaths.get(importParameter) !;
-    }
-
-    const umdModule = this.getUmdModule(importParameter.getSourceFile());
-    if (umdModule === null) {
-      return null;
-    }
-
-    const imports = getImportsOfUmdModule(umdModule);
-    if (imports === null) {
-      return null;
-    }
-
-    for (const i of imports) {
-      this.umdImportPaths.set(i.parameter, i.path);
-      if (i.parameter === importParameter) {
-        return i.path;
+    return getOrDefault(this.umdImportPaths, importParameter, (param: ts.ParameterDeclaration) => {
+      const umdModule = this.getUmdModule(param.getSourceFile());
+      if (umdModule === null) {
+        return null;
       }
-    }
 
-    return null;
+      const imports = getImportsOfUmdModule(umdModule);
+      if (imports === null) {
+        return null;
+      }
+
+      let importPath: string|null = null;
+
+      for (const i of imports) {
+        // Add all imports to the map to speed up future look ups.
+        this.umdImportPaths.set(i.parameter, i.path);
+        if (i.parameter === importParameter) {
+          importPath = i.path;
+        }
+      }
+
+      return importPath;
+    });
   }
 
   getUmdExports(sourceFile: ts.SourceFile): Map<string, Declaration>|null {
-    if (!this.umdExports.has(sourceFile)) {
-      const moduleExports = this.computeExportsOfUmdModule(sourceFile);
-      this.umdExports.set(sourceFile, moduleExports);
-    }
-    return this.umdExports.get(sourceFile) !;
+    return getOrDefault(
+        this.umdExports, sourceFile, (sf: ts.SourceFile) => this.computeExportsOfUmdModule(sf));
   }
 
   /** Get the top level statements for a module.
@@ -112,9 +111,9 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
   private computeExportsOfUmdModule(sourceFile: ts.SourceFile): Map<string, Declaration>|null {
     const moduleMap = new Map<string, Declaration>();
     for (const statement of this.getModuleStatements(sourceFile)) {
-      if (isUmdExportStatement(statement)) {
-        const declaration = this.extractUmdExportDeclaration(statement);
-        moduleMap.set(declaration.name, declaration.declaration);
+      if (isExportStatement(statement)) {
+        const exportDeclaration = this.extractUmdExportDeclaration(statement);
+        moduleMap.set(exportDeclaration.name, exportDeclaration.declaration);
       } else if (isReexportStatement(statement)) {
         const reexports = this.extractUmdReexports(statement, sourceFile);
         for (const reexport of reexports) {
@@ -125,7 +124,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
     return moduleMap;
   }
 
-  private extractUmdExportDeclaration(statement: UmdExportStatement): UmdExportDeclaration {
+  private extractUmdExportDeclaration(statement: ExportStatement): ExportDeclaration {
     const exportExpression = statement.expression.right;
     const declaration = this.getDeclarationOfExpression(exportExpression);
     const name = statement.expression.left.name.text;
@@ -144,12 +143,12 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
   }
 
   private extractUmdReexports(statement: ReexportStatement, containingFile: ts.SourceFile):
-      UmdExportDeclaration[] {
+      ExportDeclaration[] {
     const reexportArg = statement.expression.arguments[0];
 
     const requireCall = isRequireCall(reexportArg) ?
         reexportArg :
-        ts.isIdentifier(reexportArg) ? this.findRequireCallReference(reexportArg) : null;
+        ts.isIdentifier(reexportArg) ? findRequireCallReference(reexportArg, this.checker) : null;
 
     let importPath: string|null = null;
 
@@ -175,7 +174,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
     }
 
     const viaModule = stripExtension(importedFile.fileName);
-    const reexports: UmdExportDeclaration[] = [];
+    const reexports: ExportDeclaration[] = [];
     importedExports.forEach((decl, name) => {
       if (decl.node !== null) {
         reexports.push({name, declaration: {node: decl.node, viaModule}});
@@ -196,14 +195,6 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
     return declaration && ts.isParameter(declaration) ? declaration : null;
   }
 
-  private findRequireCallReference(id: ts.Identifier): RequireCall|null {
-    const symbol = id && this.checker.getSymbolAtLocation(id) || null;
-    const declaration = symbol && symbol.valueDeclaration;
-    const initializer =
-        declaration && ts.isVariableDeclaration(declaration) && declaration.initializer || null;
-    return initializer && isRequireCall(initializer) ? initializer : null;
-  }
-
   private getUmdImportedDeclaration(id: ts.Identifier): Declaration|null {
     const importInfo = this.getImportOfIdentifier(id);
     if (importInfo === null) {
@@ -282,31 +273,6 @@ interface UmdModule {
   factoryFn: ts.FunctionExpression;
 }
 
-type UmdExportStatement = ts.ExpressionStatement & {
-  expression:
-      ts.BinaryExpression & {left: ts.PropertyAccessExpression & {expression: ts.Identifier}}
-};
-
-function isUmdExportStatement(s: ts.Statement): s is UmdExportStatement {
-  return ts.isExpressionStatement(s) && ts.isBinaryExpression(s.expression) &&
-      ts.isPropertyAccessExpression(s.expression.left) &&
-      ts.isIdentifier(s.expression.left.expression) &&
-      s.expression.left.expression.text === 'exports';
-}
-
-interface UmdExportDeclaration {
-  name: string;
-  declaration: Declaration;
-}
-
-type ReexportStatement = ts.ExpressionStatement & {expression: ts.CallExpression};
-function isReexportStatement(statement: ts.Statement): statement is ReexportStatement {
-  return ts.isExpressionStatement(statement) && ts.isCallExpression(statement.expression) &&
-      ts.isIdentifier(statement.expression.expression) &&
-      statement.expression.expression.text === '__export' &&
-      statement.expression.arguments.length === 1;
-}
-
 function getRequiredModulePath(wrapperFn: ts.FunctionExpression, paramIndex: number): string {
   const statement = wrapperFn.body.statements[0];
   if (!ts.isExpressionStatement(statement)) {
@@ -335,21 +301,3 @@ function getRequiredModulePath(wrapperFn: ts.FunctionExpression, paramIndex: num
     }
   }
 }
-
-type RequireCall = ts.CallExpression & {arguments: [ts.StringLiteral]};
-function isRequireCall(node: ts.Node): node is RequireCall {
-  return ts.isCallExpression(node) && ts.isIdentifier(node.expression) &&
-      node.expression.text === 'require' && node.arguments.length === 1;
-}
-
-/**
- * If the identifier `id` is the RHS of a property access of the form `namespace.id`
- * and `namespace` is an identifer then return `namespace`, otherwise `null`.
- * @param id The identifier whose namespace we want to find.
- */
-function findNamespaceOfIdentifier(id: ts.Identifier): ts.Identifier|null {
-  return id.parent && ts.isPropertyAccessExpression(id.parent) &&
-          ts.isIdentifier(id.parent.expression) ?
-      id.parent.expression :
-      null;
-}

packages/compiler-cli/ngcc/src/rendering/commonjs_rendering_formatter.ts

@@ -11,7 +11,7 @@ import MagicString from 'magic-string';
 import {Reexport} from '../../../src/ngtsc/imports';
 import {Import, ImportManager} from '../../../src/ngtsc/translator';
 import {ExportInfo} from '../analysis/private_declarations_analyzer';
-import {isRequireCall} from '../host/commonjs_host';
+import {isRequireCall} from '../host/commonjs_umd_utils';
 import {NgccReflectionHost} from '../host/ngcc_host';
 import {Esm5RenderingFormatter} from './esm5_rendering_formatter';
 import {stripExtension} from './utils';

packages/compiler-cli/ngcc/src/utils.ts

@@ -66,6 +66,13 @@ export function findAll<T>(node: ts.Node, test: (node: ts.Node) => node is ts.No
   }
 }
 
+export function getOrDefault<K, V>(map: Map<K, V>, key: K, factory: (key: K) => V): V {
+  if (!map.has(key)) {
+    map.set(key, factory(key));
+  }
+  return map.get(key) !;
+}
+
 /**
  * Does the given declaration have a name which is an identifier?
  * @param declaration The declaration to test.

packages/compiler-cli/ngcc/test/utils_spec.ts

@@ -6,7 +6,31 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {isRelativePath} from '../src/utils';
+import {getOrDefault, isRelativePath, stripExtension} from '../src/utils';
+
+describe('getOrDefault()', () => {
+  it('should return an existing value', () => {
+    const map = new Map([['k1', 'v1'], ['k2', 'v2']]);
+    const factorySpy = jasmine.createSpy('factory');
+
+    expect(getOrDefault(map, 'k1', factorySpy)).toBe('v1');
+    expect(getOrDefault(map, 'k2', factorySpy)).toBe('v2');
+    expect(factorySpy).not.toHaveBeenCalled();
+  });
+
+  it('should create, store and return the value if it does not exist', () => {
+    const map = new Map([['k1', 'v1'], ['k2', 'v2']]);
+    const factorySpy = jasmine.createSpy('factory').and.returnValues('v3', 'never gonna happen');
+
+    expect(getOrDefault(map, 'k3', factorySpy)).toBe('v3');
+    expect(factorySpy).toHaveBeenCalledTimes(1);
+
+    factorySpy.calls.reset();
+
+    expect(getOrDefault(map, 'k3', factorySpy)).toBe('v3');
+    expect(factorySpy).not.toHaveBeenCalled();
+  });
+});
 
 describe('isRelativePath()', () => {
   it('should return true for relative paths', () => {
@@ -34,3 +58,17 @@ describe('isRelativePath()', () => {
     expect(isRelativePath('@abc/xyz')).toBe(false);
   });
 });
+
+describe('stripExtension()', () => {
+  it('should strip the extension from a file name', () => {
+    expect(stripExtension('foo.ts')).toBe('foo');
+    expect(stripExtension('/foo/bar.ts')).toBe('/foo/bar');
+    expect(stripExtension('/foo/bar.d.ts')).toBe('/foo/bar');
+  });
+
+  it('should do nothing if there is no extension in a file name', () => {
+    expect(stripExtension('foo')).toBe('foo');
+    expect(stripExtension('/foo/bar')).toBe('/foo/bar');
+    expect(stripExtension('/fo-o/b_ar')).toBe('/fo-o/b_ar');
+  });
+});