refactor(compiler-cli): efficient single-file type checking diagnostics (angular#38105)

Previously, the `TemplateTypeChecker` abstraction allowed fetching
diagnostics for a single file, but under the hood would generate type
checking code for the entire program to satisfy the request.

With this commit, an `OptimizeFor` hint is passed to `getDiagnosticsForFile`
which indicates whether the user intends to request diagnostics for the
whole program or is truly interested in just the single file. If the latter,
the `TemplateTypeChecker` can perform only the work needed to produce
diagnostics for just that file, thus returning answers more efficiently.

PR Close angular#38105

Author: alxhub

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

@@ -29,7 +29,7 @@ import {generatedFactoryTransform} from '../../shims';
 import {ivySwitchTransform} from '../../switch';
 import {aliasTransformFactory, declarationTransformFactory, DecoratorHandler, DtsTransformRegistry, ivyTransformFactory, TraitCompiler} from '../../transform';
 import {isTemplateDiagnostic, TemplateTypeCheckerImpl} from '../../typecheck';
-import {TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy} from '../../typecheck/api';
+import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy} from '../../typecheck/api';
 import {getSourceFileOrNull, isDtsPath, resolveModuleName} from '../../util/src/typescript';
 import {LazyRoute, NgCompilerAdapter, NgCompilerOptions} from '../api';
 
@@ -507,7 +507,8 @@ export class NgCompiler {
         continue;
       }
 
-      diagnostics.push(...compilation.templateTypeChecker.getDiagnosticsForFile(sf));
+      diagnostics.push(
+          ...compilation.templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram));
     }
 
     const program = this.typeCheckingProgramStrategy.getProgram();

packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts

@@ -29,8 +29,16 @@ export interface TemplateTypeChecker {
    *
    * This method will fail (throw) if there are components within the `ts.SourceFile` that do not
    * have TCBs available.
+   *
+   * Generating a template type-checking program is expensive, and in some workflows (e.g. checking
+   * an entire program before emit), it should ideally only be done once. The `optimizeFor` flag
+   * allows the caller to hint to `getDiagnosticsForFile` (which internally will create a template
+   * type-checking program if needed) whether the caller is interested in just the results of the
+   * single file, or whether they plan to query about other files in the program. Based on this
+   * flag, `getDiagnosticsForFile` will determine how much of the user's program to prepare for
+   * checking as part of the template type-checking program it creates.
    */
-  getDiagnosticsForFile(sf: ts.SourceFile): ts.Diagnostic[];
+  getDiagnosticsForFile(sf: ts.SourceFile, optimizeFor: OptimizeFor): ts.Diagnostic[];
 
   /**
    * Retrieve the top-level node representing the TCB for the given component.
@@ -41,3 +49,27 @@ export interface TemplateTypeChecker {
    */
   getTypeCheckBlock(component: ts.ClassDeclaration): ts.Node|null;
 }
+
+/**
+ * Describes the scope of the caller's interest in template type-checking results.
+ */
+export enum OptimizeFor {
+  /**
+   * Indicates that a consumer of a `TemplateTypeChecker` is only interested in results for a given
+   * file, and wants them as fast as possible.
+   *
+   * Calling `TemplateTypeChecker` methods successively for multiple files while specifying
+   * `OptimizeFor.SingleFile` can result in significant unnecessary overhead overall.
+   */
+  SingleFile,
+
+  /**
+   * Indicates that a consumer of a `TemplateTypeChecker` intends to query for results pertaining to
+   * the entire user program, and so the type-checker should internally optimize for this case.
+   *
+   * Initial calls to retrieve type-checking information may take longer, but repeated calls to
+   * gather information for the whole user program will be significantly faster with this mode of
+   * optimization.
+   */
+  WholeProgram,
+}

packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts

@@ -14,7 +14,7 @@ import {IncrementalBuild} from '../../incremental/api';
 import {ReflectionHost} from '../../reflection';
 import {isShim} from '../../shims';
 import {getSourceFileOrNull} from '../../util/src/typescript';
