refactor(compiler-cli): allow program strategies to opt out of inlini…

…ng (angular#38105)

The template type-checking engine relies on the abstraction interface
`TypeCheckingProgramStrategy` to create updated `ts.Program`s for
template type-checking. The basic API is that the type-checking engine
requests changes to certain files in the program, and the strategy provides
an updated `ts.Program`.

Typically, such changes are made to 'ngtypecheck' shim files, but certain
conditions can cause template type-checking to require "inline" operations,
which change user .ts files instead. The strategy used by 'ngc' (the
`ReusedProgramStrategy`) supports these kinds of updates, but other clients
such as the language service might not always support modifying user files.

To accommodate this, the `TypeCheckingProgramStrategy` interface was
modified to include a `supportsInlineOperations` flag. If an implementation
specifies `false` for inline support, the template type-checking system will
return diagnostics on components which would otherwise require inline
operations.

Closes angular#38059

PR Close angular#38105

goldens/public-api/compiler-cli/error_code.d.ts

@@ -32,5 +32,7 @@ export declare enum ErrorCode {
     MISSING_PIPE = 8004,
     WRITE_TO_READ_ONLY_VARIABLE = 8005,
     DUPLICATE_VARIABLE_DECLARATION = 8006,
+    INLINE_TCB_REQUIRED = 8900,
+    INLINE_TYPE_CTOR_REQUIRED = 8901,
     INJECTABLE_DUPLICATE_PROV = 9001
 }

packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts

@@ -138,6 +138,18 @@ export enum ErrorCode {
    */
   DUPLICATE_VARIABLE_DECLARATION = 8006,
 
+  /**
+   * The template type-checking engine would need to generate an inline type check block for a
+   * component, but the current type-checking environment doesn't support it.
+   */
+  INLINE_TCB_REQUIRED = 8900,
+
+  /**
+   * The template type-checking engine would need to generate an inline type constructor for a
+   * directive or component, but the current type-checking environment doesn't support it.
+   */
+  INLINE_TYPE_CTOR_REQUIRED = 8901,
+
   /**
    * An injectable already has a `ɵprov` property.
    */

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

@@ -304,6 +304,12 @@ export interface ComponentToShimMappingStrategy {
  * implement efficient template type-checking using common infrastructure.
  */
 export interface TypeCheckingProgramStrategy extends ComponentToShimMappingStrategy {
+  /**
+   * Whether this strategy supports modifying user files (inline modifications) in addition to
+   * modifying type-checking shims.
+   */
+  readonly supportsInlineOperations: boolean;
+
   /**
    * Retrieve the latest version of the program, containing all the updates made thus far.
    */

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

@@ -34,6 +34,8 @@ export class ReusedProgramStrategy implements TypeCheckingProgramStrategy {
       private originalProgram: ts.Program, private originalHost: ts.CompilerHost,
       private options: ts.CompilerOptions, private shimExtensionPrefixes: string[]) {}
 
+  readonly supportsInlineOperations = true;
+
   getProgram(): ts.Program {
     return this.program;
   }

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

@@ -16,7 +16,7 @@ import {isShim} from '../../shims';
 import {getSourceFileOrNull} from '../../util/src/typescript';
 import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
 
-import {ShimTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from './context';
+import {InliningMode, ShimTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from './context';
 import {findTypeCheckBlock, shouldReportDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics';
 import {TemplateSourceManager} from './source';
 
@@ -145,9 +145,11 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
   }
 
   private newContext(host: TypeCheckingHost): TypeCheckContextImpl {
+    const inlining = this.typeCheckingStrategy.supportsInlineOperations ? InliningMode.InlineOps :
+                                                                          InliningMode.Error;
     return new TypeCheckContextImpl(
         this.config, this.compilerHost, this.typeCheckingStrategy, this.refEmitter, this.reflector,
-        host);
+        host, inlining);
   }
 
   private updateFromContext(ctx: TypeCheckContextImpl): void {

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

@@ -114,6 +114,20 @@ export interface TypeCheckingHost {
   recordComplete(sfPath: AbsoluteFsPath): void;
 }
 
+/**
+ * How a type-checking context should handle operations which would require inlining.
+ */
+export enum InliningMode {
+  /**
+   * Use inlining operations when required.
+   */
+  InlineOps,
+
+  /**
+   * Produce diagnostics if an operation would require inlining.
+   */
+  Error,
+}
 
 /**
  * A template type checking context for a program.
@@ -129,7 +143,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
       private compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
       private componentMappingStrategy: ComponentToShimMappingStrategy,
       private refEmitter: ReferenceEmitter, private reflector: ReflectionHost,
-      private host: TypeCheckingHost) {}
+      private host: TypeCheckingHost, private inlining: InliningMode) {}
 
   /**
    * A `Map` of `ts.SourceFile`s that the context has seen to the operations (additions of methods
@@ -161,6 +175,10 @@ export class TypeCheckContextImpl implements TypeCheckContext {
       return;
     }
 
+    // Accumulate a list of any directives which could not have type constructors generated due to
+    // unsupported inlining operations.
+    let missingInlines: ClassDeclaration[] = [];
+
     const fileData = this.dataForFile(ref.node.getSourceFile());
     const shimData = this.pendingShimForComponent(ref.node);
     const boundTarget = binder.bind({template});
@@ -169,6 +187,11 @@ export class TypeCheckContextImpl implements TypeCheckContext {
       const dirRef = dir.ref as Reference<ClassDeclaration<ts.ClassDeclaration>>;
       const dirNode = dirRef.node;
       if (requiresInlineTypeCtor(dirNode, this.reflector)) {
+        if (this.inlining === InliningMode.Error) {
+          missingInlines.push(dirNode);
+          continue;
+        }
+
         // Add a type constructor operation for the directive.
         this.addInlineTypeCtor(fileData, dirNode.getSourceFile(), dirRef, {
           fnName: 'ngTypeCtor',
@@ -186,13 +209,34 @@ export class TypeCheckContextImpl implements TypeCheckContext {
       }
     }
 
+    const tcbRequiresInline = requiresInlineTypeCheckBlock(ref.node);
+
+    // If inlining is not supported, but is required for either the TCB or one of its directive
+    // dependencies, then exit here with an error.
+    if (this.inlining === InliningMode.Error && (tcbRequiresInline || missingInlines.length > 0)) {
+      // This template cannot be supported because the underlying strategy does not support inlining
+      // and inlining would be required.
+
+      // Record diagnostics to indicate the issues with this template.
+      if (tcbRequiresInline) {
+        shimData.oobRecorder.requiresInlineTcb(ref.node);
+      }
+
+      if (missingInlines.length > 0) {
+        shimData.oobRecorder.requiresInlineTypeConstructors(ref.node, missingInlines);
+      }
+
+      // Checking this template would be unsupported, so don't try.
+      return;
+    }
+
     const meta = {
       id: fileData.sourceManager.captureSource(ref.node, sourceMapping, file),
       boundTarget,
       pipes,
       schemas,
     };
-    if (requiresInlineTypeCheckBlock(ref.node)) {
+    if (tcbRequiresInline) {
       // This class didn't meet the requirements for external type checking, so generate an inline
       // TCB for the class.
       this.addInlineTypeCheckBlock(fileData, shimData, ref, meta);

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

@@ -9,7 +9,8 @@
 import {BindingPipe, PropertyWrite, TmplAstReference, TmplAstVariable} from '@angular/compiler';
 import * as ts from 'typescript';
 
-import {ErrorCode, ngErrorCode} from '../../diagnostics';
+import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
+import {ClassDeclaration} from '../../reflection';
 import {TemplateId} from '../api';
 
 import {makeTemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
@@ -61,6 +62,10 @@ export interface OutOfBandDiagnosticRecorder {
    */
   duplicateTemplateVar(
       templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void;
+
+  requiresInlineTcb(node: ClassDeclaration): void;
+
+  requiresInlineTypeConstructors(node: ClassDeclaration, directives: ClassDeclaration[]): void;
 }
 
 export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
@@ -133,4 +138,26 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
           span: firstDecl.sourceSpan,
         }));
   }
+
+  requiresInlineTcb(node: ClassDeclaration): void {
+    this._diagnostics.push(makeDiagnostic(
+        ErrorCode.INLINE_TCB_REQUIRED, node.name,
+        `This component requires inline template type-checking, which is not supported by the current environment.`));
+  }
+
+  requiresInlineTypeConstructors(node: ClassDeclaration, directives: ClassDeclaration[]): void {
+    let message: string;
+    if (directives.length > 1) {
+      message =
+          `This component uses directives which require inline type constructors, which are not supported by the current environment.`;
+    } else {
+      message =
+          `This component uses a directive which requires an inline type constructor, which is not supported by the current environment.`;
+    }
+
+    this._diagnostics.push(makeDiagnostic(
+        ErrorCode.INLINE_TYPE_CTOR_REQUIRED, node.name, message,
+        directives.map(
+            dir => makeRelatedInformation(dir.name, `Requires an inline type constructor.`))));
+  }
 }

packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel

@@ -11,6 +11,7 @@ ts_library(
     deps = [
         "//packages:types",
         "//packages/compiler",
+        "//packages/compiler-cli/src/ngtsc/diagnostics",
         "//packages/compiler-cli/src/ngtsc/file_system",
         "//packages/compiler-cli/src/ngtsc/file_system/testing",
         "//packages/compiler-cli/src/ngtsc/imports",

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

@@ -17,7 +17,6 @@ import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} fro
 import {makeProgram} from '../../testing';
 import {getRootDirs} from '../../util/src/typescript';
 import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckContext} from '../api';
-
 import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckingConfig, UpdateMode} from '../api/api';
 import {ReusedProgramStrategy} from '../src/augmented_program';
 import {TemplateTypeCheckerImpl} from '../src/checker';
