diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts index 10d0f60c682..71bea4d3c07 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, LiteralPrimitive, ParseSourceSpan, PropertyRead, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstTemplate, TmplAstTextAttribute} from '@angular/compiler'; +import {AST, LiteralPrimitive, ParseSourceSpan, PropertyRead, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import ts from 'typescript'; import {AbsoluteFsPath} from '../../../../src/ngtsc/file_system'; @@ -213,6 +213,12 @@ export interface TemplateTypeChecker { */ invalidateClass(clazz: ts.ClassDeclaration): void; + /** + * Gets the target of a template expression, if possible. + */ + getExpressionTarget(expression: AST, clazz: ts.ClassDeclaration): TmplAstReference|TmplAstVariable + |null; + /** * Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template. */ diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index c61bda38d4b..2f9e6e56d77 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, CssSelector, DomElementSchemaRegistry, ExternalExpr, LiteralPrimitive, ParseSourceSpan, PropertyRead, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstTemplate, TmplAstTextAttribute, WrappedNodeExpr} from '@angular/compiler'; +import {AST, CssSelector, DomElementSchemaRegistry, ExternalExpr, LiteralPrimitive, ParseSourceSpan, PropertyRead, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable, WrappedNodeExpr} from '@angular/compiler'; import ts from 'typescript'; import {ErrorCode, ngErrorCode} from '../../diagnostics'; @@ -345,6 +345,12 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { this.isComplete = false; } + getExpressionTarget(expression: AST, clazz: ts.ClassDeclaration): TmplAstReference|TmplAstVariable + |null { + return this.getLatestComponentState(clazz).data?.boundTarget.getExpressionTarget(expression) || + null; + } + makeTemplateDiagnostic( clazz: ts.ClassDeclaration, sourceSpan: ParseSourceSpan, category: ts.DiagnosticCategory, errorCode: T, message: string, relatedInformation?: { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index c51547ab6ec..f8862501949 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -70,9 +70,6 @@ export interface OutOfBandDiagnosticRecorder { */ deferredComponentUsedEagerly(templateId: TemplateId, element: TmplAstElement): void; - illegalAssignmentToTemplateVar( - templateId: TemplateId, assignment: PropertyWrite, target: TmplAstVariable): void; - /** * Reports a duplicate declaration of a template variable. * @@ -217,27 +214,6 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor ngErrorCode(ErrorCode.DEFERRED_DIRECTIVE_USED_EAGERLY), errorMsg)); } - illegalAssignmentToTemplateVar( - templateId: TemplateId, assignment: PropertyWrite, target: TmplAstVariable): void { - const mapping = this.resolver.getSourceMapping(templateId); - const errorMsg = `Cannot use variable '${ - assignment - .name}' as the left-hand side of an assignment expression. Template variables are read-only.`; - - const sourceSpan = this.resolver.toParseSourceSpan(templateId, assignment.sourceSpan); - if (sourceSpan === null) { - throw new Error(`Assertion failure: no SourceLocation found for property binding.`); - } - this._diagnostics.push(makeTemplateDiagnostic( - templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error, - ngErrorCode(ErrorCode.WRITE_TO_READ_ONLY_VARIABLE), errorMsg, [{ - text: `The variable ${assignment.name} is declared here.`, - start: target.valueSpan?.start.offset || target.sourceSpan.start.offset, - end: target.valueSpan?.end.offset || target.sourceSpan.end.offset, - sourceFile: mapping.node.getSourceFile(), - }])); - } - duplicateTemplateVar( templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void { const mapping = this.resolver.getSourceMapping(templateId); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_semantics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_semantics.ts deleted file mode 100644 index 5e8a7bbb6cf..00000000000 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_semantics.ts +++ /dev/null @@ -1,44 +0,0 @@ -/** - * @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.io/license - */ - -import {AST, BoundTarget, ImplicitReceiver, PropertyWrite, RecursiveAstVisitor, TmplAstVariable} from '@angular/compiler'; - -import {TemplateId} from '../api'; - -import {OutOfBandDiagnosticRecorder} from './oob'; - -/** - * Visits a template and records any semantic errors within its expressions. - */ -export class ExpressionSemanticVisitor extends RecursiveAstVisitor { - constructor( - private templateId: TemplateId, private boundTarget: BoundTarget, - private oob: OutOfBandDiagnosticRecorder) { - super(); - } - - override visitPropertyWrite(ast: PropertyWrite, context: any): void { - super.visitPropertyWrite(ast, context); - - if (!(ast.receiver instanceof ImplicitReceiver)) { - return; - } - - const target = this.boundTarget.getExpressionTarget(ast); - if (target instanceof TmplAstVariable) { - // Template variables are read-only. - this.oob.illegalAssignmentToTemplateVar(this.templateId, ast, target); - } - } - - static visit( - ast: AST, id: TemplateId, boundTarget: BoundTarget, - oob: OutOfBandDiagnosticRecorder): void { - ast.visit(new ExpressionSemanticVisitor(id, boundTarget, oob)); - } -} 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 540f3185b38..6c2f236a8c9 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 @@ -20,7 +20,6 @@ import {DomSchemaChecker} from './dom'; import {Environment} from './environment'; import {astToTypescript, NULL_AS_ANY} from './expression'; import {OutOfBandDiagnosticRecorder} from './oob'; -import {ExpressionSemanticVisitor} from './template_semantics'; import {tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util'; import {requiresInlineTypeCtor} from './type_constructor'; import {TypeParameterEmitter} from './type_parameter_emitter'; @@ -1214,9 +1213,6 @@ export class TcbDirectiveOutputsOp extends TcbOp { const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Any); this.scope.addStatement(ts.factory.createExpressionStatement(handler)); } - - ExpressionSemanticVisitor.visit( - output.handler, this.tcb.id, this.tcb.boundTarget, this.tcb.oobRecorder); } return null; @@ -1292,9 +1288,6 @@ class TcbUnclaimedOutputsOp extends TcbOp { const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Any); this.scope.addStatement(ts.factory.createExpressionStatement(handler)); } - - ExpressionSemanticVisitor.visit( - output.handler, this.tcb.id, this.tcb.boundTarget, this.tcb.oobRecorder); } return null; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/template_semantics/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/template_semantics/BUILD.bazel index 0b05934cd7f..eb04f3fff19 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/template_semantics/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/template_semantics/BUILD.bazel @@ -10,6 +10,7 @@ ts_library( "//packages/compiler", "//packages/compiler-cli/src/ngtsc/core:api", "//packages/compiler-cli/src/ngtsc/diagnostics", + "//packages/compiler-cli/src/ngtsc/typecheck", "//packages/compiler-cli/src/ngtsc/typecheck/api", "//packages/compiler-cli/src/ngtsc/typecheck/template_semantics/api", "@npm//typescript", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/template_semantics/src/template_semantics_checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/template_semantics/src/template_semantics_checker.ts index 02fe1097952..b275576bd6a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/template_semantics/src/template_semantics_checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/template_semantics/src/template_semantics_checker.ts @@ -6,9 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ +import {ImplicitReceiver, PropertyWrite, RecursiveAstVisitor, TmplAstBoundEvent, TmplAstNode, TmplAstRecursiveVisitor, TmplAstVariable} from '@angular/compiler'; import ts from 'typescript'; -import {DiagnosticCategoryLabel} from '../../../core/api'; +import {ErrorCode, ngErrorCode} from '../../../diagnostics'; import {TemplateDiagnostic, TemplateTypeChecker} from '../../api'; import {TemplateSemanticsChecker} from '../api/api'; @@ -17,12 +18,69 @@ export class TemplateSemanticsCheckerImpl implements TemplateSemanticsChecker { getDiagnosticsForComponent(component: ts.ClassDeclaration): TemplateDiagnostic[] { const template = this.templateTypeChecker.getTemplate(component); - - if (template === null) { - return []; - } - - const diagnostics: TemplateDiagnostic[] = []; - return diagnostics; + return template !== null ? + TemplateSemanticsVisitor.visit(template, component, this.templateTypeChecker) : + []; + } +} + +/** Visitor that verifies the semantics of a template. */ +class TemplateSemanticsVisitor extends TmplAstRecursiveVisitor { + private constructor(private expressionVisitor: ExpressionsSemanticsVisitor) { + super(); + } + + static visit( + nodes: TmplAstNode[], component: ts.ClassDeclaration, + templateTypeChecker: TemplateTypeChecker) { + const diagnostics: TemplateDiagnostic[] = []; + const expressionVisitor = + new ExpressionsSemanticsVisitor(templateTypeChecker, component, diagnostics); + const templateVisitor = new TemplateSemanticsVisitor(expressionVisitor); + nodes.forEach(node => node.visit(templateVisitor)); + return diagnostics; + } + + override visitBoundEvent(event: TmplAstBoundEvent): void { + super.visitBoundEvent(event); + event.handler.visit(this.expressionVisitor, event); + } +} + +/** Visitor that verifies the semantics of the expressions within a template. */ +class ExpressionsSemanticsVisitor extends RecursiveAstVisitor { + constructor( + private templateTypeChecker: TemplateTypeChecker, private component: ts.ClassDeclaration, + private diagnostics: TemplateDiagnostic[]) { + super(); + } + + override visitPropertyWrite(ast: PropertyWrite, context: TmplAstNode): void { + super.visitPropertyWrite(ast, context); + + if (!(context instanceof TmplAstBoundEvent) || !(ast.receiver instanceof ImplicitReceiver)) { + return; + } + + const target = this.templateTypeChecker.getExpressionTarget(ast, this.component); + if (target instanceof TmplAstVariable) { + const errorMessage = `Cannot use variable '${ + target + .name}' as the left-hand side of an assignment expression. Template variables are read-only.`; + this.diagnostics.push(this.makeIllegalTemplateVarDiagnostic(target, context, errorMessage)); + } + } + + private makeIllegalTemplateVarDiagnostic( + target: TmplAstVariable, expressionNode: TmplAstNode, + errorMessage: string): TemplateDiagnostic { + return this.templateTypeChecker.makeTemplateDiagnostic( + this.component, expressionNode.sourceSpan, ts.DiagnosticCategory.Error, + ngErrorCode(ErrorCode.WRITE_TO_READ_ONLY_VARIABLE), errorMessage, [{ + text: `The variable ${target.name} is declared here.`, + start: target.valueSpan?.start.offset || target.sourceSpan.start.offset, + end: target.valueSpan?.end.offset || target.sourceSpan.end.offset, + sourceFile: this.component.getSourceFile(), + }]); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index 45b84403ea9..ffd9d784e97 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -859,7 +859,6 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder { missingPipe(): void {} deferredPipeUsedEagerly(templateId: TemplateId, ast: BindingPipe): void {} deferredComponentUsedEagerly(templateId: TemplateId, element: TmplAstElement): void {} - illegalAssignmentToTemplateVar(): void {} duplicateTemplateVar(): void {} requiresInlineTcb(): void {} requiresInlineTypeConstructors(): void {}