fix(compiler): fix bazel integration and make `perform-compile` more flexible

Needed to allow custom checking for diagnostics.

integration/bazel/package.json

@@ -15,7 +15,8 @@
   "devDependencies": {
     "@angular/bazel": "file:../../dist/packages-dist/bazel",
     "@angular/compiler-cli": "file:../../dist/packages-dist/compiler-cli",
-    "typescript": "~2.3.1"
+    "typescript": "~2.3.1",
+    "@types/source-map": "0.5.1"
   },
   "scripts": {
     "postinstall": "ngc -p angular.tsconfig.json",

integration/bazel/src/BUILD.bazel

@@ -4,8 +4,8 @@ load("@angular//:index.bzl", "ng_module")
 exports_files(["tsconfig.json"])
 
 ng_module(
-    name = "app",
-    srcs = ["app.module.ts"],
+    name = "src",
+    srcs = glob(["*.ts"]),
     deps = ["//src/hello-world"],
     tsconfig = ":tsconfig.json",
-)
+)

integration/bazel/src/main.ts

@@ -0,0 +1,4 @@
+import {platformBrowser} from '@angular/platform-browser';
+import {AppModuleNgFactory} from './app.module.ngfactory';
+
+platformBrowser().bootstrapModuleFactory(AppModuleNgFactory);

packages/bazel/src/ng_module.bzl

@@ -106,13 +106,14 @@ def _compile_action(ctx, inputs, outputs, config_file_path):
   if hasattr(ctx.attr, "tsconfig") and ctx.file.tsconfig:
     action_inputs += [ctx.file.tsconfig]
 
+  arguments = ["--node_options=--expose-gc"]
   # One at-sign makes this a params-file, enabling the worker strategy.
   # Two at-signs escapes the argument so it's passed through to ngc
   # rather than the contents getting expanded.
   if ctx.attr._supports_workers:
-    arguments = ["@@" + config_file_path]
+    arguments += ["@@" + config_file_path]
   else:
