diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 7a5b8ed90d2..d483243aad9 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -658,6 +658,8 @@ export class NgCompiler { // is not disabled when `strictTemplates` is enabled. const strictTemplates = !!this.options.strictTemplates; + const useInlineTypeConstructors = this.typeCheckingProgramStrategy.supportsInlineOperations; + // First select a type-checking configuration, based on whether full template type-checking is // requested. let typeCheckingConfig: TypeCheckingConfig; @@ -689,6 +691,7 @@ export class NgCompiler { useContextGenericType: strictTemplates, strictLiteralTypes: true, enableTemplateTypeChecker: this.enableTemplateTypeChecker, + useInlineTypeConstructors, }; } else { typeCheckingConfig = { @@ -713,6 +716,7 @@ export class NgCompiler { useContextGenericType: false, strictLiteralTypes: false, enableTemplateTypeChecker: this.enableTemplateTypeChecker, + useInlineTypeConstructors, }; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index 014fa840367..81b8c998e9a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -260,6 +260,21 @@ export interface TypeCheckingConfig { * literals are cast to `any` when declared. */ strictLiteralTypes: boolean; + + /** + * Whether to use inline type constructors. + * + * If this is `true`, create inline type constructors when required. For example, if a type + * constructor's parameters has private types, it cannot be created normally, so we inline it in + * the directives definition file. + * + * If false, do not create inline type constructors. Fall back to using `any` type for + * constructors that normally require inlining. + * + * This option requires the environment to support inlining. If the environment does not support + * inlining, this must be set to `false`. + */ + useInlineTypeConstructors: boolean; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 446b9f39b65..42bc6b79e93 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -179,7 +179,12 @@ export class TypeCheckContextImpl implements TypeCheckContext { private compilerHost: Pick, private componentMappingStrategy: ComponentToShimMappingStrategy, private refEmitter: ReferenceEmitter, private reflector: ReflectionHost, - private host: TypeCheckingHost, private inlining: InliningMode) {} + private host: TypeCheckingHost, private inlining: InliningMode) { + if (inlining === InliningMode.Error && config.useInlineTypeConstructors) { + // We cannot use inlining for type checking since this environment does not support it. + throw new Error(`AssertionError: invalid inlining configuration.`); + } + } /** * A `Map` of `ts.SourceFile`s that the context has seen to the operations (additions of methods @@ -219,23 +224,21 @@ export class TypeCheckContextImpl implements TypeCheckContext { ...this.getTemplateDiagnostics(parseErrors, templateId, sourceMapping)); } - // Accumulate a list of any directives which could not have type constructors generated due to - // unsupported inlining operations. - let missingInlines: ClassDeclaration[] = []; - const boundTarget = binder.bind({template}); - // Get all of the directives used in the template and record type constructors for all of them. - for (const dir of boundTarget.getUsedDirectives()) { - const dirRef = dir.ref as Reference>; - const dirNode = dirRef.node; + if (this.inlining === InliningMode.InlineOps) { + // Get all of the directives used in the template and record inline type constructors when + // required. + for (const dir of boundTarget.getUsedDirectives()) { + const dirRef = dir.ref as Reference>; + const dirNode = dirRef.node; - if (dir.isGeneric && requiresInlineTypeCtor(dirNode, this.reflector)) { - if (this.inlining === InliningMode.Error) { - missingInlines.push(dirNode); + if (!dir.isGeneric || !requiresInlineTypeCtor(dirNode, this.reflector)) { + // inlining not required continue; } - // Add a type constructor operation for the directive. + + // Add an inline type constructor operation for the directive. this.addInlineTypeCtor(fileData, dirNode.getSourceFile(), dirRef, { fnName: 'ngTypeCtor', // The constructor should have a body if the directive comes from a .ts file, but not if @@ -262,18 +265,12 @@ export class TypeCheckContextImpl implements TypeCheckContext { // 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)) { + if (this.inlining === InliningMode.Error && tcbRequiresInline) { // 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(templateId, ref.node); - } - - if (missingInlines.length > 0) { - shimData.oobRecorder.requiresInlineTypeConstructors(templateId, ref.node, missingInlines); - } + shimData.oobRecorder.requiresInlineTcb(templateId, ref.node); // Checking this template would be unsupported, so don't try. return; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index 4f5dddb68ce..2d2fdb734de 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -43,7 +43,7 @@ export class Environment { constructor( readonly config: TypeCheckingConfig, protected importManager: ImportManager, - private refEmitter: ReferenceEmitter, private reflector: ReflectionHost, + private refEmitter: ReferenceEmitter, readonly reflector: ReflectionHost, protected contextFile: ts.SourceFile) {} /** 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 4ee589a5f1d..6a6a6abdad9 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 @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {Reference} from '../../imports'; import {ClassPropertyName} from '../../metadata'; -import {ClassDeclaration} from '../../reflection'; +import {ClassDeclaration, ReflectionHost} from '../../reflection'; import {TemplateId, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata} from '../api'; import {addExpressionIdentifier, ExpressionIdentifier, markIgnoreDiagnostics} from './comments'; @@ -21,7 +21,8 @@ import {Environment} from './environment'; import {astToTypescript, NULL_AS_ANY} from './expression'; import {OutOfBandDiagnosticRecorder} from './oob'; import {ExpressionSemanticVisitor} from './template_semantics'; -import {tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util'; +import {checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util'; +import {requiresInlineTypeCtor} from './type_constructor'; /** * Given a `ts.ClassDeclaration` for a component, and metadata regarding that component, compose a @@ -357,18 +358,13 @@ class TcbTextInterpolationOp extends TcbOp { } /** - * A `TcbOp` which constructs an instance of a directive _without_ setting any of its inputs. Inputs - * are later set in the `TcbDirectiveInputsOp`. Type checking was found to be faster when done in - * this way as opposed to `TcbDirectiveCtorOp` which is only necessary when the directive is - * generic. - * - * Executing this operation returns a reference to the directive instance variable with its inferred - * type. + * A `TcbOp` which constructs an instance of a directive. For generic directives, generic + * parameters are set to `any` type. */ -class TcbDirectiveTypeOp extends TcbOp { +abstract class TcbDirectiveTypeOpBase extends TcbOp { constructor( - private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement, - private dir: TypeCheckableDirectiveMeta) { + protected tcb: Context, protected scope: Scope, + protected node: TmplAstTemplate|TmplAstElement, protected dir: TypeCheckableDirectiveMeta) { super(); } @@ -380,9 +376,24 @@ class TcbDirectiveTypeOp extends TcbOp { } execute(): ts.Identifier { - const id = this.tcb.allocateId(); + const dirRef = this.dir.ref as Reference>; - const type = this.tcb.env.referenceType(this.dir.ref); + const rawType = this.tcb.env.referenceType(this.dir.ref); + + let type: ts.TypeNode; + if (this.dir.isGeneric === false || dirRef.node.typeParameters === undefined) { + type = rawType; + } else { + if (!ts.isTypeReferenceNode(rawType)) { + throw new Error( + `Expected TypeReferenceNode when referencing the type for ${this.dir.ref.debugName}`); + } + const typeArguments = dirRef.node.typeParameters.map( + () => ts.factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); + type = ts.factory.createTypeReferenceNode(rawType.typeName, typeArguments); + } + + const id = this.tcb.allocateId(); addExpressionIdentifier(type, ExpressionIdentifier.DIRECTIVE); addParseSpanInfo(type, this.node.startSourceSpan || this.node.sourceSpan); this.scope.addStatement(tsDeclareVariable(id, type)); @@ -390,6 +401,49 @@ class TcbDirectiveTypeOp extends TcbOp { } } +/** + * A `TcbOp` which constructs an instance of a non-generic directive _without_ setting any of its + * inputs. Inputs are later set in the `TcbDirectiveInputsOp`. Type checking was found to be + * faster when done in this way as opposed to `TcbDirectiveCtorOp` which is only necessary when the + * directive is generic. + * + * Executing this operation returns a reference to the directive instance variable with its inferred + * type. + */ +class TcbNonGenericDirectiveTypeOp extends TcbDirectiveTypeOpBase { + /** + * Creates a variable declaration for this op's directive of the argument type. Returns the id of + * the newly created variable. + */ + execute(): ts.Identifier { + const dirRef = this.dir.ref as Reference>; + if (this.dir.isGeneric) { + throw new Error(`Assertion Error: expected ${dirRef.debugName} not to be generic.`); + } + return super.execute(); + } +} + +/** + * A `TcbOp` which constructs an instance of a generic directive with its generic parameters set + * to `any` type. This op is like `TcbDirectiveTypeOp`, except that generic parameters are set to + * `any` type. This is used for situations where we want to avoid inlining. + * + * Executing this operation returns a reference to the directive instance variable with its generic + * type parameters set to `any`. + */ +class TcbGenericDirectiveTypeWithAnyParamsOp extends TcbDirectiveTypeOpBase { + execute(): ts.Identifier { + const dirRef = this.dir.ref as Reference>; + if (dirRef.node.typeParameters === undefined) { + throw new Error(`Assertion Error: expected typeParameters when creating a declaration for ${ + dirRef.debugName}`); + } + + return super.execute(); + } +} + /** * A `TcbOp` which creates a variable for a local ref in a template. * The initializer for the variable is the variable expression for the directive, template, or @@ -1383,8 +1437,27 @@ class Scope { const dirMap = new Map(); for (const dir of directives) { - const directiveOp = dir.isGeneric ? new TcbDirectiveCtorOp(this.tcb, this, node, dir) : - new TcbDirectiveTypeOp(this.tcb, this, node, dir); + let directiveOp: TcbOp; + const host = this.tcb.env.reflector; + const dirRef = dir.ref as Reference>; + + if (!dir.isGeneric) { + // The most common case is that when a directive is not generic, we use the normal + // `TcbNonDirectiveTypeOp`. + directiveOp = new TcbNonGenericDirectiveTypeOp(this.tcb, this, node, dir); + } else if ( + !requiresInlineTypeCtor(dirRef.node, host) || + this.tcb.env.config.useInlineTypeConstructors) { + // For generic directives, we use a type constructor to infer types. If a directive requires + // an inline type constructor, then inlining must be available to use the + // `TcbDirectiveCtorOp`. If not we, we fallback to using `any` – see below. + directiveOp = new TcbDirectiveCtorOp(this.tcb, this, node, dir); + } else { + // If inlining is not available, then we give up on infering the generic params, and use + // `any` type for the directive's generic parameters. + directiveOp = new TcbGenericDirectiveTypeWithAnyParamsOp(this.tcb, this, node, dir); + } + const dirIndex = this.opQueue.push(directiveOp) - 1; dirMap.set(dir, dirIndex); 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 0f1caacc250..e3935db0153 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -166,7 +166,7 @@ export function ngForTypeCheckTarget(): TypeCheckingTarget { }; } -export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { +export const ALL_ENABLED_CONFIG: Readonly = { applyTemplateContextGuards: true, checkQueries: false, checkTemplateBodies: true, @@ -188,37 +188,55 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { useContextGenericType: true, strictLiteralTypes: true, enableTemplateTypeChecker: false, + useInlineTypeConstructors: true }; // Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead. -export type TestDirective = Partial>>&{ - selector: string, name: string, file?: AbsoluteFsPath, type: 'directive', - inputs?: {[fieldName: string]: string}, outputs?: {[fieldName: string]: string}, - coercedInputFields?: string[], restrictedInputFields?: string[], - stringLiteralInputFields?: string[], undeclaredInputFields?: string[], isGeneric?: boolean; -}; + 'undeclaredInputFields'|'inputs'|'outputs'>>> { + selector: string; + name: string; + file?: AbsoluteFsPath; + type: 'directive'; + inputs?: {[fieldName: string]: string}; + outputs?: {[fieldName: string]: string}; + coercedInputFields?: string[]; + restrictedInputFields?: string[]; + stringLiteralInputFields?: string[]; + undeclaredInputFields?: string[]; + isGeneric?: boolean; + code?: string; +} -export type TestPipe = { - name: string, - file?: AbsoluteFsPath, pipeName: string, type: 'pipe', -}; +export interface TestPipe { + name: string; + file?: AbsoluteFsPath; + pipeName: string; + type: 'pipe'; + code?: string; +} export type TestDeclaration = TestDirective|TestPipe; export function tcb( - template: string, declarations: TestDeclaration[] = [], config?: TypeCheckingConfig, + template: string, declarations: TestDeclaration[] = [], config?: Partial, options?: {emitSpans?: boolean}): string { - const classes = ['Test', ...declarations.map(decl => decl.name)]; - const code = classes.map(name => `export class ${name} {}`).join('\n'); + const codeLines = [`export class Test {}`]; + for (const decl of declarations) { + if (decl.code !== undefined) { + codeLines.push(decl.code); + } else { + codeLines.push(`export class ${decl.name} {}`); + } + } const rootFilePath = absoluteFrom('/synthetic.ts'); const {program, host} = makeProgram([ - {name: rootFilePath, contents: code, isRoot: true}, + {name: rootFilePath, contents: codeLines.join('\n'), isRoot: true}, ]); const sf = getSourceFileOrError(program, rootFilePath); @@ -233,7 +251,7 @@ export function tcb( const id = 'tcb' as TemplateId; const meta: TypeCheckBlockMetadata = {id, boundTarget, pipes, schemas: []}; - config = config || { + const fullConfig: TypeCheckingConfig = { applyTemplateContextGuards: true, checkQueries: false, checkTypeOfInputBindings: true, @@ -253,6 +271,8 @@ export function tcb( useContextGenericType: true, strictLiteralTypes: true, enableTemplateTypeChecker: false, + useInlineTypeConstructors: true, + ...config }; options = options || { emitSpans: false, @@ -265,7 +285,7 @@ export function tcb( const refEmmiter: ReferenceEmitter = new ReferenceEmitter( [new LocalIdentifierStrategy(), new RelativePathStrategy(reflectionHost)]); - const env = new TypeCheckFile(fileName, config, refEmmiter, reflectionHost, host); + const env = new TypeCheckFile(fileName, fullConfig, refEmmiter, reflectionHost, host); const ref = new Reference(clazz); @@ -373,7 +393,14 @@ export function setup(targets: TypeCheckingTarget[], overrides: { program, checker, moduleResolver, new TypeScriptReflectionHost(checker)), new LogicalProjectStrategy(reflectionHost, logicalFs), ]); - const fullConfig = {...ALL_ENABLED_CONFIG, ...config}; + + const fullConfig = { + ...ALL_ENABLED_CONFIG, + useInlineTypeConstructors: overrides.inlining !== undefined ? + overrides.inlining : + ALL_ENABLED_CONFIG.useInlineTypeConstructors, + ...config + }; // Map out the scope of each target component, which is needed for the ComponentScopeReader. const scopeMap = new Map(); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 6a36e7547ce..5f06d3a3581 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -745,6 +745,7 @@ describe('type check blocks', () => { useContextGenericType: true, strictLiteralTypes: true, enableTemplateTypeChecker: false, + useInlineTypeConstructors: true }; describe('config.applyTemplateContextGuards', () => { @@ -1077,4 +1078,35 @@ describe('type check blocks', () => { }); }); }); + + it('should use `any` type for type constructors with bound generic params ' + + 'when `useInlineTypeConstructors` is `false`', + () => { + const template = ` +
+ `; + const declarations: TestDeclaration[] = [{ + code: ` + interface PrivateInterface{}; + export class Dir {}; + `, + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + inputA: 'inputA', + inputB: 'inputB', + }, + isGeneric: true + }]; + + const renderedTcb = tcb(template, declarations, {useInlineTypeConstructors: false}); + + expect(renderedTcb).toContain(`var _t1: i0.Dir = null!;`); + expect(renderedTcb).toContain(`_t1.inputA = (((ctx).foo));`); + expect(renderedTcb).toContain(`_t1.inputB = (((ctx).bar));`); + }); }); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts index 1d4cb3d1b3b..f08ad4b72e6 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts @@ -1587,6 +1587,8 @@ function assertDomBindingSymbol(tSymbol: Symbol): asserts tSymbol is DomBindingS } export function setup(targets: TypeCheckingTarget[], config?: Partial) { - return baseTestSetup( - targets, {inlining: false, config: {...config, enableTemplateTypeChecker: true}}); + return baseTestSetup(targets, { + inlining: false, + config: {...config, enableTemplateTypeChecker: true, useInlineTypeConstructors: false} + }); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts index 226840a4bc4..3774e4f9eee 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts @@ -168,49 +168,8 @@ runInEachFileSystem(() => { 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': '
'}, - declarations: [{ - name: 'TestDir', - selector: '[dir]', - file: dirFile, - type: 'directive', - isGeneric: true, - }] - }, - { - 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 {}`, - templates: {}, - } - ], - {inlining: false}); - const sf = getSourceFileOrError(program, fileName); - const diags = templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram); - 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. - const dirSf = getSourceFileOrError(program, dirFile); - 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(dirSf.fileName); - }); }); + describe('getTemplateOfComponent()', () => { it('should provide access to a component\'s real template', () => { const fileName = absoluteFrom('/main.ts'); diff --git a/packages/language-service/ivy/test/quick_info_spec.ts b/packages/language-service/ivy/test/quick_info_spec.ts index 817f127680c..11b610cf171 100644 --- a/packages/language-service/ivy/test/quick_info_spec.ts +++ b/packages/language-service/ivy/test/quick_info_spec.ts @@ -479,6 +479,49 @@ describe('quick info', () => { }); }); + describe('generics', () => { + beforeEach(() => { + initMockFileSystem('Native'); + env = LanguageServiceTestEnv.setup(); + }); + + it('should get quick info for the generic input of a directive that normally requires inlining', + () => { + // When compiling normally, we would have to inline the type constructor of `GenericDir` + // because its generic type parameter references `PrivateInterface`, which is not exported. + project = env.addProject('test', { + 'app.ts': ` + import {Directive, Component, Input, NgModule} from '@angular/core'; + + interface PrivateInterface {} + + @Directive({ + selector: '[dir]' + })export class GenericDir { + @Input('input') input: T = null!; + } + + @Component({ + selector: 'some-cmp', + templateUrl: './app.html' + })export class SomeCmp{} + + @NgModule({ + declarations: [GenericDir, SomeCmp], + })export class AppModule{} + `, + 'app.html': ``, + }); + + expectQuickInfo({ + templateOverride: `
`, + expectedSpanText: 'input', + expectedDisplayString: '(property) GenericDir.input: any' + }); + }); + }); + + describe('non-strict compiler options', () => { beforeEach(() => { initMockFileSystem('Native'); @@ -512,6 +555,7 @@ describe('quick info', () => { }); }); + function expectQuickInfo( {templateOverride, expectedSpanText, expectedDisplayString}: {templateOverride: string, expectedSpanText: string, expectedDisplayString: string}):