diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index c2b5559f6a..5ea6aec532 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -15,7 +15,7 @@ import {absoluteFrom, relative} from '../../file_system'; import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {DependencyTracker} from '../../incremental/api'; import {IndexingContext} from '../../indexer'; -import {DirectiveMeta, extractDirectiveGuards, InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; +import {DirectiveMeta, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; import {flattenInheritedDirectiveMetadata} from '../../metadata/src/inheritance'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; @@ -51,7 +51,7 @@ export interface ComponentAnalysisData { */ meta: Omit; baseClass: Reference|'dynamic'|null; - guards: ReturnType; + typeCheckMeta: DirectiveTypeCheckMeta; template: ParsedTemplateWithSource; metadataStmt: Statement|null; @@ -327,7 +327,7 @@ export class ComponentDecoratorHandler implements i18nUseExternalIds: this.i18nUseExternalIds, relativeContextFilePath, }, - guards: extractDirectiveGuards(node, this.reflector), + typeCheckMeta: extractDirectiveTypeCheckMeta(node, metadata.inputs, this.reflector), metadataStmt: generateSetClassMetadataCall( node, this.reflector, this.defaultImportRecorder, this.isCore, this.annotateForClosureCompiler), @@ -356,7 +356,7 @@ export class ComponentDecoratorHandler implements queries: analysis.meta.queries.map(query => query.propertyName), isComponent: true, baseClass: analysis.baseClass, - ...analysis.guards, + ...analysis.typeCheckMeta, }); this.injectableRegistry.registerInjectable(node); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 04c6f3f2c4..6bdd3398a7 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -11,8 +11,8 @@ import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {DefaultImportRecorder, Reference} from '../../imports'; -import {InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; -import {extractDirectiveGuards} from '../../metadata/src/util'; +import {DirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; +import {extractDirectiveTypeCheckMeta} from '../../metadata/src/util'; import {DynamicValue, EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; import {LocalModuleScopeRegistry} from '../../scope'; @@ -35,7 +35,7 @@ const LIFECYCLE_HOOKS = new Set([ export interface DirectiveHandlerData { baseClass: Reference|'dynamic'|null; - guards: ReturnType; + typeCheckMeta: DirectiveTypeCheckMeta; meta: R3DirectiveMetadata; metadataStmt: Statement|null; providersRequiringFactory: Set>|null; @@ -102,7 +102,7 @@ export class DirectiveDecoratorHandler implements node, this.reflector, this.defaultImportRecorder, this.isCore, this.annotateForClosureCompiler), baseClass: readBaseClass(node, this.reflector, this.evaluator), - guards: extractDirectiveGuards(node, this.reflector), + typeCheckMeta: extractDirectiveTypeCheckMeta(node, analysis.inputs, this.reflector), providersRequiringFactory } }; @@ -122,7 +122,7 @@ export class DirectiveDecoratorHandler implements queries: analysis.meta.queries.map(query => query.propertyName), isComponent: false, baseClass: analysis.baseClass, - ...analysis.guards, + ...analysis.typeCheckMeta, }); this.injectableRegistry.registerInjectable(node); diff --git a/packages/compiler-cli/src/ngtsc/metadata/index.ts b/packages/compiler-cli/src/ngtsc/metadata/index.ts index 9061d03146..ee3f910b1b 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/index.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/index.ts @@ -9,4 +9,4 @@ export * from './src/api'; export {DtsMetadataReader} from './src/dts'; export {CompoundMetadataRegistry, LocalMetadataRegistry, InjectableClassRegistry} from './src/registry'; -export {extractDirectiveGuards, CompoundMetadataReader} from './src/util'; +export {extractDirectiveTypeCheckMeta, CompoundMetadataReader} from './src/util'; diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index 45fac785ac..a9d58de7c7 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -32,19 +32,55 @@ export interface NgModuleMeta { rawDeclarations: ts.Expression|null; } +/** + * Typing metadata collected for a directive within an NgModule's scope. + */ +export interface DirectiveTypeCheckMeta { + /** + * List of static `ngTemplateGuard_xx` members found on the Directive's class. + * @see `TemplateGuardMeta` + */ + ngTemplateGuards: TemplateGuardMeta[]; + + /** + * Whether the Directive's class has a static ngTemplateContextGuard function. + */ + hasNgTemplateContextGuard: boolean; + + /** + * The set of input fields which have a corresponding static `ngAcceptInputType_` on the + * Directive's class. This allows inputs to accept a wider range of types and coerce the input to + * a narrower type with a getter/setter. See https://angular.io/guide/template-typecheck. + */ + coercedInputFields: Set; + + /** + * The set of input fields which map to `readonly`, `private`, or `protected` members in the + * Directive's class. + */ + restrictedInputFields: Set; + + /** + * The set of input fields which do not have corresponding members in the Directive's class. + */ + undeclaredInputFields: Set; + + /** + * Whether the Directive's class is generic, i.e. `class MyDir {...}`. + */ + isGeneric: boolean; +} + /** * Metadata collected for a directive within an NgModule's scope. */ -export interface DirectiveMeta extends T2DirectiveMeta { +export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta { ref: Reference; /** * Unparsed selector of the directive, or null if the directive does not have a selector. */ selector: string|null; queries: string[]; - ngTemplateGuards: TemplateGuardMeta[]; - hasNgTemplateContextGuard: boolean; - coercedInputFields: Set; /** * A `Reference` to the base class for the directive, if one was detected. diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index c8f2c8b2b1..faf588ab67 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -12,7 +12,7 @@ import {Reference} from '../../imports'; import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost} from '../../reflection'; import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api'; -import {extractDirectiveGuards, extractReferencesFromType, readStringArrayType, readStringMapType, readStringType} from './util'; +import {extractDirectiveTypeCheckMeta, extractReferencesFromType, readStringArrayType, readStringMapType, readStringType} from './util'; /** * A `MetadataReader` that can read metadata from `.d.ts` files, which have static Ivy properties @@ -76,16 +76,17 @@ export class DtsMetadataReader implements MetadataReader { return null; } + const inputs = readStringMapType(def.type.typeArguments[3]); return { ref, name: clazz.name.text, isComponent: def.name === 'ɵcmp', selector: readStringType(def.type.typeArguments[1]), exportAs: readStringArrayType(def.type.typeArguments[2]), - inputs: readStringMapType(def.type.typeArguments[3]), + inputs, outputs: readStringMapType(def.type.typeArguments[4]), queries: readStringArrayType(def.type.typeArguments[5]), - ...extractDirectiveGuards(clazz, this.reflector), + ...extractDirectiveTypeCheckMeta(clazz, inputs, this.reflector), baseClass: readBaseClass(clazz, this.checker, this.reflector), }; } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts index 119a7a0e39..ba775783e1 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts @@ -28,6 +28,8 @@ export function flattenInheritedDirectiveMetadata( let inputs: {[key: string]: string|[string, string]} = {}; let outputs: {[key: string]: string} = {}; let coercedInputFields = new Set(); + let undeclaredInputFields = new Set(); + let restrictedInputFields = new Set(); let isDynamic = false; const addMetadata = (meta: DirectiveMeta): void => { @@ -48,6 +50,12 @@ export function flattenInheritedDirectiveMetadata( for (const coercedInputField of meta.coercedInputFields) { coercedInputFields.add(coercedInputField); } + for (const undeclaredInputField of meta.undeclaredInputFields) { + undeclaredInputFields.add(undeclaredInputField); + } + for (const restrictedInputField of meta.restrictedInputFields) { + restrictedInputFields.add(restrictedInputField); + } }; addMetadata(topMeta); @@ -57,6 +65,8 @@ export function flattenInheritedDirectiveMetadata( inputs, outputs, coercedInputFields, + undeclaredInputFields, + restrictedInputFields, baseClass: isDynamic ? 'dynamic' : null, }; } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts index 457c8cab2c..4cf0e5eabd 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts @@ -12,7 +12,7 @@ import {Reference} from '../../imports'; import {ClassDeclaration, ClassMember, ClassMemberKind, isNamedClassDeclaration, ReflectionHost, reflectTypeEntityToDeclaration} from '../../reflection'; import {nodeDebugInfo} from '../../util/src/typescript'; -import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta, TemplateGuardMeta} from './api'; +import {DirectiveMeta, DirectiveTypeCheckMeta, MetadataReader, NgModuleMeta, PipeMeta, TemplateGuardMeta} from './api'; export function extractReferencesFromType( checker: ts.TypeChecker, def: ts.TypeNode, ngModuleImportedFrom: string|null, @@ -78,13 +78,16 @@ export function readStringArrayType(type: ts.TypeNode): string[] { return res; } - -export function extractDirectiveGuards(node: ClassDeclaration, reflector: ReflectionHost): { - ngTemplateGuards: TemplateGuardMeta[], - hasNgTemplateContextGuard: boolean, - coercedInputFields: Set, -} { - const staticMembers = reflector.getMembersOfClass(node).filter(member => member.isStatic); +/** + * Inspects the class' members and extracts the metadata that is used when type-checking templates + * that use the directive. This metadata does not contain information from a base class, if any, + * making this metadata invariant to changes of inherited classes. + */ +export function extractDirectiveTypeCheckMeta( + node: ClassDeclaration, inputs: {[fieldName: string]: string|[string, string]}, + reflector: ReflectionHost): DirectiveTypeCheckMeta { + const members = reflector.getMembersOfClass(node); + const staticMembers = members.filter(member => member.isStatic); const ngTemplateGuards = staticMembers.map(extractTemplateGuard) .filter((guard): guard is TemplateGuardMeta => guard !== null); const hasNgTemplateContextGuard = staticMembers.some( @@ -93,7 +96,40 @@ export function extractDirectiveGuards(node: ClassDeclaration, reflector: Reflec const coercedInputFields = new Set(staticMembers.map(extractCoercedInput) .filter((inputName): inputName is string => inputName !== null)); - return {hasNgTemplateContextGuard, ngTemplateGuards, coercedInputFields}; + + const restrictedInputFields = new Set(); + const undeclaredInputFields = new Set(); + + for (const fieldName of Object.keys(inputs)) { + const field = members.find(member => member.name === fieldName); + if (field === undefined || field.node === null) { + undeclaredInputFields.add(fieldName); + } else if (isRestricted(field.node)) { + restrictedInputFields.add(fieldName); + } + } + + const arity = reflector.getGenericArityOfClass(node); + + return { + hasNgTemplateContextGuard, + ngTemplateGuards, + coercedInputFields, + restrictedInputFields, + undeclaredInputFields, + isGeneric: arity !== null && arity > 0, + }; +} + +function isRestricted(node: ts.Node): boolean { + if (node.modifiers === undefined) { + return false; + } + + return node.modifiers.some( + modifier => modifier.kind === ts.SyntaxKind.PrivateKeyword || + modifier.kind === ts.SyntaxKind.ProtectedKeyword || + modifier.kind === ts.SyntaxKind.ReadonlyKeyword); } function extractTemplateGuard(member: ClassMember): TemplateGuardMeta|null { diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index 7e98f45aa9..60302988a1 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -243,6 +243,9 @@ function fakeDirective(ref: Reference): DirectiveMeta { hasNgTemplateContextGuard: false, ngTemplateGuards: [], coercedInputFields: new Set(), + restrictedInputFields: new Set(), + undeclaredInputFields: new Set(), + isGeneric: false, baseClass: null, }; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index b7db3f0f6d..f0e241144a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../file_system'; import {Reference} from '../../imports'; -import {TemplateGuardMeta} from '../../metadata'; +import {DirectiveTypeCheckMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; @@ -19,12 +19,9 @@ import {ClassDeclaration} from '../../reflection'; * Extension of `DirectiveMeta` that includes additional information required to type-check the * usage of a particular directive. */ -export interface TypeCheckableDirectiveMeta extends DirectiveMeta { +export interface TypeCheckableDirectiveMeta extends DirectiveMeta, DirectiveTypeCheckMeta { ref: Reference; queries: string[]; - ngTemplateGuards: TemplateGuardMeta[]; - coercedInputFields: Set; - hasNgTemplateContextGuard: boolean; } export type TemplateId = string&{__brand: 'TemplateId'}; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 9bd6897fde..748ee71d74 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -199,12 +199,12 @@ export class TypeCheckContextImpl implements TypeCheckContext { for (const dir of boundTarget.getUsedDirectives()) { const dirRef = dir.ref as Reference>; const dirNode = dirRef.node; - if (requiresInlineTypeCtor(dirNode, this.reflector)) { + + if (dir.isGeneric && 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', diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts index 7c58444851..96ff8178a4 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts @@ -86,6 +86,21 @@ export function tsDeclareVariable(id: ts.Identifier, type: ts.TypeNode): ts.Vari /* declarationList */[decl]); } +/** + * Creates a `ts.TypeQueryNode` for a coerced input. + * + * For example: `typeof MatInput.ngAcceptInputType_value`, where MatInput is `typeName` and `value` + * is the `coercedInputName`. + * + * @param typeName The `EntityName` of the Directive where the static coerced input is defined. + * @param coercedInputName The field name of the coerced input. + */ +export function tsCreateTypeQueryForCoercedInput( + typeName: ts.EntityName, coercedInputName: string): ts.TypeQueryNode { + return ts.createTypeQueryNode( + ts.createQualifiedName(typeName, `ngAcceptInputType_${coercedInputName}`)); +} + /** * Create a `ts.VariableStatement` that initializes a variable with a given expression. * 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 014ff17e02..d2d56de68c 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 @@ -19,7 +19,7 @@ import {Environment} from './environment'; import {astToTypescript, NULL_AS_ANY} from './expression'; import {OutOfBandDiagnosticRecorder} from './oob'; import {ExpressionSemanticVisitor} from './template_semantics'; -import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateVariable, tsDeclareVariable} from './ts_util'; +import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util'; @@ -197,6 +197,7 @@ class TcbTemplateBodyOp extends TcbOp { constructor(private tcb: Context, private scope: Scope, private template: TmplAstTemplate) { super(); } + execute(): null { // An `if` will be constructed, within which the template's children will be type checked. The // `if` is used for two reasons: it creates a new syntactic scope, isolating variables declared @@ -308,13 +309,15 @@ class TcbTextInterpolationOp extends TcbOp { } /** - * A `TcbOp` which constructs an instance of a directive with types inferred from its inputs, which - * also checks the bindings to the directive in the process. + * 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. */ -class TcbDirectiveOp extends TcbOp { +class TcbDirectiveTypeOp extends TcbOp { constructor( private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement, private dir: TypeCheckableDirectiveMeta) { @@ -323,37 +326,190 @@ class TcbDirectiveOp extends TcbOp { execute(): ts.Identifier { const id = this.tcb.allocateId(); - // Process the directive and construct expressions for each of its bindings. - const inputs = tcbGetDirectiveInputs(this.node, this.dir, this.tcb, this.scope); + + const type = this.tcb.env.referenceType(this.dir.ref); + this.scope.addStatement(tsDeclareVariable(id, type)); + return id; + } +} + +/** + * A `TcbOp` which constructs an instance of a directive with types inferred from its inputs. The + * inputs themselves are not checked here; checking of inputs is achieved in `TcbDirectiveInputsOp`. + * Any errors reported in this statement are ignored, as the type constructor call is only present + * for type-inference. + * + * When a Directive is generic, it is required that the TCB generates the instance using this method + * in order to infer the type information correctly. + * + * Executing this operation returns a reference to the directive instance variable with its inferred + * type. + */ +class TcbDirectiveCtorOp extends TcbOp { + constructor( + private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement, + private dir: TypeCheckableDirectiveMeta) { + super(); + } + + execute(): ts.Identifier { + const id = this.tcb.allocateId(); + + const genericInputs = new Map(); + + const inputs = getBoundInputs(this.dir, this.node, this.tcb); + for (const input of inputs) { + for (const fieldName of input.fieldNames) { + // Skip the field if an attribute has already been bound to it; we can't have a duplicate + // key in the type constructor call. + if (genericInputs.has(fieldName)) { + continue; + } + + const expression = translateInput(input.attribute, this.tcb, this.scope); + genericInputs.set(fieldName, { + type: 'binding', + field: fieldName, + expression, + sourceSpan: input.attribute.sourceSpan + }); + } + } + + // Add unset directive inputs for each of the remaining unset fields. + for (const fieldName of Object.keys(this.dir.inputs)) { + if (!genericInputs.has(fieldName)) { + genericInputs.set(fieldName, {type: 'unset', field: fieldName}); + } + } // Call the type constructor of the directive to infer a type, and assign the directive // instance. - const typeCtor = tcbCallTypeCtor(this.dir, this.tcb, inputs); - addParseSpanInfo(typeCtor, this.node.sourceSpan); + const typeCtor = tcbCallTypeCtor(this.dir, this.tcb, Array.from(genericInputs.values())); + ignoreDiagnostics(typeCtor); this.scope.addStatement(tsCreateVariable(id, typeCtor)); return id; } circularFallback(): TcbOp { - return new TcbDirectiveCircularFallbackOp(this.tcb, this.scope, this.node, this.dir); + return new TcbDirectiveCtorCircularFallbackOp(this.tcb, this.scope, this.node, this.dir); + } +} + +/** + * A `TcbOp` which generates code to check input bindings on an element that correspond with the + * members of a directive. + * + * Executing this operation returns nothing. + */ +class TcbDirectiveInputsOp extends TcbOp { + constructor( + private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement, + private dir: TypeCheckableDirectiveMeta) { + super(); + } + + execute(): null { + const dirId = this.scope.resolve(this.node, this.dir); + + // TODO(joost): report duplicate properties + + const inputs = getBoundInputs(this.dir, this.node, this.tcb); + for (const input of inputs) { + // For bound inputs, the property is assigned the binding expression. + let expr = translateInput(input.attribute, this.tcb, this.scope); + if (!this.tcb.env.config.checkTypeOfInputBindings) { + // If checking the type of bindings is disabled, cast the resulting expression to 'any' + // before the assignment. + expr = tsCastToAny(expr); + } else if (!this.tcb.env.config.strictNullInputBindings) { + // If strict null checks are disabled, erase `null` and `undefined` from the type by + // wrapping the expression in a non-null assertion. + expr = ts.createNonNullExpression(expr); + } + + let assignment: ts.Expression = wrapForDiagnostics(expr); + + for (const fieldName of input.fieldNames) { + let target: ts.LeftHandSideExpression; + if (this.dir.coercedInputFields.has(fieldName)) { + // The input has a coercion declaration which should be used instead of assigning the + // expression into the input field directly. To achieve this, a variable is declared + // with a type of `typeof Directive.ngAcceptInputType_fieldName` which is then used as + // target of the assignment. + const dirTypeRef = this.tcb.env.referenceType(this.dir.ref); + if (!ts.isTypeReferenceNode(dirTypeRef)) { + throw new Error( + `Expected TypeReferenceNode from reference to ${this.dir.ref.debugName}`); + } + + const id = this.tcb.allocateId(); + const type = tsCreateTypeQueryForCoercedInput(dirTypeRef.typeName, fieldName); + this.scope.addStatement(tsDeclareVariable(id, type)); + + target = id; + } else if (this.dir.undeclaredInputFields.has(fieldName)) { + // If no coercion declaration is present nor is the field declared (i.e. the input is + // declared in a `@Directive` or `@Component` decorator's `inputs` property) there is no + // assignment target available, so this field is skipped. + continue; + } else if (this.dir.restrictedInputFields.has(fieldName)) { + // To ignore errors, assign to temp variable with type of the field + const id = this.tcb.allocateId(); + const dirTypeRef = this.tcb.env.referenceType(this.dir.ref); + if (!ts.isTypeReferenceNode(dirTypeRef)) { + throw new Error( + `Expected TypeReferenceNode from reference to ${this.dir.ref.debugName}`); + } + const type = ts.createIndexedAccessTypeNode( + ts.createTypeQueryNode(dirId as ts.Identifier), + ts.createLiteralTypeNode(ts.createStringLiteral(fieldName))); + const temp = tsCreateVariable(id, ts.createNonNullExpression(ts.createNull()), type); + addParseSpanInfo(temp, input.attribute.sourceSpan); + this.scope.addStatement(temp); + target = id; + + // TODO: To get errors assign directly to the fields on the instance, using dot access + // when possible + + } else { + // Otherwise, a declaration exists in which case the `dir["fieldName"]` syntax is used + // as assignment target. An element access is used instead of a property access to + // support input names that are not valid JavaScript identifiers. Additionally, using + // element access syntax does not produce + // TS2341 "Property $prop is private and only accessible within class $class." nor + // TS2445 "Property $prop is protected and only accessible within class $class and its + // subclasses." + target = ts.createElementAccess(dirId, ts.createStringLiteral(fieldName)); + } + + // Finally the assignment is extended by assigning it into the target expression. + assignment = ts.createBinary(target, ts.SyntaxKind.EqualsToken, assignment); + } + + addParseSpanInfo(assignment, input.attribute.sourceSpan); + this.scope.addStatement(ts.createExpressionStatement(assignment)); + } + + return null; } } /** * A `TcbOp` which is used to generate a fallback expression if the inference of a directive type - * via `TcbDirectiveOp` requires a reference to its own type. This can happen using a template + * via `TcbDirectiveCtorOp` requires a reference to its own type. This can happen using a template * reference: * * ```html * * ``` * - * In this case, `TcbDirectiveCircularFallbackOp` will add a second inference of the directive type - * to the type-check block, this time calling the directive's type constructor without any input - * expressions. This infers the widest possible supertype for the directive, which is used to + * In this case, `TcbDirectiveCtorCircularFallbackOp` will add a second inference of the directive + * type to the type-check block, this time calling the directive's type constructor without any + * input expressions. This infers the widest possible supertype for the directive, which is used to * resolve any recursive references required to infer the real type. */ -class TcbDirectiveCircularFallbackOp extends TcbOp { +class TcbDirectiveCtorCircularFallbackOp extends TcbOp { constructor( private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement, private dir: TypeCheckableDirectiveMeta) { @@ -694,8 +850,8 @@ class Scope { */ private elementOpMap = new Map(); /** - * A map of maps which tracks the index of `TcbDirectiveOp`s in the `opQueue` for each directive - * on a `TmplAstElement` or `TmplAstTemplate` node. + * A map of maps which tracks the index of `TcbDirectiveCtorOp`s in the `opQueue` for each + * directive on a `TmplAstElement` or `TmplAstTemplate` node. */ private directiveOpMap = new Map>(); @@ -957,8 +1113,12 @@ class Scope { const dirMap = new Map(); for (const dir of directives) { - const dirIndex = this.opQueue.push(new TcbDirectiveOp(this.tcb, this, node, dir)) - 1; + const directiveOp = dir.isGeneric ? new TcbDirectiveCtorOp(this.tcb, this, node, dir) : + new TcbDirectiveTypeOp(this.tcb, this, node, dir); + const dirIndex = this.opQueue.push(directiveOp) - 1; dirMap.set(dir, dirIndex); + + this.opQueue.push(new TcbDirectiveInputsOp(this.tcb, this, node, dir)); } this.directiveOpMap.set(node, dirMap); @@ -1016,6 +1176,11 @@ class Scope { } } +interface TcbBoundInput { + attribute: TmplAstBoundAttribute|TmplAstTextAttribute; + fieldNames: string[]; +} + /** * Create the `ctx` parameter to the top-level TCB function. * @@ -1269,53 +1434,13 @@ function tcbCallTypeCtor( /* argumentsArray */[ts.createObjectLiteral(members)]); } -type TcbDirectiveInput = { - type: 'binding'; field: string; expression: ts.Expression; sourceSpan: ParseSourceSpan; -}|{ - type: 'unset'; - field: string; -}; +function getBoundInputs( + directive: TypeCheckableDirectiveMeta, node: TmplAstTemplate|TmplAstElement, + tcb: Context): TcbBoundInput[] { + const boundInputs: TcbBoundInput[] = []; -function tcbGetDirectiveInputs( - el: TmplAstElement|TmplAstTemplate, dir: TypeCheckableDirectiveMeta, tcb: Context, - scope: Scope): TcbDirectiveInput[] { - // Only the first binding to a property is written. - // TODO(alxhub): produce an error for duplicate bindings to the same property, independently of - // this logic. - const directiveInputs = new Map(); - // `dir.inputs` is an object map of field names on the directive class to property names. - // This is backwards from what's needed to match bindings - a map of properties to field names - // is desired. Invert `dir.inputs` into `propMatch` to create this map. - const propMatch = new Map(); - const inputs = dir.inputs; - Object.keys(inputs).forEach(key => { - Array.isArray(inputs[key]) ? propMatch.set(inputs[key][0], key) : - propMatch.set(inputs[key] as string, key); - }); - - el.inputs.forEach(processAttribute); - el.attributes.forEach(processAttribute); - if (el instanceof TmplAstTemplate) { - el.templateAttrs.forEach(processAttribute); - } - - // Add unset directive inputs for each of the remaining unset fields. - // Note: it's actually important here that `propMatch.values()` isn't used, as there can be - // multiple fields which share the same property name and only one of them will be listed as a - // value in `propMatch`. - for (const field of Object.keys(inputs)) { - if (!directiveInputs.has(field)) { - directiveInputs.set(field, {type: 'unset', field}); - } - } - - return Array.from(directiveInputs.values()); - - /** - * Add a binding expression to the map for each input/template attribute of the directive that has - * a matching binding. - */ - function processAttribute(attr: TmplAstBoundAttribute|TmplAstTextAttribute): void { + const propertyToFieldNames = invertInputs(directive.inputs); + const processAttribute = (attr: TmplAstBoundAttribute|TmplAstTextAttribute) => { // Skip non-property bindings. if (attr instanceof TmplAstBoundAttribute && attr.type !== BindingType.Property) { return; @@ -1327,34 +1452,92 @@ function tcbGetDirectiveInputs( } // Skip the attribute if the directive does not have an input for it. - if (!propMatch.has(attr.name)) { + if (!propertyToFieldNames.has(attr.name)) { return; } - const field = propMatch.get(attr.name)!; + const fieldNames = propertyToFieldNames.get(attr.name)!; + boundInputs.push({attribute: attr, fieldNames}); + }; - // Skip the attribute if a previous binding also wrote to it. - if (directiveInputs.has(field)) { - return; - } + node.inputs.forEach(processAttribute); + node.attributes.forEach(processAttribute); + if (node instanceof TmplAstTemplate) { + node.templateAttrs.forEach(processAttribute); + } - let expr: ts.Expression; - if (attr instanceof TmplAstBoundAttribute) { - // Produce an expression representing the value of the binding. - expr = tcbExpression(attr.value, tcb, scope); - } else { - // For regular attributes with a static string value, use the represented string literal. - expr = ts.createStringLiteral(attr.value); - } + return boundInputs; +} - directiveInputs.set(field, { - type: 'binding', - field: field, - expression: expr, - sourceSpan: attr.sourceSpan, - }); +/** + * Translates the given attribute binding to a `ts.Expression`. + */ +function translateInput( + attr: TmplAstBoundAttribute|TmplAstTextAttribute, tcb: Context, scope: Scope): ts.Expression { + if (attr instanceof TmplAstBoundAttribute) { + // Produce an expression representing the value of the binding. + return tcbExpression(attr.value, tcb, scope); + } else { + // For regular attributes with a static string value, use the represented string literal. + return ts.createStringLiteral(attr.value); } } +/** + * Inverts the input-mapping from field-to-property name into property-to-field name, to be able + * to match a property in a template with the corresponding field on a directive. + */ +function invertInputs(inputs: {[fieldName: string]: string|[string, string]}): + Map { + const propertyToFieldNames = new Map(); + for (const fieldName of Object.keys(inputs)) { + const propertyNames = inputs[fieldName]; + const propertyName = Array.isArray(propertyNames) ? propertyNames[0] : propertyNames; + + if (propertyToFieldNames.has(propertyName)) { + propertyToFieldNames.get(propertyName)!.push(fieldName); + } else { + propertyToFieldNames.set(propertyName, [fieldName]); + } + } + return propertyToFieldNames; +} + +/** + * An input binding that corresponds with a field of a directive. + */ +interface TcbDirectiveBoundInput { + type: 'binding'; + + /** + * The name of a field on the directive that is set. + */ + field: string; + + /** + * The `ts.Expression` corresponding with the input binding expression. + */ + expression: ts.Expression; + + /** + * The source span of the full attribute binding. + */ + sourceSpan: ParseSourceSpan; +} + +/** + * Indicates that a certain field of a directive does not have a corresponding input binding. + */ +interface TcbDirectiveUnsetInput { + type: 'unset'; + + /** + * The name of a field on the directive for which no input binding is present. + */ + field: string; +} + +type TcbDirectiveInput = TcbDirectiveBoundInput|TcbDirectiveUnsetInput; + const EVENT_PARAMETER = '$event'; const enum EventParamType { 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 f80fd0fc69..8cb2114ddf 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts @@ -11,6 +11,7 @@ import * as ts from 'typescript'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; import {TypeCtorMetadata} from '../api'; +import {tsCreateTypeQueryForCoercedInput} from './ts_util'; import {TypeParameterEmitter} from './type_parameter_emitter'; export function generateTypeCtorDeclarationFn( @@ -150,9 +151,7 @@ function constructTypeCtorParameter( /* modifiers */ undefined, /* name */ key, /* questionToken */ undefined, - /* type */ - ts.createTypeQueryNode( - ts.createQualifiedName(rawType.typeName, `ngAcceptInputType_${key}`)), + /* type */ tsCreateTypeQueryForCoercedInput(rawType.typeName, key), /* initializer */ undefined)); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index 081296f637..2c278a3d68 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -227,7 +227,8 @@ runInEachFileSystem(() => { name: 'GuardDir', selector: '[guard]', inputs: {'guard': 'guard'}, - ngTemplateGuards: [{inputName: 'guard', type: 'binding'}] + ngTemplateGuards: [{inputName: 'guard', type: 'binding'}], + undeclaredInputFields: ['guard'], }]); expect(messages).toEqual([ 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 99043a6428..f1e285c25a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -120,8 +120,9 @@ export function ngForDeclaration(): TestDeclaration { file: absoluteFrom('/ngfor.d.ts'), selector: '[ngForOf]', name: 'NgForOf', - inputs: {ngForOf: 'ngForOf'}, + inputs: {ngForOf: 'ngForOf', ngForTrackBy: 'ngForTrackBy', ngForTemplate: 'ngForTemplate'}, hasNgTemplateContextGuard: true, + isGeneric: true, }; } @@ -175,11 +176,12 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { // Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead. export type TestDirective = Partial>>&{ - selector: string, - name: string, - file?: AbsoluteFsPath, type: 'directive', - coercedInputFields?: string[], + Exclude< + keyof TypeCheckableDirectiveMeta, + 'ref'|'coercedInputFields'|'restrictedInputFields'|'undeclaredInputFields'>>>&{ + selector: string, name: string, file?: AbsoluteFsPath, type: 'directive', + coercedInputFields?: string[], restrictedInputFields?: string[], + undeclaredInputFields?: string[], isGeneric?: boolean; }; export type TestPipe = { name: string, @@ -417,6 +419,9 @@ function prepareDeclarations( isComponent: decl.isComponent || false, ngTemplateGuards: decl.ngTemplateGuards || [], coercedInputFields: new Set(decl.coercedInputFields || []), + restrictedInputFields: new Set(decl.restrictedInputFields || []), + undeclaredInputFields: new Set(decl.undeclaredInputFields || []), + isGeneric: decl.isGeneric ?? false, outputs: decl.outputs || {}, queries: decl.queries || [], }; 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 febe9c9e98..d4a6b14a55 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 @@ -55,7 +55,7 @@ describe('type check blocks', () => { selector: '[dir]', inputs: {inputA: 'inputA'}, }]; - expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": ("value")'); + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2: DirA = (null!); _t2["inputA"] = ("value");'); }); it('should handle multiple bindings to the same property', () => { @@ -67,8 +67,8 @@ describe('type check blocks', () => { inputs: {inputA: 'inputA'}, }]; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('"inputA": (1)'); - expect(block).not.toContain('"inputA": (2)'); + expect(block).toContain('_t2["inputA"] = (1);'); + expect(block).toContain('_t2["inputA"] = (2);'); }); it('should handle empty bindings', () => { @@ -79,7 +79,7 @@ describe('type check blocks', () => { selector: '[dir-a]', inputs: {inputA: 'inputA'}, }]; - expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": (undefined)'); + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2["inputA"] = (undefined);'); }); it('should handle bindings without value', () => { @@ -90,7 +90,7 @@ describe('type check blocks', () => { selector: '[dir-a]', inputs: {inputA: 'inputA'}, }]; - expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": (undefined)'); + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2["inputA"] = (undefined);'); }); it('should handle implicit vars on ng-template', () => { @@ -109,20 +109,148 @@ describe('type check blocks', () => { expect(tcb(TEMPLATE)).toContain('var _t2 = _t1.$implicit;'); }); - it('should handle missing property bindings', () => { - const TEMPLATE = `
`; - const DIRECTIVES: TestDeclaration[] = [{ - type: 'directive', - name: 'Dir', - selector: '[dir]', - inputs: { - fieldA: 'inputA', - fieldB: 'inputB', - }, - }]; - expect(tcb(TEMPLATE, DIRECTIVES)) - .toContain( - 'var _t2 = Dir.ngTypeCtor({ "fieldA": (((ctx).foo)), "fieldB": (null as any) });'); + describe('type constructors', () => { + it('should handle missing property bindings', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + fieldA: 'inputA', + fieldB: 'inputB', + }, + isGeneric: true, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2 = Dir.ngTypeCtor({ "fieldA": (((ctx).foo)), "fieldB": (null as any) });'); + }); + + it('should handle multiple bindings to the same property', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + fieldA: 'inputA', + }, + isGeneric: true, + }]; + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain('"fieldA": (1)'); + expect(block).not.toContain('"fieldA": (2)'); + }); + + + it('should only apply property bindings to directives', () => { + const TEMPLATE = ` +
+ `; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: {'color': 'color', 'strong': 'strong', 'enabled': 'enabled'}, + isGeneric: true, + }]; + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain( + 'var _t2 = Dir.ngTypeCtor({ "color": (null as any), "strong": (null as any), "enabled": (null as any) });'); + expect(block).toContain('"blue"; false; true;'); + }); + + it('should generate a circular directive reference correctly', () => { + const TEMPLATE = ` +
+ `; + const DIRECTIVES: TestDirective[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + exportAs: ['dir'], + inputs: {input: 'input'}, + isGeneric: true, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t3 = Dir.ngTypeCtor((null!)); ' + + 'var _t2 = Dir.ngTypeCtor({ "input": (_t3) });'); + }); + + it('should generate circular references between two directives correctly', () => { + const TEMPLATE = ` +
A
+
B
+`; + const DIRECTIVES: TestDirective[] = [ + { + type: 'directive', + name: 'DirA', + selector: '[dir-a]', + exportAs: ['dirA'], + inputs: {inputA: 'inputA'}, + isGeneric: true, + }, + { + type: 'directive', + name: 'DirB', + selector: '[dir-b]', + exportAs: ['dirB'], + inputs: {inputB: 'inputB'}, + isGeneric: true, + } + ]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t4 = DirA.ngTypeCtor((null!)); ' + + 'var _t3 = DirB.ngTypeCtor({ "inputB": (_t4) }); ' + + 'var _t2 = DirA.ngTypeCtor({ "inputA": (_t3) });'); + }); + + it('should handle empty bindings', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'DirA', + selector: '[dir-a]', + inputs: {inputA: 'inputA'}, + isGeneric: true, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": (undefined)'); + }); + + it('should handle bindings without value', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'DirA', + selector: '[dir-a]', + inputs: {inputA: 'inputA'}, + isGeneric: true, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('"inputA": (undefined)'); + }); + + it('should use coercion types if declared', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + fieldA: 'inputA', + }, + isGeneric: true, + coercedInputFields: ['fieldA'], + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2 = Dir.ngTypeCtor({ "fieldA": (((ctx).foo)) }); ' + + 'var _t3: typeof Dir.ngAcceptInputType_fieldA = (null!); ' + + '_t3 = (((ctx).foo));'); + }); }); it('should generate a forward element reference correctly', () => { @@ -147,7 +275,7 @@ describe('type check blocks', () => { }]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t1 = Dir.ngTypeCtor({}); "" + ((_t1).value); var _t2 = document.createElement("div");'); + 'var _t1: Dir = (null!); "" + ((_t1).value); var _t2 = document.createElement("div");'); }); it('should handle style and class bindings specially', () => { @@ -173,8 +301,10 @@ describe('type check blocks', () => { inputs: {'color': 'color', 'strong': 'strong', 'enabled': 'enabled'}, }]; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain( - 'var _t2 = Dir.ngTypeCtor({ "color": (null as any), "strong": (null as any), "enabled": (null as any) });'); + expect(block).toContain('var _t2: Dir = (null!);'); + expect(block).not.toContain('"color"'); + expect(block).not.toContain('"strong"'); + expect(block).not.toContain('"enabled"'); expect(block).toContain('"blue"; false; true;'); }); @@ -191,8 +321,8 @@ describe('type check blocks', () => { }]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t3 = Dir.ngTypeCtor((null!)); ' + - 'var _t2 = Dir.ngTypeCtor({ "input": (_t3) });'); + 'var _t2: Dir = (null!); ' + + '_t2["input"] = (_t2);'); }); it('should generate circular references between two directives correctly', () => { @@ -218,9 +348,139 @@ describe('type check blocks', () => { ]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t4 = DirA.ngTypeCtor((null!)); ' + - 'var _t3 = DirB.ngTypeCtor({ "inputA": (_t4) }); ' + - 'var _t2 = DirA.ngTypeCtor({ "inputA": (_t3) });'); + 'var _t2: DirA = (null!); ' + + 'var _t3: DirB = (null!); ' + + '_t2["inputA"] = (_t3); ' + + 'var _t4 = document.createElement("div"); ' + + '_t3["inputA"] = (_t2);'); + }); + + it('should handle undeclared properties', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + fieldA: 'inputA', + }, + undeclaredInputFields: ['fieldA'] + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2: Dir = (null!); ' + + '(((ctx).foo)); '); + }); + + it('should handle restricted properties', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + fieldA: 'inputA', + }, + restrictedInputFields: ['fieldA'] + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2: Dir = (null!); ' + + 'var _t3: typeof _t2["fieldA"] = (null!); ' + + '_t3 = (((ctx).foo)); '); + }); + + it('should handle a single property bound to multiple fields', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + field1: 'inputA', + field2: 'inputA', + }, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2: Dir = (null!); ' + + '_t2["field2"] = _t2["field1"] = (((ctx).foo));'); + }); + + it('should handle a single property bound to multiple fields, where one of them is coerced', + () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + field1: 'inputA', + field2: 'inputA', + }, + coercedInputFields: ['field1'], + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2: Dir = (null!); ' + + 'var _t3: typeof Dir.ngAcceptInputType_field1 = (null!); ' + + '_t2["field2"] = _t3 = (((ctx).foo));'); + }); + + it('should handle a single property bound to multiple fields, where one of them is undeclared', + () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + field1: 'inputA', + field2: 'inputA', + }, + undeclaredInputFields: ['field1'], + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2: Dir = (null!); ' + + '_t2["field2"] = (((ctx).foo));'); + }); + + it('should use coercion types if declared', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + fieldA: 'inputA', + }, + coercedInputFields: ['fieldA'], + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2: Dir = (null!); ' + + 'var _t3: typeof Dir.ngAcceptInputType_fieldA = (null!); ' + + '_t3 = (((ctx).foo));'); + }); + + it('should use coercion types if declared, even when backing field is not declared', () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + fieldA: 'inputA', + }, + coercedInputFields: ['fieldA'], + undeclaredInputFields: ['fieldA'], + }]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2: Dir = (null!); ' + + 'var _t3: typeof Dir.ngAcceptInputType_fieldA = (null!); ' + + '_t3 = (((ctx).foo));'); }); it('should handle $any casts', () => { @@ -379,14 +639,14 @@ describe('type check blocks', () => { it('should include null and undefined when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)) })'); + expect(block).toContain('_t2["dirInput"] = (((ctx).a));'); expect(block).toContain('((ctx).b);'); }); it('should use the non-null assertion operator when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, strictNullInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)!) })'); + expect(block).toContain('_t2["dirInput"] = (((ctx).a)!);'); expect(block).toContain('((ctx).b)!;'); }); }); @@ -395,7 +655,7 @@ describe('type check blocks', () => { it('should check types of bindings when enabled', () => { const TEMPLATE = `
`; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a)) })'); + expect(block).toContain('_t2["dirInput"] = (((ctx).a));'); expect(block).toContain('((ctx).b);'); }); @@ -404,7 +664,7 @@ describe('type check blocks', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((((ctx).a) as any)) })'); + expect(block).toContain('_t2["dirInput"] = ((((ctx).a) as any));'); expect(block).toContain('(((ctx).b) as any);'); }); @@ -413,8 +673,7 @@ describe('type check blocks', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain( - 'Dir.ngTypeCtor({ "dirInput": ((((((ctx).a)) === (((ctx).b))) as any)) })'); + expect(block).toContain('_t2["dirInput"] = ((((((ctx).a)) === (((ctx).b))) as any));'); }); }); @@ -534,17 +793,17 @@ describe('type check blocks', () => { it('should assign string value to the input when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('"disabled": ("")'); - expect(block).toContain('"cols": ("3")'); - expect(block).toContain('"rows": (2)'); + expect(block).toContain('_t2["disabled"] = ("");'); + expect(block).toContain('_t2["cols"] = ("3");'); + expect(block).toContain('_t2["rows"] = (2);'); }); it('should use any for attributes but still check bound attributes when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfAttributes: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('"disabled": (null as any)'); - expect(block).toContain('"cols": (null as any)'); - expect(block).toContain('"rows": (2)'); + expect(block).not.toContain('"disabled"'); + expect(block).not.toContain('"cols"'); + expect(block).toContain('_t2["rows"] = (2);'); }); }); 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 432a1b8dd0..0a075efbf3 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 @@ -86,6 +86,7 @@ runInEachFileSystem(os => { selector: '[dir]', file: dirFile, type: 'directive', + isGeneric: true, }; const {program, templateTypeChecker, programStrategy} = setup([ { @@ -104,7 +105,7 @@ runInEachFileSystem(os => { // 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 {}`, + export abstract class TestDir {}`, templates: {}, }, ]); @@ -161,7 +162,7 @@ runInEachFileSystem(os => { const {program, templateTypeChecker} = setup( [{ fileName, - source: `class Cmp {} // not exported, so requires inline`, + source: `abstract class Cmp {} // not exported, so requires inline`, templates: {'Cmp': '
'} }], {inlining: false}); @@ -188,6 +189,7 @@ runInEachFileSystem(os => { selector: '[dir]', file: dirFile, type: 'directive', + isGeneric: true, }] }, { @@ -196,7 +198,7 @@ runInEachFileSystem(os => { // 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 {}`, + export abstract class TestDir {}`, templates: {}, } ], diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 7c9ec4dbf4..5646850c22 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -136,11 +136,42 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags.length).toBe(1); - expect(diags[0].messageText).toEqual(`Type 'string' is not assignable to type 'number'.`); + expect(diags[0].messageText).toEqual(`Type '"2"' is not assignable to type 'number'.`); // The reported error code should be in the TS error space, not a -99 "NG" code. expect(diags[0].code).toBeGreaterThan(0); }); + it('should produce diagnostics when mapping to multiple fields and bound types are incorrect', + () => { + env.tsconfig( + {fullTemplateTypeCheck: true, strictInputTypes: true, strictAttributeTypes: true}); + env.write('test.ts', ` + import {Component, Directive, NgModule, Input} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp {} + + @Directive({selector: '[dir]'}) + class TestDir { + @Input('foo') foo1: number; + @Input('foo') foo2: number; + } + + @NgModule({ + declarations: [TestCmp, TestDir], + }) + class Module {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText).toEqual(`Type '"2"' is not assignable to type 'number'.`); + expect(diags[1].messageText).toEqual(`Type '"2"' is not assignable to type 'number'.`); + }); + it('should support inputs and outputs with names that are not JavaScript identifiers', () => { env.tsconfig( {fullTemplateTypeCheck: true, strictInputTypes: true, strictOutputEventTypes: true}); @@ -173,7 +204,7 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags.length).toBe(2); - expect(diags[0].messageText).toEqual(`Type 'number' is not assignable to type 'string'.`); + expect(diags[0].messageText).toEqual(`Type '2' is not assignable to type 'string'.`); expect(diags[1].messageText) .toEqual(`Argument of type 'string' is not assignable to parameter of type 'number'.`); }); @@ -349,7 +380,7 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags.length).toBe(2); - expect(diags[0].messageText).toEqual(`Type 'number' is not assignable to type 'string'.`); + expect(diags[0].messageText).toEqual(`Type '1' is not assignable to type 'string'.`); expect(diags[1].messageText) .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); }); @@ -359,7 +390,7 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags.length).toBe(2); - expect(diags[0].messageText).toEqual(`Type 'number' is not assignable to type 'string'.`); + expect(diags[0].messageText).toEqual(`Type '1' is not assignable to type 'string'.`); expect(diags[1].messageText) .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); }); @@ -685,8 +716,8 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags.length).toBe(2); - expect(diags[0].messageText).toEqual(`Type 'string' is not assignable to type 'boolean'.`); - expect(diags[1].messageText).toEqual(`Type 'string' is not assignable to type 'number'.`); + expect(diags[0].messageText).toEqual(`Type '""' is not assignable to type 'boolean'.`); + expect(diags[1].messageText).toEqual(`Type '"3"' is not assignable to type 'number'.`); }); it('should produce an error for text attributes when overall strictness is enabled', () => { @@ -694,8 +725,8 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags.length).toBe(2); - expect(diags[0].messageText).toEqual(`Type 'string' is not assignable to type 'boolean'.`); - expect(diags[1].messageText).toEqual(`Type 'string' is not assignable to type 'number'.`); + expect(diags[0].messageText).toEqual(`Type '""' is not assignable to type 'boolean'.`); + expect(diags[1].messageText).toEqual(`Type '"3"' is not assignable to type 'number'.`); }); it('should not produce an error for text attributes when not enabled', () => { @@ -1212,9 +1243,9 @@ export declare class AnimationEvent { expect(diags.length).toBe(3); expect(diags[0].messageText).toBe(`Type 'true' is not assignable to type 'number'.`); expect(getSourceCodeForDiagnostic(diags[0])).toEqual('[fromAbstract]="true"'); - expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'string'.`); + expect(diags[1].messageText).toBe(`Type '3' is not assignable to type 'string'.`); expect(getSourceCodeForDiagnostic(diags[1])).toEqual('[fromBase]="3"'); - expect(diags[2].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`); + expect(diags[2].messageText).toBe(`Type '4' is not assignable to type 'boolean'.`); expect(getSourceCodeForDiagnostic(diags[2])).toEqual('[fromChild]="4"'); }); @@ -1269,9 +1300,9 @@ export declare class AnimationEvent { expect(diags.length).toBe(3); expect(diags[0].messageText).toBe(`Type 'true' is not assignable to type 'number'.`); expect(getSourceCodeForDiagnostic(diags[0])).toEqual('[fromAbstract]="true"'); - expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'string'.`); + expect(diags[1].messageText).toBe(`Type '3' is not assignable to type 'string'.`); expect(getSourceCodeForDiagnostic(diags[1])).toEqual('[fromBase]="3"'); - expect(diags[2].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`); + expect(diags[2].messageText).toBe(`Type '4' is not assignable to type 'boolean'.`); expect(getSourceCodeForDiagnostic(diags[2])).toEqual('[fromChild]="4"'); }); @@ -1476,7 +1507,7 @@ export declare class AnimationEvent { it('should give an error if the binding expression type is not accepted by the coercion function', () => { env.write('test.ts', ` - import {Component, NgModule} from '@angular/core'; + import {Component, NgModule, Input, Directive} from '@angular/core'; import {MatInputModule} from '@angular/material'; @Component({ @@ -1533,6 +1564,199 @@ export declare class AnimationEvent { }); }); + describe('restricted inputs', () => { + const directiveDeclaration = ` + @Directive({selector: '[dir]'}) + export class TestDir { + @Input() + protected protectedField!: string; + @Input() + private privateField!: string; + @Input() + readonly readonlyField!: string; + } + `; + + describe('with strict inputs', () => { + beforeEach(() => { + env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true}); + }); + + it('should not produce diagnostics for correct inputs which assign to readonly, private, or protected fields', + () => { + env.write('test.ts', ` + import {Component, NgModule, Input, Directive} from '@angular/core'; + + @Component({ + selector: 'blah', + template: '
', + }) + export class FooCmp { + value = "value"; + } + + ${directiveDeclaration} + + @NgModule({ + declarations: [FooCmp, TestDir], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should not produce diagnostics for correct inputs which assign to readonly, private, or protected fields inherited from a base class', + () => { + env.write('test.ts', ` + import {Component, NgModule, Input, Directive} from '@angular/core'; + + @Component({ + selector: 'blah', + template: '
', + }) + export class FooCmp { + value = "value"; + } + + ${directiveDeclaration} + + @Directive({selector: '[child-dir]'}) + export class ChildDir extends TestDir { + } + + @NgModule({ + declarations: [FooCmp, ChildDir], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should produce diagnostics when assigning incorrect type to readonly, private, or protected fields', + () => { + env.write('test.ts', ` + import {Component, NgModule, Input, Directive} from '@angular/core'; + + @Component({ + selector: 'blah', + template: '
', + }) + export class FooCmp { + value = 1; + } + + ${directiveDeclaration} + + @NgModule({ + declarations: [FooCmp, TestDir], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(3); + expect(diags[0].messageText) + .toEqual(`Type 'number' is not assignable to type 'string'.`); + expect(diags[1].messageText) + .toEqual(`Type 'number' is not assignable to type 'string'.`); + expect(diags[2].messageText) + .toEqual(`Type 'number' is not assignable to type 'string'.`); + }); + }); + }); + + it('should not produce diagnostics for undeclared inputs', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true}); + env.write('test.ts', ` + import {Component, NgModule, Input, Directive} from '@angular/core'; + + @Component({ + selector: 'blah', + template: '
', + }) + export class FooCmp { + value = "value"; + } + + @Directive({ + selector: '[dir]', + inputs: ['undeclared'], + }) + export class TestDir { + } + + @NgModule({ + declarations: [FooCmp, TestDir], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should produce diagnostics for invalid expressions when assigned into an undeclared input', + () => { + env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true}); + env.write('test.ts', ` + import {Component, NgModule, Input, Directive} from '@angular/core'; + + @Component({ + selector: 'blah', + template: '
', + }) + export class FooCmp { + } + + @Directive({ + selector: '[dir]', + inputs: ['undeclared'], + }) + export class TestDir { + } + + @NgModule({ + declarations: [FooCmp, TestDir], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'value' does not exist on type 'FooCmp'.`); + }); + + it('should not produce diagnostics for undeclared inputs inherited from a base class', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true}); + env.write('test.ts', ` + import {Component, NgModule, Input, Directive} from '@angular/core'; + + @Component({ + selector: 'blah', + template: '
', + }) + export class FooCmp { + value = "value"; + } + + @Directive({ + inputs: ['undeclaredBase'], + }) + export class BaseDir { + } + + @Directive({selector: '[dir]'}) + export class TestDir extends BaseDir { + } + + @NgModule({ + declarations: [FooCmp, TestDir], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + describe('legacy schema checking with the DOM schema', () => { beforeEach(() => { env.tsconfig({ivyTemplateTypeCheck: true, fullTemplateTypeCheck: false});