From 400911e3b8d3c36ff788495bd7ecfd75b197c549 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 13 May 2024 12:07:04 +0000 Subject: [PATCH] fix(compiler-cli): do not throw when retrieving TCB symbol for signal input with restricted access (#55774) Currently when attempting to retrieve a TCB symbol for an input binding that refers to a signal input with e.g. `protected`, while the `honorAccessModifiersForInputBindings` flag is `false`, Angular will throw a runtime exception because the symbol retrieval code always expects a proper field access in the TCB. This is not the case with `honorAccessModifiersForInputBindings = false`, as TCB will allocate a temporary variable when ignoring the field access. This will then trigger the runtime exception (which we added to flag such "unexpected" cases). This commit handles it gracefully, as it's valid TCB, but we simply cannot generate a proper TCB symbol (yet). This is similar to `@Input` decorator inputs. In the future we may implement logic to build up TCB symbols for non-property access bindings, for both signal inputs or `@Input` inputs. This commit just avoids a build exception. Related to: #54324. PR Close #55774 --- .../typecheck/src/template_symbol_builder.ts | 28 +++++--- ...ecker__get_symbol_of_template_node_spec.ts | 66 +++++++++++++++++++ 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts index 8ce6ed6bdf777..4f89330b323fd 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts @@ -416,6 +416,7 @@ export class SymbolBuilder { } const signalInputAssignment = unwrapSignalInputWriteTAccessor(node.left); + let fieldAccessExpr: ts.PropertyAccessExpression | ts.ElementAccessExpression; let symbolInfo: TsNodeSymbolInfo | null = null; // Signal inputs need special treatment because they are generated with an extra keyed @@ -424,9 +425,18 @@ export class SymbolBuilder { // - The definition symbol of the input should be the input class member, and not the // internal write accessor. Symbol should resolve `_t1.prop`. if (signalInputAssignment !== null) { + // Note: If the field expression for the input binding refers to just an identifier, + // then we are handling the case of a temporary variable being used for the input field. + // This is the case with `honorAccessModifiersForInputBindings = false` and in those cases + // we cannot resolve the owning directive, similar to how we guard above with `isAccessExpression`. + if (ts.isIdentifier(signalInputAssignment.fieldExpr)) { + continue; + } + const fieldSymbol = this.getSymbolOfTsNode(signalInputAssignment.fieldExpr); const typeSymbol = this.getSymbolOfTsNode(signalInputAssignment.typeExpr); + fieldAccessExpr = signalInputAssignment.fieldExpr; symbolInfo = fieldSymbol === null || typeSymbol === null ? null @@ -436,6 +446,7 @@ export class SymbolBuilder { tsType: typeSymbol.tsType, }; } else { + fieldAccessExpr = node.left; symbolInfo = this.getSymbolOfTsNode(node.left); } @@ -443,10 +454,7 @@ export class SymbolBuilder { continue; } - const target = this.getDirectiveSymbolForAccessExpression( - signalInputAssignment?.fieldExpr ?? node.left, - consumer, - ); + const target = this.getDirectiveSymbolForAccessExpression(fieldAccessExpr, consumer); if (target === null) { continue; } @@ -771,7 +779,7 @@ function sourceSpanEqual(a: ParseSourceSpan, b: ParseSourceSpan) { } function unwrapSignalInputWriteTAccessor(expr: ts.LeftHandSideExpression): null | { - fieldExpr: ts.ElementAccessExpression | ts.PropertyAccessExpression; + fieldExpr: ts.ElementAccessExpression | ts.PropertyAccessExpression | ts.Identifier; typeExpr: ts.ElementAccessExpression; } { // e.g. `_t2.inputA[i2.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]` @@ -793,11 +801,15 @@ function unwrapSignalInputWriteTAccessor(expr: ts.LeftHandSideExpression): null return null; } - // Assert that the `_t2.inputA` is actually either a keyed element access, or - // property access expression. This is checked for type safety and to catch unexpected cases. + // Assert that the expression is either: + // - `_t2.inputA[ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]` or (common case) + // - or `_t2['input-A'][ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]` (non-identifier input field names) + // - or `_dirInput[ɵINPUT_SIGNAL_BRAND_WRITE_TYPE` (honorAccessModifiersForInputBindings=false) + // This is checked for type safety and to catch unexpected cases. if ( !ts.isPropertyAccessExpression(expr.expression) && - !ts.isElementAccessExpression(expr.expression) + !ts.isElementAccessExpression(expr.expression) && + !ts.isIdentifier(expr.expression) ) { throw new Error('Unexpected expression for signal input write type.'); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts index 90084ce62d825..2d33472fafe15 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts @@ -1294,6 +1294,72 @@ runInEachFileSystem(() => { ).toEqual('inputB'); }); + // Note that `honorAccessModifiersForInputBindings` is `false` even with `--strictTemplates`, + // so this captures a potential common scenario, assuming the input is restricted. + it('should not throw when retrieving a symbol for a signal-input with restricted access', () => { + const fileName = absoluteFrom('/main.ts'); + const dirFile = absoluteFrom('/dir.ts'); + const templateString = ` + @if (true) { +
+ } + `; + const {program, templateTypeChecker} = setup( + [ + { + fileName, + templates: {'Cmp': templateString}, + declarations: [ + { + name: 'TestDir', + selector: '[dir]', + file: dirFile, + type: 'directive', + restrictedInputFields: ['inputA'], + inputs: { + inputA: { + bindingPropertyName: 'inputA', + isSignal: true, + classPropertyName: 'inputA', + required: false, + transform: null, + }, + }, + }, + ], + }, + { + fileName: dirFile, + source: ` + import {InputSignal} from '@angular/core'; + + export class TestDir { + protected inputA: InputSignal = null!; + } + `, + templates: {}, + }, + ], + {honorAccessModifiersForInputBindings: false}, + ); + const sf = getSourceFileOrError(program, fileName); + const cmp = getClass(sf, 'Cmp'); + + const nodes = templateTypeChecker.getTemplate(cmp)!; + const ifNode = nodes[0] as TmplAstIfBlock; + const ifBranchNode = ifNode.branches[0]; + const testElement = ifBranchNode.children[0] as TmplAstElement; + + const inputAbinding = testElement.inputs[0]; + const aSymbol = templateTypeChecker.getSymbolOfNode(inputAbinding, cmp); + expect(aSymbol) + .withContext( + 'Symbol builder does not return symbols for restricted inputs with ' + + '`honorAccessModifiersForInputBindings = false` (same for decorator inputs)', + ) + .toBe(null); + }); + it('does not retrieve a symbol for an input when undeclared', () => { const fileName = absoluteFrom('/main.ts'); const dirFile = absoluteFrom('/dir.ts');