From 7ff3ea3667b0777ecc7e45a174c225bbc0a8aba1 Mon Sep 17 00:00:00 2001 From: cexbrayat Date: Wed, 8 Apr 2026 10:30:03 +0200 Subject: [PATCH] fix(compiler-cli): detect uninvoked signals in bound expressions using ternary Extend the interpolated signal extended diagnostic to inspect ternary-bound expressions and report uninvoked signal reads in bound bindings. ```
``` where the false branch should invoke the signal as `width()`. --- .../interpolated_signal_not_invoked/index.ts | 49 +++++-- .../interpolated_signal_not_invoked_spec.ts | 124 ++++++++++++++++++ 2 files changed, 162 insertions(+), 11 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts index 5060321e19a2..d30b4ee55dc1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts @@ -10,7 +10,10 @@ import { AST, ASTWithSource, BindingType, + Conditional, Interpolation, + NonNullAssert, + ParenthesizedExpression, PrefixNot, PropertyRead, TmplAstBoundAttribute, @@ -120,7 +123,11 @@ function checkBoundAttribute( } // otherwise, we check if the node is - const nodeAst = isPropertyReadNodeAst(node); + if (node.value instanceof ASTWithSource === false) { + return []; + } + const propertyReads = getPropertyReads(node.value.ast); + if ( // a bound property like `[prop]="mySignal"` (node.type === BindingType.Property || @@ -134,25 +141,45 @@ function checkBoundAttribute( node.type === BindingType.Animation || // or an animation binding like `[@myAnimation]="mySignal"` node.type === BindingType.LegacyAnimation) && - nodeAst + propertyReads.length > 0 ) { - return buildDiagnosticForSignal(ctx, nodeAst, component); + return propertyReads.flatMap((nodeAst) => buildDiagnosticForSignal(ctx, nodeAst, component)); } return []; } -function isPropertyReadNodeAst(node: TmplAstBoundAttribute): PropertyRead | undefined { - if (node.value instanceof ASTWithSource === false) { - return undefined; +function getPropertyReads(ast: AST): PropertyRead[] { + // Handle unary negation, such as `!mySignal`. + if (ast instanceof PrefixNot) { + return ast.expression instanceof PropertyRead ? [ast.expression] : []; } - if (node.value.ast instanceof PrefixNot && node.value.ast.expression instanceof PropertyRead) { - return node.value.ast.expression; + + // Handle direct reads, such as `mySignal`. + if (ast instanceof PropertyRead) { + return [ast]; } - if (node.value.ast instanceof PropertyRead) { - return node.value.ast; + + // Handle ternary expressions, such as `flag ? mySignal : otherSignal`. + if (ast instanceof Conditional) { + return [ + ...getPropertyReads(ast.condition), + ...getPropertyReads(ast.trueExp), + ...getPropertyReads(ast.falseExp), + ]; } - return undefined; + + // Handle parenthesized expressions, such as `(mySignal)`. + if (ast instanceof ParenthesizedExpression) { + return getPropertyReads(ast.expression); + } + + // Handle non-null assertions, such as `mySignal!`. + if (ast instanceof NonNullAssert) { + return getPropertyReads(ast.expression); + } + + return []; } function isFunctionInstanceProperty(name: string): boolean { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts index 342ea892d915..254672c89891 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts @@ -765,6 +765,130 @@ runInEachFileSystem(() => { }); }); + it('should produce a warning when a signal is not invoked in a conditional style binding branch', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + }, + source: ` + import {signal} from '@angular/core'; + + export class TestCmp { + width = signal(0); + }`, + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [interpolatedSignalFactory], + {} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe(`width`); + }); + + it('should produce a warning when a signal is not invoked in a parenthesized style binding expression', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + }, + source: ` + import {signal} from '@angular/core'; + + export class TestCmp { + width = signal(0); + }`, + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [interpolatedSignalFactory], + {} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe(`width`); + }); + + it('should produce a warning when a signal is not invoked in a non-null asserted style binding expression', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + }, + source: ` + import {signal} from '@angular/core'; + + export class TestCmp { + width = signal(0); + }`, + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [interpolatedSignalFactory], + {} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe(`width`); + }); + + it('should produce a warning when a signal is not invoked in a prefixed-not property binding expression', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': ``, + }, + source: ` + import {signal} from '@angular/core'; + + export class TestCmp { + isEnabled = signal(true); + }`, + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [interpolatedSignalFactory], + {} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe(`isEnabled`); + }); + it('should not produce a warning with other Signal type', () => { const fileName = absoluteFrom('/main.ts'); const {program, templateTypeChecker} = setup([