-    arguments = ["-p", config_file_path]
+    arguments += ["-p", config_file_path]
 
   ctx.action(
       progress_message = "Compiling Angular templates (ngc) %s" % ctx.label,

packages/bazel/src/ngc-wrapped/BUILD.bazel

@@ -22,7 +22,6 @@ nodejs_binary(
     # Entry point assumes the user is outside this WORKSPACE,
     # and references our rules with @angular//src/ngc-wrapped
     entry_point = "angular/src/ngc-wrapped/index.js",
-    args = ["--node_options=--expose-gc"],
     data = [
         ":ngc_lib",
         "@build_bazel_rules_typescript//internal:worker_protocol.proto"

packages/bazel/src/ngc-wrapped/index.ts

@@ -6,18 +6,17 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as ng from '@angular/compiler-cli';
-import {CachedFileLoader, CompilerHost, FileCache, FileLoader, UncachedFileLoader, constructManifest, debug, parseTsconfig, runAsWorker, runWorkerLoop} from '@bazel/typescript';
+import {BazelOptions, CachedFileLoader, CompilerHost, FileCache, FileLoader, UncachedFileLoader, constructManifest, debug, parseTsconfig, runAsWorker, runWorkerLoop} from '@bazel/typescript';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as tsickle from 'tsickle';
 import * as ts from 'typescript';
 
 const EXT = /(\.ts|\.d\.ts|\.js|\.jsx|\.tsx)$/;
+const NGC_GEN_FILES = /^(.*?)\.(ngfactory|ngsummary|ngstyle|shim\.ngstyle)(.*)$/;
 // FIXME: we should be able to add the assets to the tsconfig so FileLoader
 // knows about them
-const NGC_NON_TS_INPUTS =
-    /(\.(ngsummary|ngstyle|ngfactory)(\.d)?\.ts|\.ngsummary\.json|\.css|\.html)$/;
-// FIXME should need only summary, css, html
+const NGC_ASSETS = /\.(css|html|ngsummary\.json)$/;
 
 // TODO(alexeagle): probably not needed, see
 // https://github.com/bazelbuild/rules_typescript/issues/28
@@ -42,54 +41,72 @@ function runOneBuild(args: string[], inputs?: {[path: string]: string}): boolean
   let fileLoader: FileLoader;
   if (inputs) {
     fileLoader = new CachedFileLoader(fileCache, ALLOW_NON_HERMETIC_READS);
-    fileCache.updateCache(inputs);
+    // Resolve the inputs to absolute paths to match TypeScript internals
+    const resolvedInputs: {[path: string]: string} = {};
+    for (const key of Object.keys(inputs)) {
+      resolvedInputs[path.resolve(key)] = inputs[key];
+    }
+    fileCache.updateCache(resolvedInputs);
   } else {
     fileLoader = new UncachedFileLoader();
   }
   const [{options: tsOptions, bazelOpts, files, config}] = parseTsconfig(project);
+  const expectedOuts = config['angularCompilerOptions']['expectedOut'];
 
   const {basePath} = ng.calcProjectFileAndBasePath(project);
-  const ngOptions = ng.createNgCompilerOptions(basePath, config, tsOptions);
+  const compilerOpts = ng.createNgCompilerOptions(basePath, config, tsOptions);
+  const {diagnostics} = compile({fileLoader, compilerOpts, bazelOpts, files, expectedOuts});
+  return diagnostics.every(d => d.category !== ts.DiagnosticCategory.Error);
+}
+
+export function relativeToRootDirs(filePath: string, rootDirs: string[]): string {
+  if (!filePath) return filePath;
+  // NB: the rootDirs should have been sorted longest-first
+  for (const dir of rootDirs || []) {
+    const rel = path.relative(dir, filePath);
+    if (rel.indexOf('.') != 0) return rel;
+  }
+  return filePath;
+}
+
+export function compile(
+    {fileLoader, compilerOpts, bazelOpts, files, expectedOuts, gatherDiagnostics}: {
+      fileLoader: FileLoader,
+      compilerOpts: ng.CompilerOptions,
+      bazelOpts: BazelOptions,
+      files: string[],
+      expectedOuts: string[], gatherDiagnostics?: (program: ng.Program) => ng.Diagnostics
+    }): {diagnostics: ng.Diagnostics, program: ng.Program} {
   if (!bazelOpts.es5Mode) {
-    ngOptions.annotateForClosureCompiler = true;
-    ngOptions.annotationsAs = 'static fields';
+    compilerOpts.annotateForClosureCompiler = true;
+    compilerOpts.annotationsAs = 'static fields';
   }
 
-  if (!tsOptions.rootDirs) {
+  if (!compilerOpts.rootDirs) {
     throw new Error('rootDirs is not set!');
   }
 
-  function relativeToRootDirs(filePath: string, rootDirs: string[]): string {
-    if (!filePath) return filePath;
-    // NB: the rootDirs should have been sorted longest-first
-    for (const dir of rootDirs || []) {
-      const rel = path.relative(dir, filePath);
-      if (rel.indexOf('.') !== 0) return rel;
-    }
-    return filePath;
-  }
-  const expectedOuts = [...config['angularCompilerOptions']['expectedOut']];
-  const tsHost = ts.createCompilerHost(tsOptions, true);
+  const writtenExpectedOuts = [...expectedOuts];
+  const tsHost = ts.createCompilerHost(compilerOpts, true);
 
   const originalWriteFile = tsHost.writeFile.bind(tsHost);
   tsHost.writeFile =
       (fileName: string, content: string, writeByteOrderMark: boolean,
        onError?: (message: string) => void, sourceFiles?: ts.SourceFile[]) => {
-        const relative = relativeToRootDirs(fileName, [tsOptions.rootDir]);
-        const expectedIdx = expectedOuts.findIndex(o => o === relative);
+        const relative = relativeToRootDirs(fileName, [compilerOpts.rootDir]);
+        const expectedIdx = writtenExpectedOuts.findIndex(o => o === relative);
         if (expectedIdx >= 0) {
-          expectedOuts.splice(expectedIdx, 1);
+          writtenExpectedOuts.splice(expectedIdx, 1);
           originalWriteFile(fileName, content, writeByteOrderMark, onError, sourceFiles);
         }
       };
 
-
   // Patch fileExists when resolving modules, so that ngc can ask TypeScript to
   // resolve non-existing generated files that don't exist on disk, but are
   // synthetic and added to the `programWithStubs` based on real inputs.
   const generatedFileModuleResolverHost = Object.create(tsHost);
   generatedFileModuleResolverHost.fileExists = (fileName: string) => {
-    const match = /^(.*?)\.(ngfactory|ngsummary|ngstyle|shim\.ngstyle)(.*)$/.exec(fileName);
+    const match = NGC_GEN_FILES.exec(fileName);
     if (match) {
       const [, file, suffix, ext] = match;
       // Performance: skip looking for files other than .d.ts or .ts
@@ -112,22 +129,29 @@ function runOneBuild(args: string[], inputs?: {[path: string]: string}): boolean
         moduleName, containingFile, compilerOptions, generatedFileModuleResolverHost);
   }
 
+  // TODO(alexeagle): does this also work in third_party?
+  const allowNonHermeticRead = false;
   const bazelHost = new CompilerHost(
-      files, tsOptions, bazelOpts, tsHost, fileLoader, ALLOW_NON_HERMETIC_READS,
+      files, compilerOpts, bazelOpts, tsHost, fileLoader, ALLOW_NON_HERMETIC_READS,
       generatedFileModuleResolver);
-  // The file cache is populated by Bazel with workspace-relative filenames
-  // so we must relativize paths before looking them up in the cache.
-  const originalGetSourceFile = bazelHost.getSourceFile.bind(bazelHost);
-  bazelHost.getSourceFile = (fileName: string, languageVersion: ts.ScriptTarget) => {
-    return originalGetSourceFile(relativeToRootDirs(fileName, [tsOptions.rootDir]));
+  const origBazelHostFileExist = bazelHost.fileExists;
+  bazelHost.fileExists = (fileName: string) => {
+    if (NGC_ASSETS.test(fileName)) {
+      return tsHost.fileExists(fileName);
+    }
+    return origBazelHostFileExist.call(bazelHost, fileName);
   };
+  // TODO(tbosch): fix tsickle to still run regular transformers even
+  // if tsickle is not processing a file, and then remove this override,
+  // as this is only required to keep the ng transformer running,
+  // but produces e.g. too many externs.
   bazelHost.shouldSkipTsickleProcessing = (fileName: string): boolean =>
-      bazelOpts.compilationTargetSrc.indexOf(fileName) === -1 && !NGC_NON_TS_INPUTS.test(fileName);
+      bazelOpts.compilationTargetSrc.indexOf(fileName) === -1 && !NGC_GEN_FILES.test(fileName);
 
-  const ngHost = ng.createCompilerHost({options: ngOptions, tsHost: bazelHost});
+  const ngHost = ng.createCompilerHost({options: compilerOpts, tsHost: bazelHost});
 
   ngHost.fileNameToModuleName = (importedFilePath: string, containingFilePath: string) =>
-      relativeToRootDirs(importedFilePath, tsOptions.rootDirs).replace(EXT, '');
+      relativeToRootDirs(importedFilePath, compilerOpts.rootDirs).replace(EXT, '');
   ngHost.toSummaryFileName = (fileName: string, referringSrcFileName: string) =>
       ngHost.fileNameToModuleName(fileName, referringSrcFileName);
 
@@ -149,18 +173,18 @@ function runOneBuild(args: string[], inputs?: {[path: string]: string}): boolean
     customTransformers = {},
   }) =>
       tsickle.emitWithTsickle(
-          program, bazelHost, tsickleOpts, bazelHost, ngOptions, targetSourceFile, writeFile,
+          program, bazelHost, tsickleOpts, bazelHost, compilerOpts, targetSourceFile, writeFile,
           cancellationToken, emitOnlyDtsFiles, {
             beforeTs: customTransformers.before,
             afterTs: customTransformers.after,
           });
 
-  const {diagnostics, emitResult} =
-      ng.performCompilation({rootNames: files, options: ngOptions, host: ngHost, emitCallback});
+  const {diagnostics, emitResult, program} = ng.performCompilation(
+      {rootNames: files, options: compilerOpts, host: ngHost, emitCallback, gatherDiagnostics});
   const tsickleEmitResult = emitResult as tsickle.EmitResult;
   let externs = '/** @externs */\n';
   if (diagnostics.length) {
-    console.error(ng.formatDiagnostics(ngOptions, diagnostics));
+    console.error(ng.formatDiagnostics(compilerOpts, diagnostics));
   } else {
     if (bazelOpts.tsickleGenerateExterns) {
       externs += tsickle.getGeneratedExterns(tsickleEmitResult.externs);
@@ -178,11 +202,11 @@ function runOneBuild(args: string[], inputs?: {[path: string]: string}): boolean
     fs.writeFileSync(bazelOpts.tsickleExternsPath, externs);
   }
 
-  for (const missing of expectedOuts) {
+  for (const missing of writtenExpectedOuts) {
     originalWriteFile(missing, '', false);
   }
 
-  return diagnostics.every(d => d.category !== ts.DiagnosticCategory.Error);
+  return {program, diagnostics};
 }
 
 if (require.main === module) {

packages/compiler-cli/index.ts

@@ -20,7 +20,7 @@ export {BuiltinType, DeclarationKind, Definition, PipeInfo, Pipes, Signature, Sp
 export * from './src/transformers/api';
 export * from './src/transformers/entry_points';
 
-export {performCompilation, readConfiguration, formatDiagnostics, calcProjectFileAndBasePath, createNgCompilerOptions} from './src/perform_compile';
+export * from './src/perform_compile';
 
 // TODO(hansl): moving to Angular 4 need to update this API.
 export {NgTools_InternalApi_NG_2 as __NGTOOLS_PRIVATE_API_2} from './src/ngtools_api';

packages/compiler-cli/src/perform_compile.ts

@@ -129,33 +129,26 @@ export function exitCodeFromResult(result: PerformCompilationResult | undefined)
       1;
 }
 
-export function performCompilation(
-    {rootNames, options, host, oldProgram, emitCallback, customTransformers}: {
-      rootNames: string[],
-      options: api.CompilerOptions,
-      host?: api.CompilerHost,
-      oldProgram?: api.Program,
-      emitCallback?: api.TsEmitCallback,
-      customTransformers?: api.CustomTransformers
-    }): PerformCompilationResult {
+export function performCompilation({rootNames, options, host, oldProgram, emitCallback,
+                                    gatherDiagnostics = defaultGatherDiagnostics,
+                                    customTransformers}: {
+  rootNames: string[],
+  options: api.CompilerOptions,
+  host?: api.CompilerHost,
+  oldProgram?: api.Program,
+  emitCallback?: api.TsEmitCallback,
+  gatherDiagnostics?: (program: api.Program) => Diagnostics,
+  customTransformers?: api.CustomTransformers
+}): PerformCompilationResult {
   const [major, minor] = ts.version.split('.');
 
   if (Number(major) < 2 || (Number(major) === 2 && Number(minor) < 3)) {
     throw new Error('Must use TypeScript > 2.3 to have transformer support');
   }
 
-  const allDiagnostics: Diagnostics = [];
-
-  function checkDiagnostics(diags: Diagnostics | undefined) {
-    if (diags) {
-      allDiagnostics.push(...diags);
-      return diags.every(d => d.category !== ts.DiagnosticCategory.Error);
-    }
-    return true;
-  }
-
   let program: api.Program|undefined;
   let emitResult: ts.EmitResult|undefined;
+  let allDiagnostics: Diagnostics = [];
   try {
     if (!host) {
       host = ng.createCompilerHost({options});
@@ -163,25 +156,9 @@ export function performCompilation(
 
     program = ng.createProgram({rootNames, host, options, oldProgram});
 
-    let shouldEmit = true;
-    // Check parameter diagnostics
-    shouldEmit = shouldEmit && checkDiagnostics([
-                   ...program !.getTsOptionDiagnostics(), ...program !.getNgOptionDiagnostics()
-                 ]);
+    allDiagnostics.push(...gatherDiagnostics(program !));
 
-    // Check syntactic diagnostics
-    shouldEmit = shouldEmit && checkDiagnostics(program !.getTsSyntacticDiagnostics());
-
-    // Check TypeScript semantic and Angular structure diagnostics
-    shouldEmit =
-        shouldEmit &&
-        checkDiagnostics(
-            [...program !.getTsSemanticDiagnostics(), ...program !.getNgStructuralDiagnostics()]);
-
-    // Check Angular semantic diagnostics
-    shouldEmit = shouldEmit && checkDiagnostics(program !.getNgSemanticDiagnostics());
-
-    if (shouldEmit) {
+    if (!hasErrors(allDiagnostics)) {
       emitResult = program !.emit({
         emitCallback,
         customTransformers,
@@ -209,4 +186,41 @@ export function performCompilation(
         {category: ts.DiagnosticCategory.Error, messageText: errMsg, code, source: api.SOURCE});
     return {diagnostics: allDiagnostics, program};
   }
+}
+
+function defaultGatherDiagnostics(program: api.Program): Diagnostics {
+  const allDiagnostics: Diagnostics = [];
+
+  function checkDiagnostics(diags: Diagnostics | undefined) {
+    if (diags) {
+      allDiagnostics.push(...diags);
+      return !hasErrors(diags);
+    }
+    return true;
+  }
+
+  let checkOtherDiagnostics = true;
+  // Check parameter diagnostics
+  checkOtherDiagnostics = checkOtherDiagnostics &&
+      checkDiagnostics([...program.getTsOptionDiagnostics(), ...program.getNgOptionDiagnostics()]);
+
+  // Check syntactic diagnostics
+  checkOtherDiagnostics =
+      checkOtherDiagnostics && checkDiagnostics(program.getTsSyntacticDiagnostics());
+
+  // Check TypeScript semantic and Angular structure diagnostics
+  checkOtherDiagnostics =
+      checkOtherDiagnostics &&
+      checkDiagnostics(
+          [...program.getTsSemanticDiagnostics(), ...program.getNgStructuralDiagnostics()]);
+
+  // Check Angular semantic diagnostics
+  checkOtherDiagnostics =
+      checkOtherDiagnostics && checkDiagnostics(program.getNgSemanticDiagnostics());
+
+  return allDiagnostics;
+}
+
+function hasErrors(diags: Diagnostics) {
+  return diags.some(d => d.category === ts.DiagnosticCategory.Error);
 }