@@ -278,6 +277,7 @@ export interface TypeCheckingTarget {
 export function setup(targets: TypeCheckingTarget[], overrides: {
   config?: Partial<TypeCheckingConfig>,
   options?: ts.CompilerOptions,
+  inlining?: boolean,
 } = {}): {
   templateTypeChecker: TemplateTypeChecker,
   program: ts.Program,
@@ -378,6 +378,10 @@ export function setup(targets: TypeCheckingTarget[], overrides: {
   });
 
   const programStrategy = new ReusedProgramStrategy(program, host, options, ['ngtypecheck']);
+  if (overrides.inlining !== undefined) {
+    (programStrategy as any).supportsInlineOperations = overrides.inlining;
+  }
+
   const templateTypeChecker = new TemplateTypeCheckerImpl(
       program, programStrategy, checkAdapter, fullConfig, emitter, reflectionHost, host,
       NOOP_INCREMENTAL_BUILD);
@@ -501,4 +505,6 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
   missingPipe(): void {}
   illegalAssignmentToTemplateVar(): void {}
   duplicateTemplateVar(): void {}
+  requiresInlineTcb(): void {}
+  requiresInlineTypeConstructors(): void {}
 }

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

@@ -6,7 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {absoluteFrom, getSourceFileOrError} from '../../file_system';
+import {ErrorCode, ngErrorCode} from '../../diagnostics';
+import {absoluteFrom, absoluteFromSourceFile, getSourceFileOrError} from '../../file_system';
 import {runInEachFileSystem} from '../../file_system/testing';
 
 import {getClass, setup} from './test_utils';
@@ -43,5 +44,90 @@ runInEachFileSystem(() => {
       expect(block!.getText()).toMatch(/: i[0-9]\.Cmp1/);
       expect(block!.getText()).toContain(`document.createElement("div")`);
     });
+
+    describe('when inlining is unsupported', () => {
+      it('should not produce errors for components that do not require inlining', () => {
+        const fileName = absoluteFrom('/main.ts');
+        const dirFile = absoluteFrom('/dir.ts');
+        const {program, templateTypeChecker} = setup(
+            [
+              {
+                fileName,
+                source: `export class Cmp {}`,
+                templates: {'Cmp': '<div dir></div>'},
+                declarations: [{
+                  name: 'TestDir',
+                  selector: '[dir]',
+                  file: dirFile,
+                  type: 'directive',
+                }]
+              },
+              {
+                fileName: dirFile,
+                source: `export class TestDir {}`,
+                templates: {},
+              }
+            ],
+            {inlining: false});
+        const sf = getSourceFileOrError(program, fileName);
+        const diags = templateTypeChecker.getDiagnosticsForFile(sf);
+        expect(diags.length).toBe(0);
+      });
+
+      it('should produce errors for components that require TCB inlining', () => {
+        const fileName = absoluteFrom('/main.ts');
+        const {program, templateTypeChecker} = setup(
+            [{
+              fileName,
+              source: `class Cmp {} // not exported, so requires inline`,
+              templates: {'Cmp': '<div></div>'}
+            }],
+            {inlining: false});
+        const sf = getSourceFileOrError(program, fileName);
+        const diags = templateTypeChecker.getDiagnosticsForFile(sf);
+        expect(diags.length).toBe(1);
+        expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INLINE_TCB_REQUIRED));
+      });
+
+      it('should produce errors for components that require type constructor inlining', () => {
+        const fileName = absoluteFrom('/main.ts');
+        const dirFile = absoluteFrom('/dir.ts');
+        const {program, templateTypeChecker} = setup(
+            [
+              {
+                fileName,
+                source: `export class Cmp {}`,
+                templates: {'Cmp': '<div dir></div>'},
+                declarations: [{
+                  name: 'TestDir',
+                  selector: '[dir]',
+                  file: dirFile,
+                  type: 'directive',
+                }]
+              },
+              {
+                fileName: dirFile,
+                source: `
+                  // A non-exported interface used as a type bound for a generic directive causes
+                  // an inline type constructor to be required.
+                  interface NotExported {}
+                  export class TestDir<T extends NotExported> {}`,
+                templates: {},
+              }
+            ],
+            {inlining: false});
+        const sf = getSourceFileOrError(program, fileName);
+        const diags = templateTypeChecker.getDiagnosticsForFile(sf);
+        expect(diags.length).toBe(1);
+        expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INLINE_TYPE_CTOR_REQUIRED));
+
+        // The relatedInformation of the diagnostic should point to the directive which required the
+        // inline type constructor.
+        expect(diags[0].relatedInformation).not.toBeUndefined();
+        expect(diags[0].relatedInformation!.length).toBe(1);
+        expect(diags[0].relatedInformation![0].file).not.toBeUndefined();
+        expect(absoluteFromSourceFile(diags[0].relatedInformation![0].file!)).toBe(dirFile);
+      });
+    });
   });
 });

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

