diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 4493872196..52bf545974 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -23,8 +23,8 @@ import {Environment} from './environment'; import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob'; import {TypeCheckShimGenerator} from './shim'; 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>, 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); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts index 43b944d4e2..ace4cc8b2b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts +++ b/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, - usedPipes: Map>>): boolean { + usedPipes: Map>>, + 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, 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(); +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index bf384afc45..cd62b16c39 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/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>, 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, 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, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts index 56c2f85e71..a121bd46df 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts +++ b/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>, 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); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts index 8cb2114ddf..e6d3927407 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts +++ b/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, meta: TypeCtorMetadata, nodeTypeRef: ts.EntityName, @@ -196,12 +196,6 @@ export function requiresInlineTypeCtor( return !checkIfGenericTypeBoundsAreContextFree(node, host); } -function checkIfGenericTypeBoundsAreContextFree( - node: 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. * diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 09c0b72bb4..34bbc26023 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/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, ' '); diff --git a/packages/language-service/ivy/test/diagnostic_spec.ts b/packages/language-service/ivy/test/diagnostic_spec.ts index 3fadd8f966..c65aeca52f 100644 --- a/packages/language-service/ivy/test/diagnostic_spec.ts +++ b/packages/language-service/ivy/test/diagnostic_spec.ts @@ -276,6 +276,26 @@ describe('getSemanticDiagnostics', () => { expect(getTextOfDiagnostic(diag)).toBe('user'); }); + it('should process a component that would otherwise require an inline TCB', () => { + const files = { + 'app.ts': ` + import {Component, NgModule} from '@angular/core'; + import {CommonModule} from '@angular/common'; + + interface PrivateInterface {} + + @Component({ + template: 'Simple template', + }) + export class MyComponent {} + ` + }; + + const project = createModuleAndProjectWithDeclarations(env, 'test', files); + const diags = project.getDiagnosticsForFile('app.ts'); + expect(diags.length).toBe(0); + }); + it('logs perf tracing', () => { const files = { 'app.ts': ` diff --git a/packages/language-service/ivy/testing/src/util.ts b/packages/language-service/ivy/testing/src/util.ts index b763c77f4d..a17c880195 100644 --- a/packages/language-service/ivy/testing/src/util.ts +++ b/packages/language-service/ivy/testing/src/util.ts @@ -38,7 +38,7 @@ export function isNgSpecificDiagnostic(diag: ts.Diagnostic): boolean { } function getFirstClassDeclaration(declaration: string) { - const matches = declaration.match(/(?:export class )(\w+)(?:\s|\{)/); + const matches = declaration.match(/(?:export class )(\w+)(?:\s|\{|<)/); if (matches === null || matches.length !== 2) { throw new Error(`Did not find exactly one exported class in: ${declaration}`); }