From 0658eb442955688be8da86f5366f8f38a561e617 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Thu, 9 Jun 2016 14:51:53 -0700 Subject: [PATCH] fix(compiler): Added unit test to ReflectorHost and fixed issues (#9052) Refactored ReflectorHost to allow it to be tested. Fixed an issue where the .d.ts was findable but it wasn't used by the project (This happens when the .metadata.json file references a module that was not needed, such as it doesn't declare any types, and the reference to it was elided by TypeScript when writing the .d.ts file). Added tests for ReflectorHost --- modules/@angular/compiler-cli/index.ts | 2 +- modules/@angular/compiler-cli/src/codegen.ts | 9 +- .../@angular/compiler-cli/src/extract_i18n.ts | 6 +- .../compiler-cli/src/reflector_host.ts | 45 +++++- modules/@angular/compiler-cli/test/mocks.ts | 118 +++++++++++++++ .../compiler-cli/test/reflector_host_spec.ts | 138 ++++++++++++++++++ 6 files changed, 303 insertions(+), 15 deletions(-) create mode 100644 modules/@angular/compiler-cli/test/mocks.ts create mode 100644 modules/@angular/compiler-cli/test/reflector_host_spec.ts diff --git a/modules/@angular/compiler-cli/index.ts b/modules/@angular/compiler-cli/index.ts index 814aca2eb5..3656139f67 100644 --- a/modules/@angular/compiler-cli/index.ts +++ b/modules/@angular/compiler-cli/index.ts @@ -1,4 +1,4 @@ export {CodeGenerator} from './src/codegen'; -export {NodeReflectorHost} from './src/reflector_host'; +export {ReflectorHost, ReflectorHostContext, NodeReflectorHostContext} from './src/reflector_host'; export {StaticReflector, StaticReflectorHost, StaticSymbol} from './src/static_reflector'; export * from '@angular/tsc-wrapped'; diff --git a/modules/@angular/compiler-cli/src/codegen.ts b/modules/@angular/compiler-cli/src/codegen.ts index 1fc62d1ea0..511d946692 100644 --- a/modules/@angular/compiler-cli/src/codegen.ts +++ b/modules/@angular/compiler-cli/src/codegen.ts @@ -24,7 +24,7 @@ import { import {Parse5DomAdapter} from '@angular/platform-server'; -import {NodeReflectorHost} from './reflector_host'; +import {ReflectorHost, ReflectorHostContext} from './reflector_host'; import {StaticAndDynamicReflectionCapabilities} from './static_reflection_capabilities'; const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/; @@ -42,7 +42,7 @@ export class CodeGenerator { private program: ts.Program, public host: ts.CompilerHost, private staticReflector: StaticReflector, private resolver: CompileMetadataResolver, private compiler: compiler.OfflineCompiler, - private reflectorHost: NodeReflectorHost) {} + private reflectorHost: ReflectorHost) {} private generateSource(metadatas: compiler.CompileDirectiveMetadata[]) { const normalize = (metadata: compiler.CompileDirectiveMetadata) => { @@ -155,10 +155,11 @@ export class CodeGenerator { } static create(options: AngularCompilerOptions, program: ts.Program, - compilerHost: ts.CompilerHost): CodeGenerator { + compilerHost: ts.CompilerHost, + reflectorHostContext?: ReflectorHostContext): CodeGenerator { const xhr: compiler.XHR = {get: (s: string) => Promise.resolve(compilerHost.readFile(s))}; const urlResolver: compiler.UrlResolver = compiler.createOfflineCompileUrlResolver(); - const reflectorHost = new NodeReflectorHost(program, compilerHost, options); + const reflectorHost = new ReflectorHost(program, compilerHost, options, reflectorHostContext); const staticReflector = new StaticReflector(reflectorHost); StaticAndDynamicReflectionCapabilities.install(staticReflector); const htmlParser = new HtmlParser(); diff --git a/modules/@angular/compiler-cli/src/extract_i18n.ts b/modules/@angular/compiler-cli/src/extract_i18n.ts index 4cdf9a1084..2666d43c00 100644 --- a/modules/@angular/compiler-cli/src/extract_i18n.ts +++ b/modules/@angular/compiler-cli/src/extract_i18n.ts @@ -34,7 +34,7 @@ import { import {Parse5DomAdapter} from '@angular/platform-server'; -import {NodeReflectorHost} from './reflector_host'; +import {ReflectorHost} from './reflector_host'; import {StaticAndDynamicReflectionCapabilities} from './static_reflection_capabilities'; @@ -49,7 +49,7 @@ class Extractor { private program: ts.Program, public host: ts.CompilerHost, private staticReflector: StaticReflector, private resolver: CompileMetadataResolver, private compiler: compiler.OfflineCompiler, - private reflectorHost: NodeReflectorHost, private _extractor: MessageExtractor) {} + private reflectorHost: ReflectorHost, private _extractor: MessageExtractor) {} private extractCmpMessages(metadatas: compiler.CompileDirectiveMetadata[]): Promise { if (!metadatas || !metadatas.length) { @@ -151,7 +151,7 @@ class Extractor { compilerHost: ts.CompilerHost): Extractor { const xhr: compiler.XHR = {get: (s: string) => Promise.resolve(compilerHost.readFile(s))}; const urlResolver: compiler.UrlResolver = compiler.createOfflineCompileUrlResolver(); - const reflectorHost = new NodeReflectorHost(program, compilerHost, options); + const reflectorHost = new ReflectorHost(program, compilerHost, options); const staticReflector = new StaticReflector(reflectorHost); StaticAndDynamicReflectionCapabilities.install(staticReflector); const htmlParser = new HtmlParser(); diff --git a/modules/@angular/compiler-cli/src/reflector_host.ts b/modules/@angular/compiler-cli/src/reflector_host.ts index 69bcb4bfbe..85a1a20561 100644 --- a/modules/@angular/compiler-cli/src/reflector_host.ts +++ b/modules/@angular/compiler-cli/src/reflector_host.ts @@ -5,14 +5,23 @@ import * as fs from 'fs'; import * as path from 'path'; import {ImportGenerator, AssetUrl} from './compiler_private'; - const EXT = /(\.ts|\.d\.ts|\.js|\.jsx|\.tsx)$/; const DTS = /\.d\.ts$/; -export class NodeReflectorHost implements StaticReflectorHost, ImportGenerator { +export interface ReflectorHostContext { + exists(fileName: string): boolean; + read(fileName: string): string; + write(fileName: string, data: string): void; +} + +export class ReflectorHost implements StaticReflectorHost, ImportGenerator { private metadataCollector = new MetadataCollector(); + private context: ReflectorHostContext; constructor(private program: ts.Program, private compilerHost: ts.CompilerHost, - private options: AngularCompilerOptions) {} + private options: AngularCompilerOptions, + context?: ReflectorHostContext) { + this.context = context || new NodeReflectorHostContext(); + } angularImportLocations() { return { @@ -59,7 +68,7 @@ export class NodeReflectorHost implements StaticReflectorHost, ImportGenerator { console.log(`Generating empty file ${importedFile} to allow resolution of import`); } this.compilerHost.writeFile(importedFile, '', false); - fs.writeFileSync(importedFile, ''); + this.context.write(importedFile, ''); } const importModuleName = importedFile.replace(EXT, ''); @@ -108,6 +117,14 @@ export class NodeReflectorHost implements StaticReflectorHost, ImportGenerator { const tc = this.program.getTypeChecker(); const sf = this.program.getSourceFile(filePath); + if (!sf || !(sf).symbol) { + // The source file was not needed in the compile but we do need the values from + // the corresponding .ts files stored in the .metadata.json file. Just assume the + // symbol and file we resolved to be correct as we don't need this to be the + // cannonical reference as this reference could have only been generated by a + // .metadata.json file resolving values. + return this.getStaticSymbol(filePath, symbolName); + } let symbol = tc.getExportsOfModule((sf).symbol).find(m => m.name === symbolName); if (!symbol) { @@ -148,12 +165,12 @@ export class NodeReflectorHost implements StaticReflectorHost, ImportGenerator { // TODO(alexeagle): take a statictype getMetadataFor(filePath: string): ModuleMetadata { - if (!fs.existsSync(filePath)) { + if (!this.context.exists(filePath)) { throw new Error(`No such file '${filePath}'`); } if (DTS.test(filePath)) { const metadataPath = filePath.replace(DTS, '.metadata.json'); - if (fs.existsSync(metadataPath)) { + if (this.context.exists(metadataPath)) { return this.readMetadata(metadataPath); } } @@ -168,7 +185,7 @@ export class NodeReflectorHost implements StaticReflectorHost, ImportGenerator { readMetadata(filePath: string) { try { - const result = JSON.parse(fs.readFileSync(filePath, {encoding: 'utf-8'})); + const result = JSON.parse(this.context.read(filePath)); return result; } catch (e) { console.error(`Failed to read JSON file ${filePath}`); @@ -176,3 +193,17 @@ export class NodeReflectorHost implements StaticReflectorHost, ImportGenerator { } } } + +export class NodeReflectorHostContext implements ReflectorHostContext { + exists(fileName: string): boolean { + return fs.existsSync(fileName); + } + + read(fileName: string): string { + return fs.readFileSync(fileName, 'utf8'); + } + + write(fileName: string, data: string): void { + fs.writeFileSync(fileName, data, 'utf8'); + } +} diff --git a/modules/@angular/compiler-cli/test/mocks.ts b/modules/@angular/compiler-cli/test/mocks.ts new file mode 100644 index 0000000000..a210cfa243 --- /dev/null +++ b/modules/@angular/compiler-cli/test/mocks.ts @@ -0,0 +1,118 @@ +import * as ts from 'typescript'; + +import {ReflectorHost, ReflectorHostContext} from '../src/reflector_host'; + +export type Entry = string | Directory; + +export interface Directory { + [name: string]: Entry; +} + +export class MockContext implements ReflectorHostContext { + constructor (public currentDirectory: string, private files: Entry) {} + + exists(fileName: string): boolean { + return this.getEntry(fileName) !== undefined; + } + + read(fileName: string): string | undefined { + let data = this.getEntry(fileName); + if (typeof data === "string") { + return data; + } + return undefined; + } + + write(fileName: string, data: string): void { + let parts = fileName.split('/'); + let name = parts.pop(); + let entry = this.getEntry(parts); + if (entry && typeof entry !== "string") { + entry[name] = data; + } + } + + getEntry(fileName: string | string[]): Entry | undefined { + let parts = typeof fileName === "string" ? fileName.split('/') : fileName; + if (parts[0]) { + parts = this.currentDirectory.split('/').concat(parts); + } + parts.shift(); + parts = normalize(parts); + let current = this.files; + while (parts.length) { + let part = parts.shift(); + if (typeof current === "string") { + return undefined; + } + let next = (current)[part]; + if (next === undefined) { + return undefined; + } + current = next; + } + return current; + } +} + +function normalize(parts: string[]): string[] { + let result: string[] = []; + while (parts.length) { + let part = parts.shift(); + switch (part) { + case '.': break; + case '..': result.pop(); break; + default: result.push(part); + } + } + return result; +} + +export class MockCompilerHost implements ts.CompilerHost { + constructor (private context: MockContext) {} + + fileExists(fileName: string): boolean { + return this.context.exists(fileName); + } + + readFile(fileName: string): string { + return this.context.read(fileName); + } + + directoryExists(directoryName: string): boolean { + return this.context.exists(directoryName); + } + + getSourceFile(fileName: string, languageVersion: ts.ScriptTarget, onError?: (message: string) => void): ts.SourceFile { + let sourceText = this.context.read(fileName); + if (sourceText) { + return ts.createSourceFile(fileName, sourceText, languageVersion); + } else { + return undefined; + } + } + + getDefaultLibFileName(options: ts.CompilerOptions): string { + return ts.getDefaultLibFileName(options); + } + + writeFile: ts.WriteFileCallback = (fileName, text) => { + this.context.write(fileName, text); + } + + getCurrentDirectory(): string { + return this.context.currentDirectory; + } + + getCanonicalFileName(fileName: string): string { + return fileName; + } + + useCaseSensitiveFileNames(): boolean { + return false; + } + + getNewLine(): string { + return '\n'; + } +} diff --git a/modules/@angular/compiler-cli/test/reflector_host_spec.ts b/modules/@angular/compiler-cli/test/reflector_host_spec.ts new file mode 100644 index 0000000000..360ee104f5 --- /dev/null +++ b/modules/@angular/compiler-cli/test/reflector_host_spec.ts @@ -0,0 +1,138 @@ +import * as ts from 'typescript'; + +import { + describe, + it, + iit, + expect, + ddescribe, + beforeEach +} from '@angular/core/testing/testing_internal'; + +import {ReflectorHost, ReflectorHostContext} from '../src/reflector_host'; + +import {Directory, Entry, MockContext, MockCompilerHost} from './mocks'; + +describe('reflector_host', () => { + var context: MockContext; + var host: ts.CompilerHost; + var program: ts.Program; + var reflectorHost: ReflectorHost; + + beforeEach(() => { + context = new MockContext('/tmp/src', clone(FILES)); + host = new MockCompilerHost(context) + program = ts.createProgram(['main.ts'], { + module: ts.ModuleKind.CommonJS, + }, host); + // Force a typecheck + let errors = program.getSemanticDiagnostics(); + if (errors && errors.length) { + throw new Error('Expected no errors'); + } + reflectorHost = new ReflectorHost(program, host, { + genDir: '/tmp/dist', + basePath: '/tmp/src', + skipMetadataEmit: false, + skipTemplateCodegen: false, + trace: false + }, context); + }); + + it('should provide the import locations for angular', () => { + let {coreDecorators, diDecorators, diMetadata, animationMetadata, provider} = reflectorHost.angularImportLocations(); + expect(coreDecorators).toEqual('@angular/core/src/metadata'); + expect(diDecorators).toEqual('@angular/core/src/di/decorators'); + expect(diMetadata).toEqual('@angular/core/src/di/metadata'); + expect(animationMetadata).toEqual('@angular/core/src/animation/metadata'); + expect(provider).toEqual('@angular/core/src/di/provider'); + }); + + it('should be able to produce an import from main @angular/core', () => { + expect(reflectorHost.getImportPath('main.ts', 'node_modules/@angular/core.d.ts')).toEqual('@angular/core'); + }); + + it('should be ble to produce an import from main to a sub-directory', () => { + expect(reflectorHost.getImportPath('main.ts', 'lib/utils.ts')).toEqual('./lib/utils'); + }); + + it('should be able to produce an import from to a peer file', () => { + expect(reflectorHost.getImportPath('lib/utils.ts', 'lib/collections.ts')).toEqual('./collections'); + }); + + it('should be able to produce an import from to a sibling directory', () => { + expect(reflectorHost.getImportPath('lib2/utils2.ts', 'lib/utils.ts')).toEqual('../lib/utils'); + }); + + it('should be able to produce a symbol for an exported symbol', () => { + expect(reflectorHost.findDeclaration('@angular/router', 'foo', 'main.ts')).toBeDefined(); + }); + + it('should be able to produce a symbol for values space only reference', () => { + expect(reflectorHost.findDeclaration('@angular/router/src/providers', 'foo', 'main.ts')).toBeDefined(); + }); + + it('should be produce the same symbol if asked twice', () => { + let foo1 = reflectorHost.getStaticSymbol('main.ts', 'foo'); + let foo2 = reflectorHost.getStaticSymbol('main.ts', 'foo'); + expect(foo1).toBe(foo2); + }); + + it('should be able to read a metadata file', () => { + expect(reflectorHost.getMetadataFor('node_modules/@angular/core.d.ts')).toEqual({ + __symbolic: "module", + version: 1, + metadata: { + foo: { + __symbolic: "class" + } + } + }) + }); +}); + +const dummyModule = 'export let foo: any[];' +const FILES: Entry = { + 'tmp': { + 'src': { + 'main.ts': ` + import * as c from '@angular/core'; + import * as r from '@angular/router'; + import * as u from './lib/utils'; + import * as cs from './lib/collections'; + import * as u2 from './lib2/utils2'; + `, + 'lib': { + 'utils.ts': dummyModule, + 'collections.ts': dummyModule, + }, + 'lib2': { + 'utils2.ts': dummyModule + }, + 'node_modules': { + '@angular': { + 'core.d.ts': dummyModule, + 'core.metadata.json': `{"__symbolic":"module", "version": 1, "metadata": {"foo": {"__symbolic": "class"}}}`, + 'router': { + 'index.d.ts': dummyModule, + 'src': { + 'providers.d.ts': dummyModule + } + } + } + } + } + } +} + +function clone(entry: Entry): Entry { + if (typeof entry === "string") { + return entry; + } else { + let result: Directory = {}; + for (let name in entry) { + result[name] = clone(entry[name]); + } + return result; + } +}