fix(language-service): use 'any' instead of failing for inline TCBs (#…

…41513)

In environments such as the Language Service where inline type-checking code
is not supported, the compiler would previously produce a diagnostic when a
template would require inlining to check. This happened whenever its
component class had generic parameters with bounds that could not be safely
reproduced in an external TCB. However, this created a bad user experience
for the Language Service, as its features would then not function with such
templates.

Instead, this commit changes the compiler to use the same strategy for
inline TCBs as it does for inline type constructors - falling back to `any`
for generic types when inlining isn't available. This allows the LS to
support such templates with slightly weaker type-checking semantics, which
a test verifies. There is still a case where components that aren't
exported require an inline TCB, and the compiler will still generate a
diagnostic if so.

Fixes #41395

PR Close #41513

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

@@ -22,8 +22,8 @@ import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom';
 import {Environment} from './environment';
 import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob';
 import {TemplateSourceManager} from './source';
-import {requiresInlineTypeCheckBlock} from './tcb_util';
-import {generateTypeCheckBlock} from './type_check_block';
+import {requiresInlineTypeCheckBlock, TcbInliningRequirement} from './tcb_util';
+import {generateTypeCheckBlock, TcbGenericContextBehavior} from './type_check_block';
 import {TypeCheckFile} from './type_check_file';
 import {generateInlineTypeCtor, requiresInlineTypeCtor} from './type_constructor';
 
@@ -262,11 +262,12 @@ export class TypeCheckContextImpl implements TypeCheckContext {
       templateDiagnostics,
     });
 
-    const tcbRequiresInline = requiresInlineTypeCheckBlock(ref.node, pipes);
+    const inliningRequirement = requiresInlineTypeCheckBlock(ref.node, pipes, this.reflector);
 
     // 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) {
+    if (this.inlining === InliningMode.Error &&
+        inliningRequirement === TcbInliningRequirement.MustInline) {
       // This template cannot be supported because the underlying strategy does not support inlining
       // and inlining would be required.
 
@@ -285,13 +286,26 @@ export class TypeCheckContextImpl implements TypeCheckContext {
       schemas,
     };
     this.perf.eventCount(PerfEvent.GenerateTcb);
-    if (tcbRequiresInline) {
+    if (inliningRequirement !== TcbInliningRequirement.None &&
+        this.inlining === InliningMode.InlineOps) {
       // 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);
+    } else if (
+        inliningRequirement === TcbInliningRequirement.ShouldInlineForGenericBounds &&
+        this.inlining === InliningMode.Error) {
+      // It's suggested that this TCB should be generated inline due to the component's generic
+      // bounds, but inlining is not supported by the current environment. Use a non-inline type
+      // check block, but fall back to `any` generic parameters since the generic bounds can't be
+      // referenced in that context. This will infer a less useful type for the component, but allow
+      // for type-checking it in an environment where that would not be possible otherwise.
+      shimData.file.addTypeCheckBlock(
+          ref, meta, shimData.domSchemaChecker, shimData.oobRecorder,
+          TcbGenericContextBehavior.FallbackToAny);
     } else {
-      // The class can be type-checked externally as normal.
-      shimData.file.addTypeCheckBlock(ref, meta, shimData.domSchemaChecker, shimData.oobRecorder);
+      shimData.file.addTypeCheckBlock(
+          ref, meta, shimData.domSchemaChecker, shimData.oobRecorder,
+          TcbGenericContextBehavior.UseEmitter);
     }
   }
 
@@ -402,7 +416,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
       this.opMap.set(sf, []);
     }
     const ops = this.opMap.get(sf)!;
