fix(ngcc): correctly match aliased classes between src and dts files

The naïve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared.  This did not handle cases where the declared
item has been exported via an alias. This is a common scenariow when one
of the two file sets (source or typings) has been flattened, while the other
has not.

The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).

This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.

Fixes angular#33593

packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts

@@ -58,33 +58,11 @@ export class PrivateDeclarationsAnalyzer {
               if (declaration.node.name.text === exportedName) {
                 // This declaration is public so we can remove it from the list
                 privateDeclarations.delete(declaration.node.name);
-              } else if (!this.host.getDtsDeclaration(declaration.node)) {
+              } else {
                 // The referenced declaration is exported publicly but via an alias.
                 // In some cases the original declaration is missing from the dts program, such as
                 // when rolling up (flattening) the dts files.
                 // This is because the original declaration gets renamed to the exported alias.
-
-                // There is a constraint on this which we cannot handle. Consider the following
-                // code:
-                //
-                // /src/entry_point.js:
-                //     export {MyComponent as aliasedMyComponent} from './a';
-                //     export {MyComponent} from './b';`
-                //
-                // /src/a.js:
-                //     export class MyComponent {}
-                //
-                // /src/b.js:
-                //     export class MyComponent {}
-                //
-                // //typings/entry_point.d.ts:
-                //     export declare class aliasedMyComponent {}
-                //     export declare class MyComponent {}
-                //
-                // In this case we would end up matching the `MyComponent` from `/src/a.js` to the
-                // `MyComponent` declared in `/typings/entry_point.d.ts` even though that
-                // declaration is actually for the `MyComponent` in `/src/b.js`.
-
                 exportAliasDeclarations.set(declaration.node.name, exportedName);
               }
             }

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

@@ -21,7 +21,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
   protected topLevelHelperCalls = new Map<string, Map<ts.SourceFile, ts.CallExpression[]>>();
   protected program: ts.Program;
   protected compilerHost: ts.CompilerHost;