-import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
+import {OptimizeFor, ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
 
 import {InliningMode, ShimTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from './context';
 import {findTypeCheckBlock, shouldReportDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics';
@@ -41,8 +41,15 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
    * Retrieve type-checking diagnostics from the given `ts.SourceFile` using the most recent
    * type-checking program.
    */
-  getDiagnosticsForFile(sf: ts.SourceFile): ts.Diagnostic[] {
-    this.ensureAllShimsForAllFiles();
+  getDiagnosticsForFile(sf: ts.SourceFile, optimizeFor: OptimizeFor): ts.Diagnostic[] {
+    switch (optimizeFor) {
+      case OptimizeFor.WholeProgram:
+        this.ensureAllShimsForAllFiles();
+        break;
+      case OptimizeFor.SingleFile:
+        this.ensureAllShimsForOneFile(sf);
+        break;
+    }
 
     const sfPath = absoluteFromSourceFile(sf);
     const fileRecord = this.state.get(sfPath)!;
@@ -68,7 +75,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
   }
 
   getTypeCheckBlock(component: ts.ClassDeclaration): ts.Node|null {
-    this.ensureAllShimsForAllFiles();
+    this.ensureAllShimsForOneFile(component.getSourceFile());
 
     const program = this.typeCheckingStrategy.getProgram();
     const filePath = absoluteFromSourceFile(component.getSourceFile());
@@ -81,7 +88,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
     const id = fileRecord.sourceManager.getTemplateId(component);
 
     const shimSf = getSourceFileOrNull(program, shimPath);
-    if (shimSf === null) {
+    if (shimSf === null || !fileRecord.shimData.has(shimPath)) {
       throw new Error(`Error: no shim file in program: ${shimPath}`);
     }
 
@@ -144,6 +151,27 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
     this.isComplete = true;
   }
 
+  private ensureAllShimsForOneFile(sf: ts.SourceFile): void {
+    this.maybeAdoptPriorResultsForFile(sf);
+
+    const sfPath = absoluteFromSourceFile(sf);
+
+    const fileData = this.getFileData(sfPath);
+    if (fileData.isComplete) {
+      // All data for this file is present and accounted for already.
+      return;
+    }
+
+    const host = new SingleFileTypeCheckingHost(sfPath, fileData, this.typeCheckingStrategy, this);
+    const ctx = this.newContext(host);
+
+    this.typeCheckAdapter.typeCheck(sf, ctx);
+
+    fileData.isComplete = true;
+
+    this.updateFromContext(ctx);
+  }
+
   private newContext(host: TypeCheckingHost): TypeCheckContextImpl {
     const inlining = this.typeCheckingStrategy.supportsInlineOperations ? InliningMode.InlineOps :
                                                                           InliningMode.Error;
@@ -152,6 +180,30 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
         host, inlining);
   }
 
+  /**
+   * Remove any shim data that depends on inline operations applied to the type-checking program.
+   *
+   * This can be useful if new inlines need to be applied, and it's not possible to guarantee that
+   * they won't overwrite or corrupt existing inlines that are used by such shims.
+   */
+  clearAllShimDataUsingInlines(): void {
+    for (const fileData of this.state.values()) {
+      if (!fileData.hasInlines) {
+        continue;
+      }
+
+      for (const [shimFile, shimData] of fileData.shimData.entries()) {
+        if (shimData.hasInlines) {
+          fileData.shimData.delete(shimFile);
+        }
+      }
+
+      fileData.hasInlines = false;
+      fileData.isComplete = false;
+      this.isComplete = false;
+    }
+  }
+
   private updateFromContext(ctx: TypeCheckContextImpl): void {
     const updates = ctx.finalize();
     this.typeCheckingStrategy.updateFiles(updates, UpdateMode.Incremental);
@@ -240,3 +292,61 @@ class WholeProgramTypeCheckingHost implements TypeCheckingHost {
     this.impl.getFileData(sfPath).isComplete = true;
   }
 }
+
+/**
+ * Drives a `TypeCheckContext` to generate type-checking code efficiently for a single input file.
+ */
+class SingleFileTypeCheckingHost implements TypeCheckingHost {
+  private seenInlines = false;
+
+  constructor(
+      private sfPath: AbsoluteFsPath, private fileData: FileTypeCheckingData,
+      private strategy: TypeCheckingProgramStrategy, private impl: TemplateTypeCheckerImpl) {}
+
+  private assertPath(sfPath: AbsoluteFsPath): void {
+    if (this.sfPath !== sfPath) {
+      throw new Error(`AssertionError: querying TypeCheckingHost outside of assigned file`);
+    }
+  }
+
+  getSourceManager(sfPath: AbsoluteFsPath): TemplateSourceManager {
+    this.assertPath(sfPath);
+    return this.fileData.sourceManager;
+  }
+
+  shouldCheckComponent(node: ts.ClassDeclaration): boolean {
+    if (this.sfPath !== absoluteFromSourceFile(node.getSourceFile())) {
+      return false;
+    }
+    const shimPath = this.strategy.shimPathForComponent(node);
+
+    // Only need to generate a TCB for the class if no shim exists for it currently.
+    return !this.fileData.shimData.has(shimPath);
+  }
+
+  recordShimData(sfPath: AbsoluteFsPath, data: ShimTypeCheckingData): void {
+    this.assertPath(sfPath);
+
+    // Previous type-checking state may have required the use of inlines (assuming they were
+    // supported). If the current operation also requires inlines, this presents a problem:
+    // generating new inlines may invalidate any old inlines that old state depends on.
+    //
+    // Rather than resolve this issue by tracking specific dependencies on inlines, if the new state
+    // relies on inlines, any old state that relied on them is simply cleared. This happens when the
+    // first new state that uses inlines is encountered.
+    if (data.hasInlines && !this.seenInlines) {
+      this.impl.clearAllShimDataUsingInlines();
+      this.seenInlines = true;
+    }
+
+    this.fileData.shimData.set(data.path, data);
+    if (data.hasInlines) {
+      this.fileData.hasInlines = true;
+    }
+  }
+
+  recordComplete(sfPath: AbsoluteFsPath): void {
+    this.assertPath(sfPath);
+    this.fileData.isComplete = true;
+  }
+}

packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts

@@ -10,7 +10,7 @@ import * as ts from 'typescript';
 
 import {absoluteFrom, getSourceFileOrError} from '../../file_system';
 import {runInEachFileSystem, TestFile} from '../../file_system/testing';
-import {TypeCheckingConfig} from '../api';
+import {OptimizeFor, TypeCheckingConfig} from '../api';
 
 import {ngForDeclaration, ngForDts, setup, TestDeclaration} from './test_utils';
 
@@ -472,7 +472,7 @@ function diagnose(
       ],
       {config, options});
   const sf = getSourceFileOrError(program, sfPath);
-  const diagnostics = templateTypeChecker.getDiagnosticsForFile(sf);
+  const diagnostics = templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram);
   return diagnostics.map(diag => {
     const text =
         typeof diag.messageText === 'string' ? diag.messageText : diag.messageText.messageText;

packages/compiler-cli/src/ngtsc/typecheck/test/program_spec.ts

@@ -12,7 +12,7 @@ import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError} from '../../file_sys
 import {runInEachFileSystem} from '../../file_system/testing';
 import {sfExtensionData, ShimReferenceTagger} from '../../shims';
 import {expectCompleteReuse, makeProgram} from '../../testing';
-import {UpdateMode} from '../api';
+import {OptimizeFor, UpdateMode} from '../api';
 import {ReusedProgramStrategy} from '../src/augmented_program';
 
 import {setup} from './test_utils';
@@ -28,7 +28,7 @@ runInEachFileSystem(() => {
       }]);
       const sf = getSourceFileOrError(program, fileName);
 
-      templateTypeChecker.getDiagnosticsForFile(sf);
+      templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram);
       // expect() here would create a really long error message, so this is checked manually.
       if (programStrategy.getProgram() !== program) {
         fail('Template type-checking created a new ts.Program even though it had no changes.');