-    ops.push(new TcbOp(
+    ops.push(new InlineTcbOp(
         ref, tcbMeta, this.config, this.reflector, shimData.domSchemaChecker,
         shimData.oobRecorder));
     fileData.hasInlines = true;
@@ -481,9 +495,9 @@ interface Op {
 }
 
 /**
- * A type check block operation which produces type check code for a particular component.
+ * A type check block operation which produces inline type check code for a particular component.
  */
-class TcbOp implements Op {
+class InlineTcbOp implements Op {
   constructor(
       readonly ref: Reference<ClassDeclaration<ts.ClassDeclaration>>,
       readonly meta: TypeCheckBlockMetadata, readonly config: TypeCheckingConfig,
@@ -501,8 +515,12 @@ class TcbOp implements Op {
       string {
     const env = new Environment(this.config, im, refEmitter, this.reflector, sf);
     const fnName = ts.createIdentifier(`_tcb_${this.ref.node.pos}`);
+
+    // Inline TCBs should copy any generic type parameter nodes directly, as the TCB code is inlined
+    // into the class in a context where that will always be legal.
     const fn = generateTypeCheckBlock(
-        env, this.ref, fnName, this.meta, this.domSchemaChecker, this.oobRecorder);
+        env, this.ref, fnName, this.meta, this.domSchemaChecker, this.oobRecorder,
+        TcbGenericContextBehavior.CopyClassNodes);
     return printer.printNode(ts.EmitHint.Unspecified, fn, sf);
   }
 }

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

@@ -7,7 +7,7 @@
  */
 
 import {AbsoluteSourceSpan, ParseSourceSpan} from '@angular/compiler';
-import {ClassDeclaration} from '@angular/compiler-cli/src/ngtsc/reflection';
+import {ClassDeclaration, ReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection';
 import * as ts from 'typescript';
 
 import {Reference} from '../../imports';
@@ -16,6 +16,7 @@ import {FullTemplateMapping, SourceLocation, TemplateId, TemplateSourceMapping}
 
 import {hasIgnoreForDiagnosticsMarker, readSpanComment} from './comments';
 import {checkIfClassIsExported, checkIfGenericTypesAreUnbound} from './ts_util';
+import {TypeParameterEmitter} from './type_parameter_emitter';
 
 /**
  * Adapter interface which allows the template type-checking diagnostics code to interpret offsets
@@ -38,25 +39,51 @@ export interface TemplateSourceResolver {
   toParseSourceSpan(id: TemplateId, span: AbsoluteSourceSpan): ParseSourceSpan|null;
 }
 
+/**
+ * Indicates whether a particular component requires an inline type check block.
+ *
+ * This is not a boolean state as inlining might only be required to get the best possible
+ * type-checking, but the component could theoretically still be checked without it.
+ */
+export enum TcbInliningRequirement {
+  /**
+   * There is no way to type check this component without inlining.
+   */
+  MustInline,
+
+  /**
+   * Inlining should be used due to the component's generic bounds, but a non-inlining fallback
+   * method can be used if that's not possible.
+   */
+  ShouldInlineForGenericBounds,
+
+  /**
+   * There is no requirement for this component's TCB to be inlined.
+   */
+  None,
+}
+
 export function requiresInlineTypeCheckBlock(
     node: ClassDeclaration<ts.ClassDeclaration>,
-    usedPipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>): boolean {
+    usedPipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>,
+    reflector: ReflectionHost): TcbInliningRequirement {
   // In order to qualify for a declared TCB (not inline) two conditions must be met:
   // 1) the class must be exported
-  // 2) it must not have constrained generic types
+  // 2) it must not have contextual generic type bounds
   if (!checkIfClassIsExported(node)) {
     // Condition 1 is false, the class is not exported.
-    return true;
-  } else if (!checkIfGenericTypesAreUnbound(node)) {
-    // Condition 2 is false, the class has constrained generic types
-    return true;
+    return TcbInliningRequirement.MustInline;
+  } else if (!checkIfGenericTypeBoundsAreContextFree(node, reflector)) {
+    // Condition 2 is false, the class has constrained generic types. It should be checked with an
+    // inline TCB if possible, but can potentially use fallbacks to avoid inlining if not.
+    return TcbInliningRequirement.ShouldInlineForGenericBounds;
   } else if (Array.from(usedPipes.values())
                  .some(pipeRef => !checkIfClassIsExported(pipeRef.node))) {
     // If one of the pipes used by the component is not exported, a non-inline TCB will not be able
     // to import it, so this requires an inline TCB.
-    return true;
+    return TcbInliningRequirement.MustInline;
   } else {
-    return false;
+    return TcbInliningRequirement.None;
   }
 }
 
@@ -147,3 +174,9 @@ function getTemplateId(
     return commentText;
   }) as TemplateId || null;
 }
+
+export function checkIfGenericTypeBoundsAreContextFree(
+    node: ClassDeclaration<ts.ClassDeclaration>, reflector: ReflectionHost): boolean {
+  // Generic type parameters are considered context free if they can be emitted into any context.
+  return new TypeParameterEmitter(node.typeParameters, reflector).canEmit();
+}

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

@@ -21,8 +21,37 @@ import {Environment} from './environment';
 import {astToTypescript, NULL_AS_ANY} from './expression';
 import {OutOfBandDiagnosticRecorder} from './oob';
 import {ExpressionSemanticVisitor} from './template_semantics';
-import {checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util';
+import {tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util';
 import {requiresInlineTypeCtor} from './type_constructor';
+import {TypeParameterEmitter} from './type_parameter_emitter';
+
+/**
+ * Controls how generics for the component context class will be handled during TCB generation.
+ */
+export enum TcbGenericContextBehavior {
+  /**
+   * References to generic parameter bounds will be emitted via the `TypeParameterEmitter`.
+   *
+   * The caller must verify that all parameter bounds are emittable in order to use this mode.
+   */
+  UseEmitter,
+
+  /**
+   * Generic parameter declarations will be copied directly from the `ts.ClassDeclaration` of the
+   * component class.
+   *
+   * The caller must only use the generated TCB code in a context where such copies will still be
+   * valid, such as an inline type check block.
+   */
+  CopyClassNodes,
+
+  /**
+   * Any generic parameters for the component context class will be set to `any`.
+   *
+   * Produces a less useful type, but is always safe to use.
+   */
+  FallbackToAny,
+}
 
 /**
  * Given a `ts.ClassDeclaration` for a component, and metadata regarding that component, compose a
@@ -45,11 +74,14 @@ import {requiresInlineTypeCtor} from './type_constructor';
  * and bindings.
  * @param oobRecorder used to record errors regarding template elements which could not be correctly
  * translated into types during TCB generation.
+ * @param genericContextBehavior controls how generic parameters (especially parameters with generic
+ * bounds) will be referenced from the generated TCB code.
  */
 export function generateTypeCheckBlock(
     env: Environment, ref: Reference<ClassDeclaration<ts.ClassDeclaration>>, name: ts.Identifier,
     meta: TypeCheckBlockMetadata, domSchemaChecker: DomSchemaChecker,
-    oobRecorder: OutOfBandDiagnosticRecorder): ts.FunctionDeclaration {
+    oobRecorder: OutOfBandDiagnosticRecorder,
+    genericContextBehavior: TcbGenericContextBehavior): ts.FunctionDeclaration {
   const tcb = new Context(
       env, domSchemaChecker, oobRecorder, meta.id, meta.boundTarget, meta.pipes, meta.schemas);
   const scope = Scope.forNodes(tcb, null, tcb.boundTarget.target.template !, /* guard */ null);
@@ -58,7 +90,34 @@ export function generateTypeCheckBlock(
     throw new Error(
         `Expected TypeReferenceNode when referencing the ctx param for ${ref.debugName}`);
   }
-  const paramList = [tcbCtxParam(ref.node, ctxRawType.typeName, env.config.useContextGenericType)];
+
+  let typeParameters: ts.TypeParameterDeclaration[]|undefined = undefined;
+  let typeArguments: ts.TypeNode[]|undefined = undefined;
+
+  if (ref.node.typeParameters !== undefined) {
+    if (!env.config.useContextGenericType) {
+      genericContextBehavior = TcbGenericContextBehavior.FallbackToAny;
+    }
+
+    switch (genericContextBehavior) {
+      case TcbGenericContextBehavior.UseEmitter:
+        // Guaranteed to emit type parameters since we checked that the class has them above.
+        typeParameters = new TypeParameterEmitter(ref.node.typeParameters, env.reflector)
+                             .emit(typeRef => env.referenceType(typeRef))!;
+        typeArguments = typeParameters.map(param => ts.factory.createTypeReferenceNode(param.name));
+        break;
+      case TcbGenericContextBehavior.CopyClassNodes:
+        typeParameters = [...ref.node.typeParameters];
+        typeArguments = typeParameters.map(param => ts.factory.createTypeReferenceNode(param.name));
+        break;
+      case TcbGenericContextBehavior.FallbackToAny:
+        typeArguments = ref.node.typeParameters.map(
+            () => ts.factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));
+        break;
+    }
+  }
+
+  const paramList = [tcbCtxParam(ref.node, ctxRawType.typeName, typeArguments)];
 
   const scopeStatements = scope.render();
   const innerBody = ts.createBlock([
@@ -74,7 +133,7 @@ export function generateTypeCheckBlock(
       /* modifiers */ undefined,
       /* asteriskToken */ undefined,
       /* name */ name,
-      /* typeParameters */ env.config.useContextGenericType ? ref.node.typeParameters : undefined,
+      /* typeParameters */ env.config.useContextGenericType ? typeParameters : undefined,
       /* parameters */ paramList,
       /* type */ undefined,
       /* body */ body);
@@ -1571,27 +1630,13 @@ interface TcbBoundInput {
 }
 
 /**
- * Create the `ctx` parameter to the top-level TCB function.
- *
- * This is a parameter with a type equivalent to the component type, with all generic type
- * parameters listed (without their generic bounds).
+ * Create the `ctx` parameter to the top-level TCB function, with the given generic type arguments.
  */
 function tcbCtxParam(
     node: ClassDeclaration<ts.ClassDeclaration>, name: ts.EntityName,
-    useGenericType: boolean): ts.ParameterDeclaration {
-  let typeArguments: ts.TypeNode[]|undefined = undefined;
-  // Check if the component is generic, and pass generic type parameters if so.
-  if (node.typeParameters !== undefined) {
-    if (useGenericType) {
-      typeArguments =
-          node.typeParameters.map(param => ts.createTypeReferenceNode(param.name, undefined));
-    } else {
-      typeArguments =
-          node.typeParameters.map(() => ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));
-    }
-  }
-  const type = ts.createTypeReferenceNode(name, typeArguments);
-  return ts.createParameter(
+    typeArguments: ts.TypeNode[]|undefined): ts.ParameterDeclaration {
+  const type = ts.factory.createTypeReferenceNode(name, typeArguments);
+  return ts.factory.createParameterDeclaration(
       /* decorators */ undefined,
       /* modifiers */ undefined,
       /* dotDotDotToken */ undefined,

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

@@ -16,7 +16,7 @@ import {TypeCheckBlockMetadata, TypeCheckingConfig} from '../api';
 import {DomSchemaChecker} from './dom';
 import {Environment} from './environment';
 import {OutOfBandDiagnosticRecorder} from './oob';
-import {generateTypeCheckBlock} from './type_check_block';
+import {generateTypeCheckBlock, TcbGenericContextBehavior} from './type_check_block';
 
 
 
@@ -43,9 +43,11 @@ export class TypeCheckFile extends Environment {
 
   addTypeCheckBlock(
       ref: Reference<ClassDeclaration<ts.ClassDeclaration>>, meta: TypeCheckBlockMetadata,
-      domSchemaChecker: DomSchemaChecker, oobRecorder: OutOfBandDiagnosticRecorder): void {
+      domSchemaChecker: DomSchemaChecker, oobRecorder: OutOfBandDiagnosticRecorder,
+      genericContextBehavior: TcbGenericContextBehavior): void {
     const fnId = ts.createIdentifier(`_tcb${this.nextTcbId++}`);
-    const fn = generateTypeCheckBlock(this, ref, fnId, meta, domSchemaChecker, oobRecorder);
+    const fn = generateTypeCheckBlock(
+        this, ref, fnId, meta, domSchemaChecker, oobRecorder, genericContextBehavior);
     this.tcbStatements.push(fn);
   }
 

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

@@ -10,9 +10,9 @@ import * as ts from 'typescript';
 
 import {ClassDeclaration, ReflectionHost} from '../../reflection';
 import {TypeCtorMetadata} from '../api';
+import {checkIfGenericTypeBoundsAreContextFree} from './tcb_util';
 
 import {tsCreateTypeQueryForCoercedInput} from './ts_util';
-import {TypeParameterEmitter} from './type_parameter_emitter';
 
 export function generateTypeCtorDeclarationFn(
     node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata, nodeTypeRef: ts.EntityName,
@@ -196,12 +196,6 @@ export function requiresInlineTypeCtor(
   return !checkIfGenericTypeBoundsAreContextFree(node, host);
 }
 
-function checkIfGenericTypeBoundsAreContextFree(
-    node: ClassDeclaration<ts.ClassDeclaration>, reflector: ReflectionHost): boolean {
-  // Generic type parameters are considered context free if they can be emitted into any context.
-  return new TypeParameterEmitter(node.typeParameters, reflector).canEmit();
-}
-
 /**
  * Add a default `= any` to type parameters that don't have a default value already.
  *

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

@@ -28,7 +28,7 @@ import {DomSchemaChecker} from '../src/dom';
 import {Environment} from '../src/environment';
 import {OutOfBandDiagnosticRecorder} from '../src/oob';
 import {TypeCheckShimGenerator} from '../src/shim';
-import {generateTypeCheckBlock} from '../src/type_check_block';
+import {generateTypeCheckBlock, TcbGenericContextBehavior} from '../src/type_check_block';
 import {TypeCheckFile} from '../src/type_check_file';
 
 export function typescriptLibDts(): TestFile {
@@ -290,13 +290,9 @@ export function tcb(
 
   const env = new TypeCheckFile(fileName, fullConfig, refEmmiter, reflectionHost, host);
 
-  const ref = new Reference(clazz);
-
-  const tcb = generateTypeCheckBlock(
-      env, ref, ts.createIdentifier('Test_TCB'), meta, new NoopSchemaChecker(),
-      new NoopOobRecorder());
-
-  env.addTypeCheckBlock(ref, meta, new NoopSchemaChecker(), new NoopOobRecorder());
+  env.addTypeCheckBlock(
+      new Reference(clazz), meta, new NoopSchemaChecker(), new NoopOobRecorder(),
+      TcbGenericContextBehavior.UseEmitter);
 
   const rendered = env.render(!options.emitSpans /* removeComments */);
   return rendered.replace(/\s+/g, ' ');