-  constructor(logger: Logger, isCore: boolean, src: BundleProgram, dts?: BundleProgram|null) {
+  constructor(logger: Logger, isCore: boolean, src: BundleProgram, dts: BundleProgram|null = null) {
     super(logger, isCore, src, dts);
     this.program = src.program;
     this.compilerHost = src.host;

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

@@ -50,7 +50,7 @@ export const CONSTRUCTOR_PARAMS = 'ctorParameters' as ts.__String;
  *   a static method called `ctorParameters`.
  */
 export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements NgccReflectionHost {
-  protected dtsDeclarationMap: Map<string, ts.Declaration>|null;
+  protected dtsDeclarationMap: Map<ts.Declaration, ts.Declaration>|undefined = undefined;
 
   /**
    * The set of source files that have already been preprocessed.
@@ -83,11 +83,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
   protected decoratorCache = new Map<ClassDeclaration, DecoratorInfo>();
 
   constructor(
-      protected logger: Logger, protected isCore: boolean, src: BundleProgram,
-      dts?: BundleProgram|null) {
+      protected logger: Logger, protected isCore: boolean, protected src: BundleProgram,
+      protected dts: BundleProgram|null = null) {
     super(src.program.getTypeChecker());
-    this.dtsDeclarationMap =
-        dts && this.computeDtsDeclarationMap(dts.path, dts.program, dts.package) || null;
   }
 
   /**
@@ -484,16 +482,18 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
    * `ts.Program` as the input declaration.
    */
   getDtsDeclaration(declaration: ts.Declaration): ts.Declaration|null {
-    if (!this.dtsDeclarationMap) {
+    if (!this.dts) {
       return null;
     }
+    if (!this.dtsDeclarationMap) {
+      this.dtsDeclarationMap = this.computeDtsDeclarationMap(this.src, this.dts);
+    }
     if (!isNamedDeclaration(declaration)) {
       throw new Error(
           `Cannot get the dts file for a declaration that has no name: ${declaration.getText()} in ${declaration.getSourceFile().fileName}`);
     }
-    return this.dtsDeclarationMap.has(declaration.name.text) ?
-        this.dtsDeclarationMap.get(declaration.name.text) ! :
-        null;
+    return this.dtsDeclarationMap.has(declaration) ? this.dtsDeclarationMap.get(declaration) ! :
+                                                     null;
   }
 
   /**
@@ -1499,42 +1499,25 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
   }
 
   /**
-   * Extract all the class declarations from the dtsTypings program, storing them in a map
-   * where the key is the declared name of the class and the value is the declaration itself.
-   *
-   * It is possible for there to be multiple class declarations with the same local name.
-   * Only the first declaration with a given name is added to the map; subsequent classes will be
-   * ignored.
-   *
-   * We are most interested in classes that are publicly exported from the entry point, so these
-   * are added to the map first, to ensure that they are not ignored.
+   * Compute a mapping from the source declarations to the typings declarations.
    *
-   * @param dtsRootFileName The filename of the entry-point to the `dtsTypings` program.
-   * @param dtsProgram The program containing all the typings files.
-   * @returns a map of class names to class declarations.
+   * @param src the program bundle containing the source files.
+   * @param dts The program bundle containing the typings files.
+   * @returns a map of source declarations to typings declarations.
    */
-  protected computeDtsDeclarationMap(
-      dtsRootFileName: AbsoluteFsPath, dtsProgram: ts.Program,
-      dtsPackage: AbsoluteFsPath): Map<string, ts.Declaration> {
-    const dtsDeclarationMap = new Map<string, ts.Declaration>();
-    const checker = dtsProgram.getTypeChecker();
-
-    // First add all the classes that are publicly exported from the entry-point
-    const rootFile = dtsProgram.getSourceFile(dtsRootFileName);
-    if (!rootFile) {
-      throw new Error(`The given file ${dtsRootFileName} is not part of the typings program.`);
-    }
-    collectExportedDeclarations(checker, dtsDeclarationMap, rootFile);
-
-    // Now add any additional classes that are exported from individual  dts files,
-    // but are not publicly exported from the entry-point.
-    dtsProgram.getSourceFiles().forEach(sourceFile => {
-      if (!isWithinPackage(dtsPackage, sourceFile)) {
-        return;
+  protected computeDtsDeclarationMap(src: BundleProgram, dts: BundleProgram):
+      Map<ts.Declaration, ts.Declaration> {
+    const dtsMap = computeDtsExportDeclarationMap(dts);
+    const declarationMap = new Map<ts.Declaration, ts.Declaration>();
+    forEachFileInProgram(src, file => {
+      const fileExports = this.getExportsOfModule(file !);
+      for (const [exportName, {node: declaration}] of fileExports !) {
+        if (declaration !== null && dtsMap.has(exportName)) {
+          declarationMap.set(declaration, dtsMap.get(exportName) !);
+        }
       }
-      collectExportedDeclarations(checker, dtsDeclarationMap, sourceFile);
     });
-    return dtsDeclarationMap;
+    return declarationMap;
   }
 
   /**
@@ -1863,21 +1846,45 @@ function isClassMemberType(node: ts.Declaration): node is ts.ClassElement|
 }
 
 /**
- * Collect mappings between exported declarations in a source file and its associated
- * declaration in the typings program.
+ * Create a mapping between the names of exports in a dts program and the declaration of those
+ * exports.
+ *
+ * It is possible for there to be multiple declarations with the same exported name.
+ * Only the first declaration exported with a given name is added to the map; subsequent classes
+ * are ignored.
+ *
+ * We are most interested in classes that are publicly exported from the root file, so these
+ * are added to the map first, to ensure that they are not ignored.
+ *
+ * @param dtsRoot The path to the root file for the dts program.
+ * @param dtsProgram The program containing all the dts declarations.
+ * @returns a map of exported names to declarations.
+ */
+function computeDtsExportDeclarationMap(dts: BundleProgram): Map<string, ts.Declaration> {
+  const declarationMap = new Map<string, ts.Declaration>();
+  const checker = dts.program.getTypeChecker();
+  forEachFileInProgram(
+      dts, sourceFile => collectDtsExportedDeclarations(checker, declarationMap, sourceFile));
+  return declarationMap;
+}
+
+/**
+ * Collect mappings between names of exported declarations in a file and its actual declaration.
+ *
+ * Any new mappings are added to the `dtsDeclarationMap`.
  */
-function collectExportedDeclarations(
+function collectDtsExportedDeclarations(
     checker: ts.TypeChecker, dtsDeclarationMap: Map<string, ts.Declaration>,
     srcFile: ts.SourceFile): void {
   const srcModule = srcFile && checker.getSymbolAtLocation(srcFile);
   const moduleExports = srcModule && checker.getExportsOfModule(srcModule);
   if (moduleExports) {
     moduleExports.forEach(exportedSymbol => {
+      const name = exportedSymbol.name;
       if (exportedSymbol.flags & ts.SymbolFlags.Alias) {
         exportedSymbol = checker.getAliasedSymbol(exportedSymbol);
       }
       const declaration = exportedSymbol.valueDeclaration;
-      const name = exportedSymbol.name;
       if (declaration && !dtsDeclarationMap.has(name)) {
         dtsDeclarationMap.set(name, declaration);
       }
@@ -1972,3 +1979,24 @@ function getContainingStatement(node: ts.Node): ts.ExpressionStatement|null {
   }
   return node || null;
 }
+
+/**
+ * Execute the `callBack` for each file in the bundle program, starting with the root file.
+ *
+ * @param bundle the bundle program whose files should be iterated over
+ * @param callBack the function to all for each of the files in the program
+ */
+function forEachFileInProgram(bundle: BundleProgram, callBack: (file: ts.SourceFile) => void) {
+  const rootFile = bundle.program.getSourceFile(bundle.path);
+  if (!rootFile) {
+    throw new Error(`The given rootPath ${rootFile} is not a file of the given program.`);
+  }
+  // First run the call back on the root file.
+  callBack(rootFile);
+  bundle.program.getSourceFiles().forEach(file => {
+    // Ignore if the file is the root (already processed) or if the file is not part of the package.
+    if (file !== rootFile && isWithinPackage(bundle.package, file)) {
+      callBack(file);
+    }
+  });
+}

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

@@ -20,7 +20,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
   protected umdImportPaths = new Map<ts.ParameterDeclaration, string|null>();
   protected program: ts.Program;
   protected compilerHost: ts.CompilerHost;
-  constructor(logger: Logger, isCore: boolean, src: BundleProgram, dts?: BundleProgram|null) {
+  constructor(logger: Logger, isCore: boolean, src: BundleProgram, dts: BundleProgram|null = null) {
     super(logger, isCore, src, dts);
     this.program = src.program;
     this.compilerHost = src.host;

packages/compiler-cli/ngcc/test/analysis/private_declarations_analyzer_spec.ts

@@ -225,7 +225,7 @@ runInEachFileSystem(() => {
         expect(analyses).toEqual([{
           identifier: 'ComponentOne',
           from: _('/node_modules/test-package/src/a.js'),
-          dtsFrom: null,
+          dtsFrom: _('/node_modules/test-package/typings/entry_point.d.ts'),
           alias: 'aliasedComponentOne',
         }]);
       });