diff --git a/goldens/public-api/compiler-cli/compiler_options.api.md b/goldens/public-api/compiler-cli/compiler_options.api.md index 1fdd88e1e51..1c608111f82 100644 --- a/goldens/public-api/compiler-cli/compiler_options.api.md +++ b/goldens/public-api/compiler-cli/compiler_options.api.md @@ -60,6 +60,7 @@ export interface MiscOptions { compileNonExportedClasses?: boolean; disableTypeScriptVersionCheck?: boolean; forbidOrphanComponents?: boolean; + typeCheckHostBindings?: boolean; } // @public diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts index 5534f900431..927968b6e79 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts @@ -11,6 +11,7 @@ import { ParseSourceSpan, SchemaMetadata, TmplAstElement, + TmplAstHostElement, } from '@angular/compiler'; import ts from 'typescript'; @@ -59,7 +60,7 @@ export interface DomSchemaChecker { /** * Check a property binding on an element and record any diagnostics about it. * - * @param id the template ID, suitable for resolution with a `TcbSourceResolver`. + * @param id the type check ID, suitable for resolution with a `TcbSourceResolver`. * @param element the element node in question. * @param name the name of the property being checked. * @param span the source span of the binding. This is redundant with `element.attributes` but is @@ -67,7 +68,7 @@ export interface DomSchemaChecker { * @param schemas any active schemas for the template, which might affect the validity of the * property. */ - checkProperty( + checkTemplateElementProperty( id: string, element: TmplAstElement, name: string, @@ -75,6 +76,23 @@ export interface DomSchemaChecker { schemas: SchemaMetadata[], hostIsStandalone: boolean, ): void; + + /** + * Check a property binding on a host element and record any diagnostics about it. + * @param id the type check ID, suitable for resolution with a `TcbSourceResolver`. + * @param element the element node in question. + * @param name the name of the property being checked. + * @param span the source span of the binding. + * @param schemas any active schemas for the template, which might affect the validity of the + * property. + */ + checkHostElementProperty( + id: string, + element: TmplAstHostElement, + name: string, + span: ParseSourceSpan, + schemas: SchemaMetadata[], + ): void; } /** @@ -129,7 +147,7 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker { } } - checkProperty( + checkTemplateElementProperty( id: TypeCheckId, element: TmplAstElement, name: string, @@ -171,4 +189,31 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker { this._diagnostics.push(diag); } } + + checkHostElementProperty( + id: TypeCheckId, + element: TmplAstHostElement, + name: string, + span: ParseSourceSpan, + schemas: SchemaMetadata[], + ): void { + for (const tagName of element.tagNames) { + if (REGISTRY.hasProperty(tagName, name, schemas)) { + continue; + } + + const errorMessage = `Can't bind to '${name}' since it isn't a known property of '${tagName}'.`; + const mapping = this.resolver.getHostBindingsMapping(id); + const diag = makeTemplateDiagnostic( + id, + mapping, + span, + ts.DiagnosticCategory.Error, + ngErrorCode(ErrorCode.SCHEMA_INVALID_ATTRIBUTE), + errorMessage, + ); + this._diagnostics.push(diag); + break; + } + } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/host_bindings.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/host_bindings.ts index 2ab05fddb56..20bf705eb60 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/host_bindings.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/host_bindings.ts @@ -34,6 +34,12 @@ import ts from 'typescript'; import {createSourceSpan} from '../../annotations/common'; import {ClassDeclaration} from '../../reflection'; +/** + * Comment attached to an AST node that serves as a guard to distinguish nodes + * used for type checking host bindings from ones used for templates. + */ +const GUARD_COMMENT_TEXT = 'hostBindingsBlockGuard'; + /** Node that represent a static name of a member. */ type StaticName = ts.Identifier | ts.StringLiteralLike; @@ -119,6 +125,50 @@ export function createHostElement( return new TmplAstHostElement(tagNames, bindings, listeners, createSourceSpan(sourceNode.name)); } +/** + * Creates an AST node that can be used as a guard in `if` statements to distinguish TypeScript + * nodes used for checking host bindings from ones used for checking templates. + */ +export function createHostBindingsBlockGuard(): ts.Expression { + // Note that the comment text is quite generic. This doesn't really matter, because it is + // used only inside a TCB and there's no way for users to produce a comment there. + // `true /*hostBindings*/`. + const trueExpr = ts.addSyntheticTrailingComment( + ts.factory.createTrue(), + ts.SyntaxKind.MultiLineCommentTrivia, + GUARD_COMMENT_TEXT, + ); + // Wrap the expression in parentheses to ensure that the comment is attached to the correct node. + return ts.factory.createParenthesizedExpression(trueExpr); +} + +/** + * Determines if a given node is a guard that indicates that descendant nodes are used to check + * host bindings. + */ +export function isHostBindingsBlockGuard(node: ts.Node): boolean { + if (!ts.isIfStatement(node)) { + return false; + } + + // Needs to be kept in sync with `createHostBindingsMarker`. + const expr = node.expression; + if (!ts.isParenthesizedExpression(expr) || expr.expression.kind !== ts.SyntaxKind.TrueKeyword) { + return false; + } + + const text = expr.getSourceFile().text; + return ( + ts.forEachTrailingCommentRange( + text, + expr.expression.getEnd(), + (pos, end, kind) => + kind === ts.SyntaxKind.MultiLineCommentTrivia && + text.substring(pos + 2, end - 2) === GUARD_COMMENT_TEXT, + ) || false + ); +} + /** * If possible, creates and tracks the relevant AST node for a binding declared * through a property on the `host` literal. 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 233966dc23d..1dd33905c00 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts @@ -17,6 +17,7 @@ import {FullSourceMapping, SourceLocation, TypeCheckId, SourceMapping} from '../ import {hasIgnoreForDiagnosticsMarker, readSpanComment} from './comments'; import {ReferenceEmitEnvironment} from './reference_emit_environment'; import {TypeParameterEmitter} from './type_parameter_emitter'; +import {isHostBindingsBlockGuard} from './host_bindings'; /** * External modules/identifiers that always should exist for type check @@ -129,14 +130,37 @@ export function getSourceMapping( return null; } - const mapping = resolver.getTemplateSourceMapping(sourceLocation.id); + if (isInHostBindingTcb(node)) { + const hostSourceMapping = resolver.getHostBindingsMapping(sourceLocation.id); + const span = resolver.toHostParseSourceSpan(sourceLocation.id, sourceLocation.span); + if (span === null) { + return null; + } + return {sourceLocation, sourceMapping: hostSourceMapping, span}; + } + const span = resolver.toTemplateParseSourceSpan(sourceLocation.id, sourceLocation.span); if (span === null) { return null; } // TODO(atscott): Consider adding a context span by walking up from `node` until we get a // different span. - return {sourceLocation, sourceMapping: mapping, span}; + return { + sourceLocation, + sourceMapping: resolver.getTemplateSourceMapping(sourceLocation.id), + span, + }; +} + +function isInHostBindingTcb(node: ts.Node): boolean { + let current = node; + while (current && !ts.isFunctionDeclaration(current)) { + if (isHostBindingsBlockGuard(current)) { + return true; + } + current = current.parent; + } + return false; } export function findTypeCheckBlock( @@ -145,7 +169,7 @@ export function findTypeCheckBlock( isDiagnosticRequest: boolean, ): ts.Node | null { for (const stmt of file.statements) { - if (ts.isFunctionDeclaration(stmt) && getTemplateId(stmt, file, isDiagnosticRequest) === id) { + if (ts.isFunctionDeclaration(stmt) && getTypeCheckId(stmt, file, isDiagnosticRequest) === id) { return stmt; } } @@ -174,7 +198,7 @@ export function findSourceLocation( if (span !== null) { // Once the positional information has been extracted, search further up the TCB to extract // the unique id that is attached with the TCB's function declaration. - const id = getTemplateId(node, sourceFile, isDiagnosticsRequest); + const id = getTypeCheckId(node, sourceFile, isDiagnosticsRequest); if (id === null) { return null; } @@ -187,7 +211,7 @@ export function findSourceLocation( return null; } -function getTemplateId( +function getTypeCheckId( node: ts.Node, sourceFile: ts.SourceFile, isDiagnosticRequest: boolean, 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 ce5b3f61f7f..200a0743e6d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts @@ -61,15 +61,32 @@ export function tsCastToAny(expr: ts.Expression): ts.Expression { * Thanks to narrowing of `document.createElement()`, this expression will have its type inferred * based on the tag name, including for custom elements that have appropriate .d.ts definitions. */ -export function tsCreateElement(tagName: string): ts.Expression { +export function tsCreateElement(...tagNames: string[]): ts.Expression { const createElement = ts.factory.createPropertyAccessExpression( /* expression */ ts.factory.createIdentifier('document'), 'createElement', ); + + let arg: ts.Expression; + + if (tagNames.length === 1) { + // If there's only one tag name, we can pass it in directly. + arg = ts.factory.createStringLiteral(tagNames[0]); + } else { + // If there's more than one name, we have to generate a union of all the tag names. To do so, + // create an expression in the form of `null! as 'tag-1' | 'tag-2' | 'tag-3'`. This allows + // TypeScript to infer the type as a union of the differnet tags. + const assertedNullExpression = ts.factory.createNonNullExpression(ts.factory.createNull()); + const type = ts.factory.createUnionTypeNode( + tagNames.map((tag) => ts.factory.createLiteralTypeNode(ts.factory.createStringLiteral(tag))), + ); + arg = ts.factory.createAsExpression(assertedNullExpression, type); + } + return ts.factory.createCallExpression( /* expression */ createElement, /* typeArguments */ undefined, - /* argumentsArray */ [ts.factory.createStringLiteral(tagName)], + /* argumentsArray */ [arg], ); } 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 dd153ccaf29..cabb92f65ed 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 @@ -50,6 +50,7 @@ import { TmplAstText, TmplAstTextAttribute, TmplAstVariable, + TmplAstHostElement, TmplAstViewportDeferredTrigger, TransplantedType, } from '@angular/compiler'; @@ -81,6 +82,7 @@ import { } from './ts_util'; import {requiresInlineTypeCtor} from './type_constructor'; import {TypeParameterEmitter} from './type_parameter_emitter'; +import {createHostBindingsBlockGuard} from './host_bindings'; /** * Controls how generics for the component context class will be handled during TCB generation. @@ -154,7 +156,6 @@ export function generateTypeCheckBlock( meta.isStandalone, meta.preserveWhitespaces, ); - const scope = Scope.forNodes(tcb, null, null, tcb.boundTarget.target.template!, /* guard */ null); const ctxRawType = env.referenceType(ref); if (!ts.isTypeReferenceNode(ctxRawType)) { throw new Error( @@ -195,15 +196,28 @@ export function generateTypeCheckBlock( } const paramList = [tcbThisParam(ctxRawType.typeName, typeArguments)]; + const statements: ts.Statement[] = []; - const scopeStatements = scope.render(); - const innerBody = ts.factory.createBlock([...env.getPreludeStatements(), ...scopeStatements]); + // Add the template type checking code. + if (tcb.boundTarget.target.template !== undefined) { + const templateScope = Scope.forNodes( + tcb, + null, + null, + tcb.boundTarget.target.template, + /* guard */ null, + ); - // Wrap the body in an "if (true)" expression. This is unnecessary but has the effect of causing - // the `ts.Printer` to format the type-check block nicely. - const body = ts.factory.createBlock([ - ts.factory.createIfStatement(ts.factory.createTrue(), innerBody, undefined), - ]); + statements.push(renderBlockStatements(env, templateScope, ts.factory.createTrue())); + } + + // Add the host bindings type checking code. + if (tcb.boundTarget.target.host !== undefined) { + const hostScope = Scope.forNodes(tcb, null, tcb.boundTarget.target.host, null, null); + statements.push(renderBlockStatements(env, hostScope, createHostBindingsBlockGuard())); + } + + const body = ts.factory.createBlock(statements); const fnDecl = ts.factory.createFunctionDeclaration( /* modifiers */ undefined, /* asteriskToken */ undefined, @@ -217,13 +231,28 @@ export function generateTypeCheckBlock( return fnDecl; } +function renderBlockStatements( + env: Environment, + scope: Scope, + wrapperExpression: ts.Expression, +): ts.Statement { + const scopeStatements = scope.render(); + const innerBody = ts.factory.createBlock([...env.getPreludeStatements(), ...scopeStatements]); + + // Wrap the body in an if statement. This serves two purposes: + // 1. It allows us to distinguish between the sections of the block (e.g. host or template). + // 2. It allows the `ts.Printer` to produce better-looking output. + return ts.factory.createIfStatement(wrapperExpression, innerBody); +} + /** Types that can referenced locally in a template. */ type LocalSymbol = | TmplAstElement | TmplAstTemplate | TmplAstVariable | TmplAstLetDeclaration - | TmplAstReference; + | TmplAstReference + | TmplAstHostElement; /** * A code generation operation that's involved in the construction of a Type Check Block. @@ -1104,9 +1133,9 @@ class TcbDirectiveCtorCircularFallbackOp extends TcbOp { class TcbDomSchemaCheckerOp extends TcbOp { constructor( private tcb: Context, - private element: TmplAstElement, + private element: TmplAstElement | TmplAstHostElement, private checkElement: boolean, - private claimedInputs: Set, + private claimedInputs: Set | null, ) { super(); } @@ -1116,21 +1145,25 @@ class TcbDomSchemaCheckerOp extends TcbOp { } override execute(): ts.Expression | null { - if (this.checkElement) { + const element = this.element; + const isTemplateElement = element instanceof TmplAstElement; + const bindings = isTemplateElement ? element.inputs : element.bindings; + + if (this.checkElement && isTemplateElement) { this.tcb.domSchemaChecker.checkElement( this.tcb.id, - this.element, + element, this.tcb.schemas, this.tcb.hostIsStandalone, ); } // TODO(alxhub): this could be more efficient. - for (const binding of this.element.inputs) { + for (const binding of bindings) { const isPropertyBinding = binding.type === BindingType.Property || binding.type === BindingType.TwoWay; - if (isPropertyBinding && this.claimedInputs.has(binding.name)) { + if (isPropertyBinding && this.claimedInputs?.has(binding.name)) { // Skip this binding as it was claimed by a directive. continue; } @@ -1138,14 +1171,25 @@ class TcbDomSchemaCheckerOp extends TcbOp { if (isPropertyBinding && binding.name !== 'style' && binding.name !== 'class') { // A direct binding to a property. const propertyName = ATTR_TO_PROP.get(binding.name) ?? binding.name; - this.tcb.domSchemaChecker.checkProperty( - this.tcb.id, - this.element, - propertyName, - binding.sourceSpan, - this.tcb.schemas, - this.tcb.hostIsStandalone, - ); + + if (isTemplateElement) { + this.tcb.domSchemaChecker.checkTemplateElementProperty( + this.tcb.id, + element, + propertyName, + binding.sourceSpan, + this.tcb.schemas, + this.tcb.hostIsStandalone, + ); + } else { + this.tcb.domSchemaChecker.checkHostElementProperty( + this.tcb.id, + element, + propertyName, + binding.keySpan, + this.tcb.schemas, + ); + } } } return null; @@ -1284,6 +1328,31 @@ class TcbControlFlowContentProjectionOp extends TcbOp { } } +/** + * A `TcbOp` which creates an expression for a the host element of a directive. + * + * Executing this operation returns a reference to the element variable. + */ +class TcbHostElementOp extends TcbOp { + override readonly optional = true; + + constructor( + private tcb: Context, + private scope: Scope, + private element: TmplAstHostElement, + ) { + super(); + } + + override execute(): ts.Identifier { + const id = this.tcb.allocateId(); + const initializer = tsCreateElement(...this.element.tagNames); + addParseSpanInfo(initializer, this.element.sourceSpan); + this.scope.addStatement(tsCreateVariable(id, initializer)); + return id; + } +} + /** * Mapping between attributes names that don't correspond to their element property names. * Note: this mapping has to be kept in sync with the equally named mapping in the runtime. @@ -1313,8 +1382,9 @@ class TcbUnclaimedInputsOp extends TcbOp { constructor( private tcb: Context, private scope: Scope, - private element: TmplAstElement, - private claimedInputs: Set, + private inputs: TmplAstBoundAttribute[], + private target: LocalSymbol, + private claimedInputs: Set | null, ) { super(); } @@ -1329,11 +1399,11 @@ class TcbUnclaimedInputsOp extends TcbOp { let elId: ts.Expression | null = null; // TODO(alxhub): this could be more efficient. - for (const binding of this.element.inputs) { + for (const binding of this.inputs) { const isPropertyBinding = binding.type === BindingType.Property || binding.type === BindingType.TwoWay; - if (isPropertyBinding && this.claimedInputs.has(binding.name)) { + if (isPropertyBinding && this.claimedInputs?.has(binding.name)) { // Skip this binding as it was claimed by a directive. continue; } @@ -1343,7 +1413,7 @@ class TcbUnclaimedInputsOp extends TcbOp { if (this.tcb.env.config.checkTypeOfDomBindings && isPropertyBinding) { if (binding.name !== 'style' && binding.name !== 'class') { if (elId === null) { - elId = this.scope.resolve(this.element); + elId = this.scope.resolve(this.target); } // A direct binding to a property. const propertyName = ATTR_TO_PROP.get(binding.name) ?? binding.name; @@ -1459,8 +1529,10 @@ class TcbUnclaimedOutputsOp extends TcbOp { constructor( private tcb: Context, private scope: Scope, - private element: TmplAstElement, - private claimedOutputs: Set, + private target: LocalSymbol, + private outputs: TmplAstBoundEvent[], + private inputs: TmplAstBoundAttribute[] | null, + private claimedOutputs: Set | null, ) { super(); } @@ -1473,15 +1545,19 @@ class TcbUnclaimedOutputsOp extends TcbOp { let elId: ts.Expression | null = null; // TODO(alxhub): this could be more efficient. - for (const output of this.element.outputs) { - if (this.claimedOutputs.has(output.name)) { + for (const output of this.outputs) { + if (this.claimedOutputs?.has(output.name)) { // Skip this event handler as it was claimed by a directive. continue; } - if (this.tcb.env.config.checkTypeOfOutputEvents && output.name.endsWith('Change')) { + if ( + this.tcb.env.config.checkTypeOfOutputEvents && + this.inputs !== null && + output.name.endsWith('Change') + ) { const inputName = output.name.slice(0, -6); - if (checkSplitTwoWayBinding(inputName, output, this.element.inputs, this.tcb)) { + if (checkSplitTwoWayBinding(inputName, output, this.inputs, this.tcb)) { // Skip this event handler as the error was already handled. continue; } @@ -1504,7 +1580,7 @@ class TcbUnclaimedOutputsOp extends TcbOp { const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer); if (elId === null) { - elId = this.scope.resolve(this.element); + elId = this.scope.resolve(this.target); } const propertyAccess = ts.factory.createPropertyAccessExpression(elId, 'addEventListener'); addParseSpanInfo(propertyAccess, output.keySpan); @@ -1993,6 +2069,12 @@ class Scope { * A map of `TmplAstElement`s to the index of their `TcbElementOp` in the `opQueue` */ private elementOpMap = new Map(); + + /** + * A map of `TmplAstHostElement`s to the index of their `TcbHostElementOp` in the `opQueue` + */ + private hostElementOpMap = new Map(); + /** * A map of maps which tracks the index of `TcbDirectiveCtorOp`s in the `opQueue` for each * directive on a `TmplAstElement` or `TmplAstTemplate` node. @@ -2066,8 +2148,13 @@ class Scope { static forNodes( tcb: Context, parentScope: Scope | null, - scopedNode: TmplAstTemplate | TmplAstIfBlockBranch | TmplAstForLoopBlock | null, - children: TmplAstNode[], + scopedNode: + | TmplAstTemplate + | TmplAstIfBlockBranch + | TmplAstForLoopBlock + | TmplAstHostElement + | null, + children: TmplAstNode[] | null, guard: ts.Expression | null, ): Scope { const scope = new Scope(tcb, parentScope, guard); @@ -2128,9 +2215,13 @@ class Scope { new TcbBlockImplicitVariableOp(tcb, scope, type, variable), ); } + } else if (scopedNode instanceof TmplAstHostElement) { + scope.appendNode(scopedNode); } - for (const node of children) { - scope.appendNode(node); + if (children !== null) { + for (const node of children) { + scope.appendNode(node); + } } // Once everything is registered, we need to check if there are `@let` // declarations that conflict with other local symbols defined after them. @@ -2301,6 +2392,8 @@ class Scope { } else if (ref instanceof TmplAstElement && this.elementOpMap.has(ref)) { // Resolving the DOM node of an element in this template. return this.resolveOp(this.elementOpMap.get(ref)!); + } else if (ref instanceof TmplAstHostElement && this.hostElementOpMap.has(ref)) { + return this.resolveOp(this.hostElementOpMap.get(ref)!); } else { return null; } @@ -2389,6 +2482,14 @@ class Scope { } else { this.letDeclOpMap.set(node.name, {opIndex, node}); } + } else if (node instanceof TmplAstHostElement) { + const opIndex = this.opQueue.push(new TcbHostElementOp(this.tcb, this, node)) - 1; + this.hostElementOpMap.set(node, opIndex); + this.opQueue.push( + new TcbUnclaimedInputsOp(this.tcb, this, node.bindings, node, null), + new TcbUnclaimedOutputsOp(this.tcb, this, node, node.listeners, null, null), + new TcbDomSchemaCheckerOp(this.tcb, node, false, null), + ); } } @@ -2427,8 +2528,8 @@ class Scope { // If there are no directives, then all inputs are unclaimed inputs, so queue an operation // to add them if needed. if (node instanceof TmplAstElement) { - this.opQueue.push(new TcbUnclaimedInputsOp(this.tcb, this, node, claimedInputs)); this.opQueue.push( + new TcbUnclaimedInputsOp(this.tcb, this, node.inputs, node, claimedInputs), new TcbDomSchemaCheckerOp(this.tcb, node, /* checkElement */ true, claimedInputs), ); } @@ -2486,7 +2587,7 @@ class Scope { } } - this.opQueue.push(new TcbUnclaimedInputsOp(this.tcb, this, node, claimedInputs)); + this.opQueue.push(new TcbUnclaimedInputsOp(this.tcb, this, node.inputs, node, claimedInputs)); // If there are no directives which match this element, then it's a "plain" DOM element (or a // web component), and should be checked against the DOM schema. If any directives match, // we must assume that the element could be custom (either a component, or a directive like @@ -2504,7 +2605,16 @@ class Scope { // If there are no directives, then all outputs are unclaimed outputs, so queue an operation // to add them if needed. if (node instanceof TmplAstElement) { - this.opQueue.push(new TcbUnclaimedOutputsOp(this.tcb, this, node, claimedOutputs)); + this.opQueue.push( + new TcbUnclaimedOutputsOp( + this.tcb, + this, + node, + node.outputs, + node.inputs, + claimedOutputs, + ), + ); } return; } @@ -2524,7 +2634,9 @@ class Scope { } } - this.opQueue.push(new TcbUnclaimedOutputsOp(this.tcb, this, node, claimedOutputs)); + this.opQueue.push( + new TcbUnclaimedOutputsOp(this.tcb, this, node, node.outputs, node.inputs, claimedOutputs), + ); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index 6b7d57e5450..7358918e2ff 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -10,13 +10,11 @@ import { BindingPipe, CssSelector, ParseSourceFile, - ParseSourceSpan, parseTemplate, ParseTemplateOptions, PropertyRead, PropertyWrite, R3TargetBinder, - SchemaMetadata, SelectorMatcher, TmplAstElement, TmplAstLetDeclaration, @@ -1011,20 +1009,9 @@ export class NoopSchemaChecker implements DomSchemaChecker { return []; } - checkElement( - id: string, - element: TmplAstElement, - schemas: SchemaMetadata[], - hostIsStandalone: boolean, - ): void {} - checkProperty( - id: string, - element: TmplAstElement, - name: string, - span: ParseSourceSpan, - schemas: SchemaMetadata[], - hostIsStandalone: boolean, - ): void {} + checkElement(): void {} + checkTemplateElementProperty(): void {} + checkHostElementProperty(): void {} } export class NoopOobRecorder implements OutOfBandDiagnosticRecorder { diff --git a/packages/compiler-cli/test/ngtsc/host_bindings_type_check_spec.ts b/packages/compiler-cli/test/ngtsc/host_bindings_type_check_spec.ts new file mode 100644 index 00000000000..2eff5264572 --- /dev/null +++ b/packages/compiler-cli/test/ngtsc/host_bindings_type_check_spec.ts @@ -0,0 +1,720 @@ +/*! + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; +import {loadStandardTestFiles} from '@angular/compiler-cli/src/ngtsc/testing'; +import {NgtscTestEnvironment} from './env'; +import ts from 'typescript'; + +const testFiles = loadStandardTestFiles(); + +function getDiagnosticSourceCode(diag: ts.Diagnostic): string { + return diag.file!.text.slice(diag.start!, diag.start! + diag.length!); +} + +runInEachFileSystem(() => { + describe('type checking of host bindings', () => { + let env!: NgtscTestEnvironment; + + beforeEach(() => { + env = NgtscTestEnvironment.setup(testFiles); + env.tsconfig({ + // Necessary for testing host bindings. + typeCheckHostBindings: true, + + // Not required for host bindings, but they allow us to + // exercise more parts of the type checker. + strictTemplates: true, + strictAttributeTypes: true, + strictDomEventTypes: true, + strictOutputEventTypes: true, + }); + }); + + it('should check the value of an attribute host binding', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + host: {'[attr.id]': 'exists + doesNotExist'}, + }) + export class Comp { + exists = 'exists'; + } + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'doesNotExist' does not exist on type 'Comp'.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('doesNotExist'); + }); + + it('should check the value of a style host binding', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + host: {'[style.color]': 'exists + doesNotExist'}, + }) + export class Comp { + exists = 'exists'; + } + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'doesNotExist' does not exist on type 'Comp'.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('doesNotExist'); + }); + + it('should check the value of a class host binding', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + host: {'[class.foo]': 'exists + doesNotExist'}, + }) + export class Comp { + exists = 'exists'; + } + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'doesNotExist' does not exist on type 'Comp'.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('doesNotExist'); + }); + + it('should check the value of an animation host binding', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + host: {'[@someAnimation]': 'exists + doesNotExist'}, + }) + export class Comp { + exists = 'exists'; + } + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'doesNotExist' does not exist on type 'Comp'.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('doesNotExist'); + }); + + it('should check the value of a property host binding', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + host: {'[id]': 'exists + doesNotExist'}, + }) + export class Comp { + exists = 'exists'; + } + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'doesNotExist' does not exist on type 'Comp'.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('doesNotExist'); + }); + + it('should validate host bidings against the schema', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + host: {'[foo]': '123'}, + }) + export class Comp {} + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe( + `Can't bind to 'foo' since it isn't a known property of 'ng-component'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('[foo]'); + }); + + it('should infer the element name from the selector', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + selector: 'input[foo]', + host: {'[foo]': '123'}, + }) + export class Comp {} + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe( + `Can't bind to 'foo' since it isn't a known property of 'input'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('[foo]'); + }); + + it('should report if one tag name supports a property, but another one does not', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + selector: 'input[foo], div[foo]', + host: {'[value]': '123'}, + }) + export class Comp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe( + `Can't bind to 'value' since it isn't a known property of 'div'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('[value]'); + }); + + it('should check host event listeners', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + selector: 'button[foo]', + host: {'(click)': 'handleEvent($event)'}, + }) + export class Comp { + handleEvent(event: KeyboardEvent) {} + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect((diags[0].messageText as ts.DiagnosticMessageChain).messageText).toBe( + `Argument of type 'MouseEvent' is not assignable to parameter of type 'KeyboardEvent'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('$event'); + }); + + it('should check host event listeners with a target', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + selector: 'button[foo]', + host: {'(document:click)': 'handleEvent($event)'}, + }) + export class Comp { + handleEvent(event: KeyboardEvent) {} + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect((diags[0].messageText as ts.DiagnosticMessageChain).messageText).toBe( + `Argument of type 'MouseEvent' is not assignable to parameter of type 'KeyboardEvent'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('$event'); + }); + + it('should check host animation event listeners', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + selector: 'button[foo]', + host: {'(@someAnimation.done)': 'handleEvent()'}, + }) + export class Comp { + handleEvent(event: Event) {} + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Expected 1 arguments, but got 0.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('handleEvent'); + }); + + it('should not leak @let from the template into the host bindings', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + host: {'[attr.id]': 'foo + bar'}, + template: \` + @let bar = 'bar'; + {{foo + bar}} + \`, + }) + export class Comp { + foo = 'foo'; + } + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'bar' does not exist on type 'Comp'.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('bar'); + }); + + it('should not leak local reference from the template into the host bindings', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + host: {'[attr.id]': 'foo + bar.id'}, + template: \` +
+ {{foo + bar.id}} + \`, + }) + export class Comp { + foo = 'foo'; + } + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'bar' does not exist on type 'Comp'.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('bar'); + }); + + it('should be able to report diagnostics both from a static inline template and host bindings', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '
{{doesNotExistTemplate}}
', + host: {'[attr.id]': 'doesNotExistHost'}, + }) + export class Comp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText).toBe( + `Property 'doesNotExistTemplate' does not exist on type 'Comp'.`, + ); + expect(diags[1].messageText).toBe( + `Property 'doesNotExistHost' does not exist on type 'Comp'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('doesNotExistTemplate'); + expect(getDiagnosticSourceCode(diags[1])).toBe('doesNotExistHost'); + }); + + it('should be able to report diagnostics both from an external template and host bindings', () => { + env.write('template.html', '
{{doesNotExistTemplate}}
'); + + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: './template.html', + host: {'[attr.id]': 'doesNotExistHost'}, + }) + export class Comp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText).toBe( + `Property 'doesNotExistTemplate' does not exist on type 'Comp'.`, + ); + expect(diags[1].messageText).toBe( + `Property 'doesNotExistHost' does not exist on type 'Comp'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('doesNotExistTemplate'); + expect(getDiagnosticSourceCode(diags[1])).toBe('doesNotExistHost'); + }); + + it('should be able to report diagnostics both from a dynamic template and host bindings', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + const templateRemainder = 'ExistTemplate}}'; + + @Component({ + template: '
{{doesNot' + templateRemainder, + host: {'[attr.id]': 'doesNotExistHost'}, + }) + export class Comp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText).toBe( + `Property 'doesNotExistTemplate' does not exist on type 'Comp'.`, + ); + expect(diags[1].messageText).toBe( + `Property 'doesNotExistHost' does not exist on type 'Comp'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('doesNotExistTemplate'); + expect(getDiagnosticSourceCode(diags[1])).toBe('doesNotExistHost'); + }); + + it('should check @HostBinding decorator with no arguments', () => { + env.write( + 'test.ts', + ` + import {Component, HostBinding} from '@angular/core'; + + @Component({template: ''}) + export class Comp { + @HostBinding() foo = 'foo'; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe( + `Can't bind to 'foo' since it isn't a known property of 'ng-component'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('foo'); + }); + + it('should check @HostBinding decorator with a string argument', () => { + env.write( + 'test.ts', + ` + import {Component, HostBinding} from '@angular/core'; + + @Component({template: ''}) + export class Comp { + @HostBinding('foo') id = 'foo'; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe( + `Can't bind to 'foo' since it isn't a known property of 'ng-component'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('foo'); + }); + + it('should check @HostListener decorator that does not pass enough arguments', () => { + env.write( + 'test.ts', + ` + import {Component, HostListener} from '@angular/core'; + + @Component({template: ''}) + export class Comp { + @HostListener('click') handleClick(event: MouseEvent) {}; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Expected 1 arguments, but got 0.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('handleClick'); + }); + + it('should check @HostListener decorator that passes too many arguments', () => { + env.write( + 'test.ts', + ` + import {Component, HostListener} from '@angular/core'; + + @Component({template: ''}) + export class Comp { + @HostListener('click', ['$event']) handleClick() {}; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Expected 0 arguments, but got 1.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('$event'); + }); + + it('should infer the type of @HostListener parameters', () => { + env.write( + 'test.ts', + ` + import {Component, HostListener} from '@angular/core'; + + @Component({template: ''}) + export class Comp { + @HostListener('click', ['$event']) handleClick(value: string) {}; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe( + `Argument of type 'Event' is not assignable to parameter of type 'string'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('$event'); + }); + + it('should ignore @HostListener parameters that are not static', () => { + env.write( + 'test.ts', + ` + import {Component, HostListener} from '@angular/core'; + + const two = 'null'; + + @Component({template: ''}) + export class Comp { + @HostListener('click', ['1', two, '3']) handleClick(value: string) {}; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe('Expected 1 arguments, but got 3.'); + expect(getDiagnosticSourceCode(diags[0])).toBe('two'); + }); + + it('should report host decorators on private members', () => { + env.write( + 'test.ts', + ` + import {Component, HostBinding, HostListener} from '@angular/core'; + + @Component({template: ''}) + export class Comp { + @HostBinding() private id = '123'; + @HostListener('click') private handleClick() {}; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText).toBe( + `Property 'id' is private and only accessible within class 'Comp'.`, + ); + expect(diags[1].messageText).toBe( + `Property 'handleClick' is private and only accessible within class 'Comp'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('id'); + expect(getDiagnosticSourceCode(diags[1])).toBe('handleClick'); + }); + + it('should report diagnostic on the entire initializer of property binding if node contains escaped string', () => { + env.write( + 'test.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({ + host: { + '[attr.id]': 'prefix + \\'123\\' + doesNotExist' + }, + }) + export class Dir { + prefix = 'prefix'; + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`Property 'doesNotExist' does not exist on type 'Dir'.`); + expect(getDiagnosticSourceCode(diags[0])).toBe(`'prefix + \\'123\\' + doesNotExist'`); + }); + + it('should report diagnostic on the entire initializer of event binding if node contains escaped string', () => { + env.write( + 'test.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({ + host: { + '(click)': 'handleClick(\\'foo\\')' + }, + }) + export class Dir { + handleClick(value: number) {} + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe( + `Argument of type 'string' is not assignable to parameter of type 'number'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe(`'handleClick(\\'foo\\')'`); + }); + + it('should preserve diagnostic location of nodes that occur before escaped string', () => { + env.write( + 'test.ts', + ` + import {Directive} from '@angular/core'; + + @Directive({ + host: { + '[attr.id]': 'getIdFromString(123) + \\'foo\\'' + }, + }) + export class Dir { + getIdFromString(value: string) { + return value; + } + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe( + `Argument of type 'number' is not assignable to parameter of type 'string'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('123'); + }); + + it('should not check non-static host bindings', () => { + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + const BINDINGS = {'[attr.id]': 'doesNotExist'}; + + @Component({ + template: '', + host: { + ...BINDINGS, + } + }) + export class Comp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should check host bindings on a directive', () => { + env.write( + 'dir.ts', + ` + import {Directive, HostBinding, HostListener} from '@angular/core'; + + @Directive({ + selector: 'button[dir]', + host: { + '[attr.id]': 'exists + literalBindingDoesNotExist', + '(click)': 'literalListenerDoesNotExist($event)', + } + }) + export class SomeDir { + exists = 'exists'; + + @HostBinding('foo') doesNotExistDecorator = 123; + + @HostListener('mousedown') + directiveDecoratorHostListener(event: Event) {} + } + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(4); + expect(diags[0].messageText).toBe( + `Property 'literalBindingDoesNotExist' does not exist on type 'SomeDir'.`, + ); + expect(getDiagnosticSourceCode(diags[0])).toBe('literalBindingDoesNotExist'); + expect(diags[1].messageText).toBe( + `Property 'literalListenerDoesNotExist' does not exist on type 'SomeDir'.`, + ); + expect(getDiagnosticSourceCode(diags[1])).toBe('literalListenerDoesNotExist'); + expect(diags[2].messageText).toBe(`Expected 1 arguments, but got 0.`); + expect(getDiagnosticSourceCode(diags[2])).toBe('directiveDecoratorHostListener'); + expect(diags[3].messageText).toBe( + `Can't bind to 'foo' since it isn't a known property of 'button'.`, + ); + expect(getDiagnosticSourceCode(diags[3])).toBe('foo'); + }); + + it('should not check host bindings if the compiler flag is disabled', () => { + env.tsconfig({typeCheckHostBindings: false}); + env.write( + 'test.ts', + ` + import {Component} from '@angular/core'; + + @Component({ + template: '', + host: {'[attr.id]': 'exists + doesNotExist'}, + }) + export class Comp { + exists = 'exists'; + } + `, + ); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + }); +});