fix(CompilerCli): assert that all pipes and directives are declared by a module

modules/@angular/compiler-cli/src/codegen.ts

@@ -11,7 +11,7 @@
  * Intended to be used in a build step.
  */
 import * as compiler from '@angular/compiler';
-import {Directive, NgModule, ViewEncapsulation} from '@angular/core';
+import {ViewEncapsulation} from '@angular/core';
 import {AngularCompilerOptions, NgcCliOptions} from '@angular/tsc-wrapped';
 import * as path from 'path';
 import * as ts from 'typescript';
@@ -35,65 +35,11 @@ const PREAMBLE = `/**
 
 `;
 
-export class CodeGeneratorModuleCollector {
-  constructor(
-      private staticReflector: StaticReflector, private reflectorHost: StaticReflectorHost,
-      private program: ts.Program, private options: AngularCompilerOptions) {}
-
-  getModuleSymbols(): StaticSymbol[] {
-    // Compare with false since the default should be true
-    const skipFileNames =
-        this.options.generateCodeForLibraries === false ? GENERATED_OR_DTS_FILES : GENERATED_FILES;
-
-    const ngModules: StaticSymbol[] = [];
-
-    this.program.getSourceFiles()
-        .filter(sourceFile => !skipFileNames.test(sourceFile.fileName))
-        .forEach(sourceFile => {
-          const absSrcPath = this.reflectorHost.getCanonicalFileName(sourceFile.fileName);
-
-          const moduleMetadata = this.staticReflector.getModuleMetadata(absSrcPath);
-          if (!moduleMetadata) {
-            console.log(`WARNING: no metadata found for ${absSrcPath}`);
-            return;
-          }
-
-          const metadata = moduleMetadata['metadata'];
-
-          if (!metadata) {
-            return;
-          }
-
-          for (const symbol of Object.keys(metadata)) {
-            if (metadata[symbol] && metadata[symbol].__symbolic == 'error') {
-              // Ignore symbols that are only included to record error information.
-              continue;
-            }
-            const staticType = this.reflectorHost.findDeclaration(absSrcPath, symbol, absSrcPath);
-            const annotations = this.staticReflector.annotations(staticType);
-            annotations.some((annotation) => {
-              if (annotation instanceof NgModule) {
-                ngModules.push(staticType);
-                return true;
-              }
-            });
-          }
-        });
-
-    return ngModules;
-  }
-}
-
 export class CodeGenerator {
-  private moduleCollector: CodeGeneratorModuleCollector;
-
   constructor(
       private options: AngularCompilerOptions, private program: ts.Program,
       public host: ts.CompilerHost, private staticReflector: StaticReflector,
-      private compiler: compiler.OfflineCompiler, private reflectorHost: StaticReflectorHost) {
+      private compiler: compiler.OfflineCompiler, private reflectorHost: StaticReflectorHost) {}
-    this.moduleCollector =
-        new CodeGeneratorModuleCollector(staticReflector, reflectorHost, program, options);
-  }
 
   // Write codegen in a directory structure matching the sources.
   private calculateEmitPath(filePath: string): string {
@@ -118,10 +64,11 @@ export class CodeGenerator {
     return path.join(this.options.genDir, relativePath);
   }
 
-  codegen(): Promise<any> {
+  codegen(options: {transitiveModules: boolean}): Promise<any> {
-    const ngModules = this.moduleCollector.getModuleSymbols();
+    const staticSymbols =
+        extractProgramSymbols(this.program, this.staticReflector, this.reflectorHost, this.options);
 
-    return this.compiler.compileModules(ngModules).then(generatedModules => {
+    return this.compiler.compileModules(staticSymbols, options).then(generatedModules => {
       generatedModules.forEach(generatedModule => {
         const sourceFile = this.program.getSourceFile(generatedModule.fileUrl);
         const emitPath = this.calculateEmitPath(generatedModule.moduleUrl);
@@ -194,3 +141,41 @@ export class CodeGenerator {
         options, program, compilerHost, staticReflector, offlineCompiler, reflectorHost);
   }
 }
+
+export function extractProgramSymbols(
+    program: ts.Program, staticReflector: StaticReflector, reflectorHost: StaticReflectorHost,
+    options: AngularCompilerOptions): StaticSymbol[] {
+  // Compare with false since the default should be true
+  const skipFileNames =
+      options.generateCodeForLibraries === false ? GENERATED_OR_DTS_FILES : GENERATED_FILES;
+
+  const staticSymbols: StaticSymbol[] = [];
+
+  program.getSourceFiles()
+      .filter(sourceFile => !skipFileNames.test(sourceFile.fileName))
+      .forEach(sourceFile => {
+        const absSrcPath = reflectorHost.getCanonicalFileName(sourceFile.fileName);
+
+        const moduleMetadata = staticReflector.getModuleMetadata(absSrcPath);
+        if (!moduleMetadata) {
+          console.log(`WARNING: no metadata found for ${absSrcPath}`);
+          return;
+        }
+
+        const metadata = moduleMetadata['metadata'];
+
+        if (!metadata) {
+          return;
+        }
+
+        for (const symbol of Object.keys(metadata)) {
+          if (metadata[symbol] && metadata[symbol].__symbolic == 'error') {
+            // Ignore symbols that are only included to record error information.
+            continue;
+          }
+          staticSymbols.push(reflectorHost.findDeclaration(absSrcPath, symbol, absSrcPath));
+        }
+      });
+
+  return staticSymbols;
+}

modules/@angular/compiler-cli/src/extractor.ts

@@ -9,23 +9,20 @@
 
 /**
  * Extract i18n messages from source code
- *
- * TODO(vicb): factorize code with the CodeGenerator
  */
 // Must be imported first, because angular2 decorators throws on load.
 import 'reflect-metadata';
 
 import * as compiler from '@angular/compiler';
-import {Component, NgModule, ViewEncapsulation} from '@angular/core';
+import {ViewEncapsulation} from '@angular/core';
 import * as tsc from '@angular/tsc-wrapped';
 import * as ts from 'typescript';
 
-import {ReflectorHost, ReflectorHostContext} from './reflector_host';
+import {extractProgramSymbols} from './codegen';
+import {ReflectorHost} from './reflector_host';
 import {StaticAndDynamicReflectionCapabilities} from './static_reflection_capabilities';
 import {StaticReflector, StaticSymbol} from './static_reflector';
 
-const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
-
 export class Extractor {
   constructor(
       private options: tsc.AngularCompilerOptions, private program: ts.Program,
@@ -34,48 +31,13 @@ export class Extractor {
       private metadataResolver: compiler.CompileMetadataResolver,
       private directiveNormalizer: compiler.DirectiveNormalizer) {}
 
-  private readModuleSymbols(absSourcePath: string): StaticSymbol[] {
-    const moduleMetadata = this.staticReflector.getModuleMetadata(absSourcePath);
-    const modSymbols: StaticSymbol[] = [];
-    if (!moduleMetadata) {
-      console.log(`WARNING: no metadata found for ${absSourcePath}`);
-      return modSymbols;
-    }
-
-    const metadata = moduleMetadata['metadata'];
-    const symbols = metadata && Object.keys(metadata);
-    if (!symbols || !symbols.length) {
-      return modSymbols;
-    }
-
-    for (const symbol of symbols) {
-      if (metadata[symbol] && metadata[symbol].__symbolic == 'error') {
-        // Ignore symbols that are only included to record error information.
-        continue;
-      }
-
-      const staticType = this.reflectorHost.findDeclaration(absSourcePath, symbol, absSourcePath);
-      const annotations = this.staticReflector.annotations(staticType);
-
-      annotations.some(a => {
-        if (a instanceof NgModule) {
-          modSymbols.push(staticType);
-          return true;
-        }
-      });
-    }
-
-    return modSymbols;
-  }
-
   extract(): Promise<compiler.MessageBundle> {
-    const filePaths =
+    const programSymbols: StaticSymbol[] =
-        this.program.getSourceFiles().map(sf => sf.fileName).filter(f => !GENERATED_FILES.test(f));
+        extractProgramSymbols(this.program, this.staticReflector, this.reflectorHost, this.options);
-    const ngModules: StaticSymbol[] = [];
 
-    filePaths.forEach((filePath) => ngModules.push(...this.readModuleSymbols(filePath)));
+    const files =
-
+        compiler.analyzeNgModules(programSymbols, {transitiveModules: true}, this.metadataResolver)
-    const files = compiler.analyzeNgModules(ngModules, this.metadataResolver).files;
+            .files;
     const errors: compiler.ParseError[] = [];
     const filePromises: Promise<any>[] = [];
 

modules/@angular/compiler-cli/src/main.ts

@@ -19,7 +19,9 @@ import {CodeGenerator} from './codegen';
 function codegen(
     ngOptions: tsc.AngularCompilerOptions, cliOptions: tsc.NgcCliOptions, program: ts.Program,
     host: ts.CompilerHost) {
-  return CodeGenerator.create(ngOptions, cliOptions, program, host).codegen();
+  return CodeGenerator.create(ngOptions, cliOptions, program, host).codegen({
+    transitiveModules: true
+  });
 }
 
 // CLI entry point

modules/@angular/compiler/src/offline_compiler.ts

@@ -29,56 +29,81 @@ export class SourceModule {
 
 // Returns all the source files and a mapping from modules to directives
 export function analyzeNgModules(
-    ngModules: StaticSymbol[], metadataResolver: CompileMetadataResolver): {
+    programStaticSymbols: StaticSymbol[], options: {transitiveModules: boolean},
-  ngModuleByDirective: Map<StaticSymbol, CompileNgModuleMetadata>,
+    metadataResolver: CompileMetadataResolver): {
+  ngModuleByPipeOrDirective: Map<StaticSymbol, CompileNgModuleMetadata>,
   files: Array<{srcUrl: string, directives: StaticSymbol[], ngModules: StaticSymbol[]}>
 } {
+  const {
+    ngModules: programNgModules,
+    pipesAndDirectives: programPipesOrDirectives,
+  } = _extractModulesAndPipesOrDirectives(programStaticSymbols, metadataResolver);
+
   const moduleMetasByRef = new Map<any, CompileNgModuleMetadata>();
 
+  programNgModules.forEach(modMeta => {
+    if (options.transitiveModules) {
       // For every input modules add the list of transitively included modules
-  ngModules.forEach(ngModule => {
-    const modMeta = metadataResolver.getNgModuleMetadata(ngModule);
       modMeta.transitiveModule.modules.forEach(
           modMeta => { moduleMetasByRef.set(modMeta.type.reference, modMeta); });
+    } else {
+      moduleMetasByRef.set(modMeta.type.reference, modMeta);
+    }
   });
 
   const ngModuleMetas = MapWrapper.values(moduleMetasByRef);
-  const ngModuleByDirective = new Map<StaticSymbol, CompileNgModuleMetadata>();
+  const ngModuleByPipeOrDirective = new Map<StaticSymbol, CompileNgModuleMetadata>();
   const ngModulesByFile = new Map<string, StaticSymbol[]>();
   const ngDirectivesByFile = new Map<string, StaticSymbol[]>();
-  const srcFileUrls = new Set<string>();
+  const filePaths = new Set<string>();
 
   // Looping over all modules to construct:
-  // - a map from files to modules `ngModulesByFile`,
+  // - a map from file to modules `ngModulesByFile`,
-  // - a map from files to directives `ngDirectivesByFile`,
+  // - a map from file to directives `ngDirectivesByFile`,
-  // - a map from modules to directives `ngModuleByDirective`.
+  // - a map from directive/pipe to module `ngModuleByPipeOrDirective`.
   ngModuleMetas.forEach((ngModuleMeta) => {
     const srcFileUrl = ngModuleMeta.type.reference.filePath;
-    srcFileUrls.add(srcFileUrl);
+    filePaths.add(srcFileUrl);
     ngModulesByFile.set(
         srcFileUrl, (ngModulesByFile.get(srcFileUrl) || []).concat(ngModuleMeta.type.reference));
 
     ngModuleMeta.declaredDirectives.forEach((dirMeta: CompileDirectiveMetadata) => {
       const fileUrl = dirMeta.type.reference.filePath;
-      srcFileUrls.add(fileUrl);
+      filePaths.add(fileUrl);
       ngDirectivesByFile.set(
           fileUrl, (ngDirectivesByFile.get(fileUrl) || []).concat(dirMeta.type.reference));
-      ngModuleByDirective.set(dirMeta.type.reference, ngModuleMeta);
+      ngModuleByPipeOrDirective.set(dirMeta.type.reference, ngModuleMeta);
+    });
+
+    ngModuleMeta.declaredPipes.forEach((pipeMeta: CompilePipeMetadata) => {
+      const fileUrl = pipeMeta.type.reference.filePath;
+      filePaths.add(fileUrl);
+      ngModuleByPipeOrDirective.set(pipeMeta.type.reference, ngModuleMeta);
     });
   });
 
+  // Throw an error if any of the program pipe or directives is not declared by a module
+  const symbolsMissingModule =
+      programPipesOrDirectives.filter(s => !ngModuleByPipeOrDirective.has(s));
+
+  if (symbolsMissingModule.length) {
+    const messages = symbolsMissingModule.map(
+        s => `Cannot determine the module for class ${s.name} in ${s.filePath}!`);
+    throw new Error(messages.join('\n'));
+  }
+
   const files: {srcUrl: string, directives: StaticSymbol[], ngModules: StaticSymbol[]}[] = [];
 
-  srcFileUrls.forEach((srcUrl) => {
+  filePaths.forEach((srcUrl) => {
     const directives = ngDirectivesByFile.get(srcUrl) || [];
     const ngModules = ngModulesByFile.get(srcUrl) || [];
     files.push({srcUrl, directives, ngModules});
   });
 
   return {
-      // map from modules to declared directives
+      // map directive/pipe to module
-      ngModuleByDirective,
+      ngModuleByPipeOrDirective,
-      // list of modules and directives for every source file
+      // list modules and directives for every source file
       files,
   };
 }
@@ -100,19 +125,21 @@ export class OfflineCompiler {
     this._metadataResolver.clearCache();
   }
 
-  compileModules(ngModules: StaticSymbol[]): Promise<SourceModule[]> {
+  compileModules(staticSymbols: StaticSymbol[], options: {transitiveModules: boolean}):
-    const {ngModuleByDirective, files} = analyzeNgModules(ngModules, this._metadataResolver);
+      Promise<SourceModule[]> {
+    const {ngModuleByPipeOrDirective, files} =
+        analyzeNgModules(staticSymbols, options, this._metadataResolver);
 
     const sourceModules = files.map(
         file => this._compileSrcFile(
-            file.srcUrl, ngModuleByDirective, file.directives, file.ngModules));
+            file.srcUrl, ngModuleByPipeOrDirective, file.directives, file.ngModules));
 
     return Promise.all(sourceModules)
         .then((modules: SourceModule[][]) => ListWrapper.flatten(modules));
   }
 
   private _compileSrcFile(
-      srcFileUrl: string, ngModuleByDirective: Map<StaticSymbol, CompileNgModuleMetadata>,
+      srcFileUrl: string, ngModuleByPipeOrDirective: Map<StaticSymbol, CompileNgModuleMetadata>,
       directives: StaticSymbol[], ngModules: StaticSymbol[]): Promise<SourceModule[]> {
     const fileSuffix = _splitTypescriptSuffix(srcFileUrl)[1];
     const statements: o.Statement[] = [];
@@ -134,9 +161,10 @@ export class OfflineCompiler {
           if (!compMeta.isComponent) {
             return Promise.resolve(null);
           }
-          const ngModule = ngModuleByDirective.get(dirType);
+          const ngModule = ngModuleByPipeOrDirective.get(dirType);
           if (!ngModule) {
-            throw new Error(`Cannot determine the module for component ${compMeta.type.name}!`);
+            throw new Error(
+                `Internal Error: cannot determine the module for component ${compMeta.type.name}!`);
           }
 
           return Promise
@@ -330,3 +358,28 @@ function _splitTypescriptSuffix(path: string): string[] {
 
   return [path, ''];
 }
+
+// Group the symbols by types:
+// - NgModules,
+// - Pipes and Directives.
+function _extractModulesAndPipesOrDirectives(
+    programStaticSymbols: StaticSymbol[], metadataResolver: CompileMetadataResolver) {
+  const ngModules: CompileNgModuleMetadata[] = [];
+  const pipesAndDirectives: StaticSymbol[] = [];
+
+  programStaticSymbols.forEach(staticSymbol => {
+    const ngModule = metadataResolver.getNgModuleMetadata(staticSymbol, false);
+    const directive = metadataResolver.getDirectiveMetadata(staticSymbol, false);
+    const pipe = metadataResolver.getPipeMetadata(<any>staticSymbol, false);
+
+    if (ngModule) {
+      ngModules.push(ngModule);
+    } else if (directive) {
+      pipesAndDirectives.push(staticSymbol);
+    } else if (pipe) {
+      pipesAndDirectives.push(staticSymbol);
+    }
+  });
+
+  return {ngModules, pipesAndDirectives};
+}