From 722163222808c7a0071a872013036675462b916b Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 25 Oct 2016 16:28:22 -0700 Subject: [PATCH] fix(CompilerCli): assert that all pipes and directives are declared by a module --- modules/@angular/compiler-cli/src/codegen.ts | 103 ++++++++---------- .../@angular/compiler-cli/src/extractor.ts | 54 ++------- modules/@angular/compiler-cli/src/main.ts | 4 +- .../@angular/compiler/src/offline_compiler.ts | 103 +++++++++++++----- 4 files changed, 133 insertions(+), 131 deletions(-) diff --git a/modules/@angular/compiler-cli/src/codegen.ts b/modules/@angular/compiler-cli/src/codegen.ts index 569e66080b..2aedf26ed2 100644 --- a/modules/@angular/compiler-cli/src/codegen.ts +++ b/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) { - this.moduleCollector = - new CodeGeneratorModuleCollector(staticReflector, reflectorHost, program, options); - } + private compiler: compiler.OfflineCompiler, private reflectorHost: StaticReflectorHost) {} // 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 { - const ngModules = this.moduleCollector.getModuleSymbols(); + codegen(options: {transitiveModules: boolean}): Promise { + 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; +} \ No newline at end of file diff --git a/modules/@angular/compiler-cli/src/extractor.ts b/modules/@angular/compiler-cli/src/extractor.ts index cd93036626..4b9aacf99c 100644 --- a/modules/@angular/compiler-cli/src/extractor.ts +++ b/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 { - const filePaths = - this.program.getSourceFiles().map(sf => sf.fileName).filter(f => !GENERATED_FILES.test(f)); - const ngModules: StaticSymbol[] = []; + const programSymbols: StaticSymbol[] = + extractProgramSymbols(this.program, this.staticReflector, this.reflectorHost, this.options); - filePaths.forEach((filePath) => ngModules.push(...this.readModuleSymbols(filePath))); - - const files = compiler.analyzeNgModules(ngModules, this.metadataResolver).files; + const files = + compiler.analyzeNgModules(programSymbols, {transitiveModules: true}, this.metadataResolver) + .files; const errors: compiler.ParseError[] = []; const filePromises: Promise[] = []; diff --git a/modules/@angular/compiler-cli/src/main.ts b/modules/@angular/compiler-cli/src/main.ts index 265687c6e0..73312909ee 100644 --- a/modules/@angular/compiler-cli/src/main.ts +++ b/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 diff --git a/modules/@angular/compiler/src/offline_compiler.ts b/modules/@angular/compiler/src/offline_compiler.ts index 7ae27e5c51..1dec0f2a7f 100644 --- a/modules/@angular/compiler/src/offline_compiler.ts +++ b/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): { - ngModuleByDirective: Map, + programStaticSymbols: StaticSymbol[], options: {transitiveModules: boolean}, + metadataResolver: CompileMetadataResolver): { + ngModuleByPipeOrDirective: Map, files: Array<{srcUrl: string, directives: StaticSymbol[], ngModules: StaticSymbol[]}> } { + const { + ngModules: programNgModules, + pipesAndDirectives: programPipesOrDirectives, + } = _extractModulesAndPipesOrDirectives(programStaticSymbols, metadataResolver); + const moduleMetasByRef = new Map(); - // 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); }); + programNgModules.forEach(modMeta => { + if (options.transitiveModules) { + // For every input modules add the list of transitively included modules + 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(); + const ngModuleByPipeOrDirective = new Map(); const ngModulesByFile = new Map(); const ngDirectivesByFile = new Map(); - const srcFileUrls = new Set(); + const filePaths = new Set(); // Looping over all modules to construct: - // - a map from files to modules `ngModulesByFile`, - // - a map from files to directives `ngDirectivesByFile`, - // - a map from modules to directives `ngModuleByDirective`. + // - a map from file to modules `ngModulesByFile`, + // - a map from file to directives `ngDirectivesByFile`, + // - 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 - ngModuleByDirective, - // list of modules and directives for every source file + // map directive/pipe to module + ngModuleByPipeOrDirective, + // list modules and directives for every source file files, }; } @@ -100,19 +125,21 @@ export class OfflineCompiler { this._metadataResolver.clearCache(); } - compileModules(ngModules: StaticSymbol[]): Promise { - const {ngModuleByDirective, files} = analyzeNgModules(ngModules, this._metadataResolver); + compileModules(staticSymbols: StaticSymbol[], options: {transitiveModules: boolean}): + Promise { + 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, + srcFileUrl: string, ngModuleByPipeOrDirective: Map, directives: StaticSymbol[], ngModules: StaticSymbol[]): Promise { 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(staticSymbol, false); + + if (ngModule) { + ngModules.push(ngModule); + } else if (directive) { + pipesAndDirectives.push(staticSymbol); + } else if (pipe) { + pipesAndDirectives.push(staticSymbol); + } + }); + + return {ngModules, pipesAndDirectives}; +}