@@ -15,7 +15,7 @@ import {getDeclaration, makeProgram} from '../../testing';
 import {getRootDirs} from '../../util/src/typescript';
 import {ComponentToShimMappingStrategy, UpdateMode} from '../api';
 import {ReusedProgramStrategy} from '../src/augmented_program';
-import {PendingFileTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from '../src/context';
+import {InliningMode, PendingFileTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from '../src/context';
 import {TemplateSourceManager} from '../src/source';
 import {TypeCheckFile} from '../src/type_check_file';
 
@@ -74,7 +74,7 @@ TestClass.ngTypeCtor({value: 'test'});
         ]);
         const ctx = new TypeCheckContextImpl(
             ALL_ENABLED_CONFIG, host, new TestMappingStrategy(), emitter, reflectionHost,
-            new TestTypeCheckingHost());
+            new TestTypeCheckingHost(), InliningMode.InlineOps);
         const TestClass =
             getDeclaration(program, _('/main.ts'), 'TestClass', isNamedClassDeclaration);
         const pendingFile = makePendingFile();
@@ -113,7 +113,7 @@ TestClass.ngTypeCtor({value: 'test'});
         const pendingFile = makePendingFile();
         const ctx = new TypeCheckContextImpl(
             ALL_ENABLED_CONFIG, host, new TestMappingStrategy(), emitter, reflectionHost,
-            new TestTypeCheckingHost());
+            new TestTypeCheckingHost(), InliningMode.InlineOps);
         const TestClass =
             getDeclaration(program, _('/main.ts'), 'TestClass', isNamedClassDeclaration);
         ctx.addInlineTypeCtor(
@@ -158,7 +158,7 @@ TestClass.ngTypeCtor({value: 'test'});
         const pendingFile = makePendingFile();
         const ctx = new TypeCheckContextImpl(
             ALL_ENABLED_CONFIG, host, new TestMappingStrategy(), emitter, reflectionHost,
-            new TestTypeCheckingHost());
+            new TestTypeCheckingHost(), InliningMode.InlineOps);
         const TestClass =
             getDeclaration(program, _('/main.ts'), 'TestClass', isNamedClassDeclaration);
         ctx.addInlineTypeCtor(

packages/language-service/ivy/compiler/compiler.ts

@@ -76,6 +76,7 @@ export class Compiler {
 function createTypeCheckingProgramStrategy(project: ts.server.Project):
     TypeCheckingProgramStrategy {
   return {
+    supportsInlineOperations: false,
     shimPathForComponent(component: ts.ClassDeclaration): AbsoluteFsPath {
       return TypeCheckShimGenerator.shimFor(absoluteFromSourceFile(component.getSourceFile()));
     },