Comiler fixes

packages/compiler-cli/src/diagnostics/translate_diagnostics.ts

@@ -38,10 +38,10 @@ export function translateDiagnostics(host: TypeCheckHost, untranslatedDiagnostic
           source: SOURCE,
           code: DEFAULT_ERROR_CODE
         });
-        return;
       }
+    } else {
+      ts.push(diagnostic);
     }
-    ts.push(diagnostic);
   });
   return {ts, ng};
 }

packages/compiler-cli/src/perform_watch.ts

@@ -129,6 +129,7 @@ export function performWatchCompilation(host: PerformWatchHost):
   return {close, ready: cb => readyPromise.then(cb), firstCompileResult};
 
   function cacheEntry(fileName: string): CacheEntry {
+    fileName = path.normalize(fileName);
     let entry = fileCache.get(fileName);
     if (!entry) {
       entry = {};
@@ -191,6 +192,10 @@ export function performWatchCompilation(host: PerformWatchHost):
       };
     }
     ingoreFilesForWatch.clear();
+    const oldProgram = cachedProgram;
+    // We clear out the `cachedProgram` here as a
+    // program can only be used as `oldProgram` 1x
+    cachedProgram = undefined;
     const compileResult = performCompilation({
       rootNames: cachedOptions.rootNames,
       options: cachedOptions.options,
@@ -245,7 +250,7 @@ export function performWatchCompilation(host: PerformWatchHost):
     if (event === FileChangeEvent.CreateDeleteDir) {
       fileCache.clear();
     } else {
-      fileCache.delete(fileName);
+      fileCache.delete(path.normalize(fileName));
     }
 
     if (!ingoreFilesForWatch.has(path.normalize(fileName))) {

packages/compiler-cli/src/transformers/program.ts

@@ -18,7 +18,7 @@ import {CompilerHost, CompilerOptions, CustomTransformers, DEFAULT_ERROR_CODE, D
 import {CodeGenerator, TsCompilerAotCompilerTypeCheckHostAdapter, getOriginalReferences} from './compiler_host';
 import {LowerMetadataCache, getExpressionLoweringTransformFactory} from './lower_expressions';
 import {getAngularEmitterTransformFactory} from './node_emitter_transform';
-import {GENERATED_FILES, StructureIsReused, createMessageDiagnostic, isInRootDir, tsStructureIsReused} from './util';
+import {GENERATED_FILES, StructureIsReused, createMessageDiagnostic, isInRootDir, ngToTsDiagnostic, tsStructureIsReused} from './util';
 
 
 
@@ -149,11 +149,9 @@ class AngularCompilerProgram implements Program {
 
   getTsSemanticDiagnostics(sourceFile?: ts.SourceFile, cancellationToken?: ts.CancellationToken):
       ts.Diagnostic[] {
-    if (sourceFile) {
-      return this.tsProgram.getSemanticDiagnostics(sourceFile, cancellationToken);
-    }
+    const sourceFiles = sourceFile ? [sourceFile] : this.tsProgram.getSourceFiles();
     let diags: ts.Diagnostic[] = [];
-    this.tsProgram.getSourceFiles().forEach(sf => {
+    sourceFiles.forEach(sf => {
       if (!GENERATED_FILES.test(sf.fileName)) {
         diags.push(...this.tsProgram.getSemanticDiagnostics(sf, cancellationToken));
       }
@@ -177,15 +175,17 @@ class AngularCompilerProgram implements Program {
     if (this._analyzedModules) {
       throw new Error('Angular structure already loaded');
     }
-    const {tmpProgram, sourceFiles, rootNames} = this._createProgramWithBasicStubs();
-    return this.compiler.loadFilesAsync(sourceFiles)
-        .catch(this.catchAnalysisError.bind(this))
-        .then(analyzedModules => {
-          if (this._analyzedModules) {
-            throw new Error('Angular structure loaded both synchronously and asynchronsly');
-          }
-          this._updateProgramWithTypeCheckStubs(tmpProgram, analyzedModules, rootNames);
-        });
+    return Promise.resolve()
+        .then(() => {
+          const {tmpProgram, sourceFiles, rootNames} = this._createProgramWithBasicStubs();
+          return this.compiler.loadFilesAsync(sourceFiles).then(analyzedModules => {
+            if (this._analyzedModules) {
+              throw new Error('Angular structure loaded both synchronously and asynchronsly');
+            }
+            this._updateProgramWithTypeCheckStubs(tmpProgram, analyzedModules, rootNames);
+          });
+        })
+        .catch(e => this._createProgramOnError(e));
   }
 
   listLazyRoutes(route?: string): LazyRoute[] {
@@ -300,6 +300,10 @@ class AngularCompilerProgram implements Program {
       }
     }
     this.emittedSourceFiles = emittedSourceFiles;
+    // translate the diagnostics in the emitResult as well.
+    const translatedEmitDiags = translateDiagnostics(this.hostAdapter, emitResult.diagnostics);
+    emitResult.diagnostics = translatedEmitDiags.ts.concat(
+        this.structuralDiagnostics.concat(translatedEmitDiags.ng).map(ngToTsDiagnostic));
 
     if (!outSrcMapping.length) {
       // if no files were emitted by TypeScript, also don't emit .json files
@@ -400,14 +404,13 @@ class AngularCompilerProgram implements Program {
     if (this._analyzedModules) {
       return;
     }
-    const {tmpProgram, sourceFiles, rootNames} = this._createProgramWithBasicStubs();
-    let analyzedModules: NgAnalyzedModules|null;
     try {
-      analyzedModules = this.compiler.loadFilesSync(sourceFiles);
+      const {tmpProgram, sourceFiles, rootNames} = this._createProgramWithBasicStubs();
+      const analyzedModules = this.compiler.loadFilesSync(sourceFiles);
+      this._updateProgramWithTypeCheckStubs(tmpProgram, analyzedModules, rootNames);
     } catch (e) {
-      analyzedModules = this.catchAnalysisError(e);
+      this._createProgramOnError(e);
     }
-    this._updateProgramWithTypeCheckStubs(tmpProgram, analyzedModules, rootNames);
   }
 
   private _createCompiler() {
@@ -455,7 +458,7 @@ class AngularCompilerProgram implements Program {
 
     let rootNames = this.rootNames;
     if (this.options.generateCodeForLibraries !== false) {
-      // if we should generateCodeForLibraries, enver include
+      // if we should generateCodeForLibraries, never include
       // generated files in the program as otherwise we will
       // ovewrite them and typescript will report the error
       // TS5055: Cannot write file ... because it would overwrite input file.
@@ -480,23 +483,21 @@ class AngularCompilerProgram implements Program {
   }
 
   private _updateProgramWithTypeCheckStubs(
-      tmpProgram: ts.Program, analyzedModules: NgAnalyzedModules|null, rootNames: string[]) {
-    this._analyzedModules = analyzedModules || emptyModules;
-    if (analyzedModules) {
-      tmpProgram.getSourceFiles().forEach(sf => {
-        if (sf.fileName.endsWith('.ngfactory.ts')) {
-          const {generate, baseFileName} = this.hostAdapter.shouldGenerateFile(sf.fileName);
-          if (generate) {
-            // Note: ! is ok as hostAdapter.shouldGenerateFile will always return a basefileName
-            // for .ngfactory.ts files.
-            const genFile = this.compiler.emitTypeCheckStub(sf.fileName, baseFileName !);
-            if (genFile) {
-              this.hostAdapter.updateGeneratedFile(genFile);
-            }
+      tmpProgram: ts.Program, analyzedModules: NgAnalyzedModules, rootNames: string[]) {
+    this._analyzedModules = analyzedModules;
+    tmpProgram.getSourceFiles().forEach(sf => {
+      if (sf.fileName.endsWith('.ngfactory.ts')) {
+        const {generate, baseFileName} = this.hostAdapter.shouldGenerateFile(sf.fileName);
+        if (generate) {
+          // Note: ! is ok as hostAdapter.shouldGenerateFile will always return a basefileName
+          // for .ngfactory.ts files.
+          const genFile = this.compiler.emitTypeCheckStub(sf.fileName, baseFileName !);
+          if (genFile) {
+            this.hostAdapter.updateGeneratedFile(genFile);
           }
         }
-      });
-    }
+      }
+    });
     this._tsProgram = ts.createProgram(rootNames, this.options, this.hostAdapter, tmpProgram);
     // Note: the new ts program should be completely reusable by TypeScript as:
     // - we cache all the files in the hostAdapter
@@ -507,7 +508,13 @@ class AngularCompilerProgram implements Program {
     }
   }
 
-  private catchAnalysisError(e: any): NgAnalyzedModules|null {
+  private _createProgramOnError(e: any) {
+    // Still fill the analyzedModules and the tsProgram
+    // so that we don't cause other errors for users who e.g. want to emit the ngProgram.
+    this._analyzedModules = emptyModules;
+    this.oldTsProgram = undefined;
+    this._hostAdapter.isSourceFile = () => false;
+    this._tsProgram = ts.createProgram(this.rootNames, this.options, this.hostAdapter);
     if (isSyntaxError(e)) {
       const parserErrors = getParseErrors(e);
       if (parserErrors && parserErrors.length) {
@@ -531,7 +538,7 @@ class AngularCompilerProgram implements Program {
           }
         ];
       }
-      return null;
+      return;
     }
     throw e;
   }

packages/compiler-cli/src/transformers/util.ts

@@ -51,3 +51,29 @@ function pathStartsWithPrefix(prefix: string, fullPath: string): string|null {
   const rel = path.relative(prefix, fullPath);
   return rel.startsWith('..') ? null : rel;
 }
+
+/**
+ * Converts a ng.Diagnostic into a ts.Diagnostic.
+ * This looses some information, and also uses an incomplete object as `file`.
+ *
+ * I.e. only use this where the API allows only a ts.Diagnostic.
+ */
+export function ngToTsDiagnostic(ng: Diagnostic): ts.Diagnostic {
+  let file: ts.SourceFile|undefined;
+  let start: number|undefined;
+  let length: number|undefined;
+  if (ng.span) {
+    // Note: We can't use a real ts.SourceFile,
+    // but we can at least mirror the properties `fileName` and `text`, which
+    // are mostly used for error reporting.
+    file = { fileName: ng.span.start.file.url, text: ng.span.start.file.content } as ts.SourceFile;
+    start = ng.span.start.offset;
+    length = ng.span.end.offset - start;
+  }
+  return {
+    file,
+    messageText: ng.messageText,
+    category: ng.category,
+    code: ng.code, start, length,
+  };
+}

packages/compiler-cli/test/ngc_spec.ts

@@ -1464,8 +1464,40 @@ describe('ngc transformer command-line', () => {
       const messages: string[] = [];
       const exitCode =
           main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message));
-      expect(exitCode).toBe(2, 'Compile was expected to fail');
+      expect(exitCode).toBe(1, 'Compile was expected to fail');
       expect(messages[0]).toContain(['Tagged template expressions are not supported in metadata']);
     });
+
+    it('should allow using 2 classes with the same name in declarations with noEmitOnError=true',
+       () => {
+         write('src/tsconfig.json', `{
+        "extends": "../tsconfig-base.json",
+        "compilerOptions": {
+          "noEmitOnError": true
+        },
+        "files": ["test-module.ts"]
+      }`);
+         function writeComp(fileName: string) {
+           write(fileName, `
+        import {Component} from '@angular/core';
+
+        @Component({selector: 'comp', template: ''})
+        export class TestComponent {}
+      `);
+         }
+         writeComp('src/comp1.ts');
+         writeComp('src/comp2.ts');
+         write('src/test-module.ts', `
+        import {NgModule} from '@angular/core';
+        import {TestComponent as Comp1} from './comp1';
+        import {TestComponent as Comp2} from './comp2';
+
+        @NgModule({
+          declarations: [Comp1, Comp2],
+        })
+        export class MyModule {}
+      `);
+         expect(main(['-p', path.join(basePath, 'src/tsconfig.json')])).toBe(0);
+       });
   });
 });

packages/compiler-cli/test/perform_watch_spec.ts

@@ -105,6 +105,47 @@ describe('perform watch', () => {
     expect(getSourceFileSpy !).toHaveBeenCalledWith(mainTsPath, ts.ScriptTarget.ES5);
     expect(getSourceFileSpy !).toHaveBeenCalledWith(utilTsPath, ts.ScriptTarget.ES5);
   });
+
+  it('should recover from static analysis errors', () => {
+    const config = createConfig();
+    const host = new MockWatchHost(config);
+
+    const okFileContent = `
+      import {NgModule} from '@angular/core';
+
+      @NgModule()
+      export class MyModule {}
+    `;
+    const errorFileContent = `
+      import {NgModule} from '@angular/core';
+
+      @NgModule(() => (1===1 ? null as any : null as any))
+      export class MyModule {}
+    `;
+    const indexTsPath = path.resolve(testSupport.basePath, 'src', 'index.ts');
+
+    testSupport.write(indexTsPath, okFileContent);
+
+    performWatchCompilation(host);
+    expectNoDiagnostics(config.options, host.diagnostics);
+
+    // Do it multiple times as the watch mode switches internal modes.
+    // E.g. from regular compile to using summaries, ...
+    for (let i = 0; i < 3; i++) {
+      host.diagnostics = [];
+      testSupport.write(indexTsPath, okFileContent);
+      host.triggerFileChange(FileChangeEvent.Change, indexTsPath);
+      expectNoDiagnostics(config.options, host.diagnostics);
+
+      host.diagnostics = [];
+      testSupport.write(indexTsPath, errorFileContent);
+      host.triggerFileChange(FileChangeEvent.Change, indexTsPath);
+
+      const errDiags = host.diagnostics.filter(d => d.category === ts.DiagnosticCategory.Error);
+      expect(errDiags.length).toBe(1);
+      expect(errDiags[0].messageText).toContain('Function calls are not supported.');
+    }
+  });
 });
 
 function createModuleAndCompSource(prefix: string, template: string = prefix + 'template') {
@@ -122,7 +163,8 @@ function createModuleAndCompSource(prefix: string, template: string = prefix + '
 }
 
 class MockWatchHost {
-  timeoutListeners: Array<(() => void)|null> = [];
+  nextTimeoutListenerId = 1;
+  timeoutListeners: {[id: string]: (() => void)} = {};
   fileChangeListeners: Array<((event: FileChangeEvent, fileName: string) => void)|null> = [];
   diagnostics: ng.Diagnostics = [];
   constructor(public config: ng.ParsedConfiguration) {}
@@ -141,16 +183,16 @@ class MockWatchHost {
       close: () => this.fileChangeListeners[id] = null,
     };
   }
-  setTimeout(callback: () => void, ms: number): any {
-    const id = this.timeoutListeners.length;
-    this.timeoutListeners.push(callback);
+  setTimeout(callback: () => void): any {
+    const id = this.nextTimeoutListenerId++;
+    this.timeoutListeners[id] = callback;
     return id;
   }
-  clearTimeout(timeoutId: any): void { this.timeoutListeners[timeoutId] = null; }
+  clearTimeout(timeoutId: any): void { delete this.timeoutListeners[timeoutId]; }
   flushTimeouts() {
-    this.timeoutListeners.forEach(cb => {
-      if (cb) cb();
-    });
+    const listeners = this.timeoutListeners;
+    this.timeoutListeners = {};
+    Object.keys(listeners).forEach(id => listeners[id]());
   }
   triggerFileChange(event: FileChangeEvent, fileName: string) {
     this.fileChangeListeners.forEach(listener => {

packages/compiler-cli/test/test_support.ts

@@ -64,10 +64,10 @@ export function setup(): TestSupport {
   function write(fileName: string, content: string) {
     const dir = path.dirname(fileName);
     if (dir != '.') {
-      const newDir = path.join(basePath, dir);
+      const newDir = path.resolve(basePath, dir);
       if (!fs.existsSync(newDir)) fs.mkdirSync(newDir);
     }
-    fs.writeFileSync(path.join(basePath, fileName), content, {encoding: 'utf-8'});
+    fs.writeFileSync(path.resolve(basePath, fileName), content, {encoding: 'utf-8'});
   }
 
   function writeFiles(...mockDirs: {[fileName: string]: string}[]) {