From f28b8f621da350c5aa14cb79a144ef055ff841bb Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 3 Mar 2026 13:07:38 +0100 Subject: [PATCH] refactor(compiler-cli): resolve presubmit issues Resolves issues caught during the presubmit. --- .../src/ngtsc/typecheck/src/expression.ts | 23 +++++++++------ .../src/ngtsc/typecheck/src/ops/bindings.ts | 4 +-- .../src/ngtsc/typecheck/src/ops/codegen.ts | 2 +- .../src/ngtsc/typecheck/src/ops/if_block.ts | 2 +- .../src/ngtsc/typecheck/src/ops/scope.ts | 2 +- .../ngtsc/typecheck/src/ops/switch_block.ts | 4 +-- .../typecheck/test/type_check_block_spec.ts | 28 ++++++++++--------- 7 files changed, 37 insertions(+), 28 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index cebaeedef0b..c1b199c28db 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts @@ -86,7 +86,10 @@ class TcbExprTranslator implements AstVisitor { const rhs = this.translate(ast.right); lhs.wrapForTypeChecker(); rhs.wrapForTypeChecker(); - const node = new TcbExpr(`${lhs.print()} ${ast.operation} ${rhs.print()}`); + const expression = `${lhs.print()} ${ast.operation} ${rhs.print()}`; + const node = new TcbExpr( + ast.operation === '??' || ast.operation === '**' ? `(${expression})` : expression, + ); node.addParseSpanInfo(ast.sourceSpan); return node; } @@ -150,14 +153,14 @@ class TcbExprTranslator implements AstVisitor { visitLiteralArray(ast: LiteralArray): TcbExpr { const elements = ast.expressions.map((expr) => this.translate(expr)); - let content = `[${elements.map((el) => el.print()).join(', ')}]`; + let literal = `[${elements.map((el) => el.print()).join(', ')}]`; // If strictLiteralTypes is disabled, array literals are cast to `any`. if (!this.config.strictLiteralTypes) { - content += ' as any'; + literal = `(${literal} as any)`; } - return new TcbExpr(content).addParseSpanInfo(ast.sourceSpan); + return new TcbExpr(literal).addParseSpanInfo(ast.sourceSpan); } visitLiteralMap(ast: LiteralMap): TcbExpr { @@ -173,10 +176,14 @@ class TcbExprTranslator implements AstVisitor { } }); - // If strictLiteralTypes is disabled, object literals are cast to `any`. - return new TcbExpr( - `{ ${properties.join(', ')} }${this.config.strictLiteralTypes ? '' : ' as any'}`, - ).addParseSpanInfo(ast.sourceSpan); + let literal = `{ ${properties.join(', ')} }`; + + // If strictLiteralTypes is disabled, array literals are cast to `any`. + if (!this.config.strictLiteralTypes) { + literal = `(${literal} as any)`; + } + + return new TcbExpr(literal).addParseSpanInfo(ast.sourceSpan); } visitLiteralPrimitive(ast: LiteralPrimitive): TcbExpr { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/bindings.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/bindings.ts index f08094ceb6f..0f8986fbdf1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/bindings.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/bindings.ts @@ -188,7 +188,7 @@ export function widenBinding(expr: TcbExpr, tcb: Context, originalValue: string if (!tcb.env.config.checkTypeOfInputBindings) { // If checking the type of bindings is disabled, cast the resulting expression to 'any' // before the assignment. - return new TcbExpr(`(${expr.print()} as any)`); + return new TcbExpr(`((${expr.print()}) as any)`); } else if (!tcb.env.config.strictNullInputBindings) { if (originalValue instanceof LiteralMap || originalValue instanceof LiteralArray) { // Object literals and array literals should not be wrapped in non-null assertions as that @@ -198,7 +198,7 @@ export function widenBinding(expr: TcbExpr, tcb: Context, originalValue: string } else { // If strict null checks are disabled, erase `null` and `undefined` from the type by // wrapping the expression in a non-null assertion. - return new TcbExpr(`${expr.print()}!`); + return new TcbExpr(`(${expr.print()})!`); } } else { // No widening is requested, use the expression as is. diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/codegen.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/codegen.ts index 91f1baba836..3d8bbe31e76 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/codegen.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/codegen.ts @@ -28,7 +28,7 @@ export class TcbExpr { /** * Converts the node's current state to a string. - * @param ignoreComments Whether the comments associated with the expresion should be skipped. + * @param ignoreComments Whether the comments associated with the expression should be skipped. */ print(ignoreComments = false): string { if (ignoreComments) { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/if_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/if_block.ts index f03b837976c..76ca25efad4 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/if_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/if_block.ts @@ -129,7 +129,7 @@ export class TcbIfOp extends TcbOp { guard = guard === null ? comparisonExpression - : new TcbExpr(`${guard.print()} && ${comparisonExpression.print()}`); + : new TcbExpr(`(${guard.print()}) && (${comparisonExpression.print()})`); } return guard; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/scope.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/scope.ts index 0c38aefd97f..213a7335a76 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/scope.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/scope.ts @@ -355,7 +355,7 @@ export class Scope { // It is important that the parent guard is used as left operand, given that it may provide // narrowing that is required for this scope's guard to be valid. const guard = typeof this.guard === 'string' ? this.guard : this.guard.print(); - return new TcbExpr(`${parentGuards.print()} && ${guard}`); + return new TcbExpr(`(${parentGuards.print()}) && (${guard})`); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/switch_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/switch_block.ts index 2f779015b2d..7798a78efde 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ops/switch_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ops/switch_block.ts @@ -95,7 +95,7 @@ export class TcbSwitchOp extends TcbOp { if (guard === null) { guard = comparison; } else { - guard = new TcbExpr(`${guard.print()} || ${comparison.print()}`); + guard = new TcbExpr(`(${guard.print()}) || (${comparison.print()})`); } } } @@ -132,7 +132,7 @@ export class TcbSwitchOp extends TcbOp { if (guard === null) { guard = comparison; } else { - guard = new TcbExpr(`${guard.print()} && ${comparison.print()}`); + guard = new TcbExpr(`(${guard.print()}) && (${comparison.print()})`); } } } 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 4d928317e3e..a51d93b7be1 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 @@ -69,9 +69,11 @@ describe('type check blocks', () => { it('should handle nullish coalescing operator', () => { expect(tcb('{{ a ?? b }}')).toContain('((((this).a)) ?? (((this).b)))'); - expect(tcb('{{ a ?? b ?? c }}')).toContain('(((((this).a)) ?? (((this).b))) ?? (((this).c)))'); + expect(tcb('{{ a ?? b ?? c }}')).toContain( + '((((((this).a)) ?? (((this).b)))) ?? (((this).c)))', + ); expect(tcb('{{ (a ?? b) + (c ?? e) }}')).toContain( - '((((((this).a)) ?? (((this).b)))) + (((((this).c)) ?? (((this).e)))))', + '(((((((this).a)) ?? (((this).b))))) + ((((((this).c)) ?? (((this).e))))))', ); }); @@ -104,9 +106,9 @@ describe('type check blocks', () => { it('should handle exponentiation expressions', () => { expect(tcb('{{a * b ** c + d}}')).toContain( - '(((((this).a)) * ((((this).b)) ** (((this).c)))) + (((this).d)))', + '(((((this).a)) * (((((this).b)) ** (((this).c))))) + (((this).d)))', ); - expect(tcb('{{a ** b ** c}}')).toContain('((((this).a)) ** ((((this).b)) ** (((this).c))))'); + expect(tcb('{{a ** b ** c}}')).toContain('((((this).a)) ** (((((this).b)) ** (((this).c)))))'); }); it('should handle "in" expressions', () => { @@ -1195,8 +1197,8 @@ describe('type check blocks', () => { strictNullInputBindings: false, }; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('_t1.dirInput = (((this).a)!);'); - expect(block).toContain('((this).b)!;'); + expect(block).toContain('_t1.dirInput = ((((this).a))!);'); + expect(block).toContain('(((this).b))!;'); }); }); @@ -1215,8 +1217,8 @@ describe('type check blocks', () => { checkTypeOfInputBindings: false, }; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('_t1.dirInput = ((((this).a) as any));'); - expect(block).toContain('(((this).b) as any);'); + expect(block).toContain('_t1.dirInput = (((((this).a)) as any));'); + expect(block).toContain('((((this).b)) as any);'); }); it('should wrap the cast to any in parentheses when required', () => { @@ -1226,7 +1228,7 @@ describe('type check blocks', () => { checkTypeOfInputBindings: false, }; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('_t1.dirInput = (((((this).a)) === (((this).b)) as any));'); + expect(block).toContain('_t1.dirInput = ((((((this).a)) === (((this).b))) as any));'); }); }); @@ -1920,13 +1922,13 @@ describe('type check blocks', () => { expect(result).toContain(`if ((((this).expr)) === (0)) { (this).zero(); }`); expect(result).toContain( - `if (!((((this).expr)) === (0)) && (((this).expr)) === (1)) { (this).one(); }`, + `if ((!((((this).expr)) === (0))) && ((((this).expr)) === (1))) { (this).one(); }`, ); expect(result).toContain( - `if (!((((this).expr)) === (0)) && !((((this).expr)) === (1)) && (((this).expr)) === (2)) { (this).two(); }`, + `if (((!((((this).expr)) === (0))) && (!((((this).expr)) === (1)))) && ((((this).expr)) === (2))) { (this).two(); }`, ); expect(result).toContain( - `if (!((((this).expr)) === (0)) && !((((this).expr)) === (1)) && !((((this).expr)) === (2))) { (this).otherwise(); }`, + `if (((!((((this).expr)) === (0))) && (!((((this).expr)) === (1)))) && (!((((this).expr)) === (2)))) { (this).otherwise(); }`, ); }); @@ -2089,7 +2091,7 @@ describe('type check blocks', () => { expect(result).toContain(`if (((this).expr) === 1) { (this).one(); }`); expect(result).toContain(`if (((this).expr) === 2) { (this).two(); }`); expect(result).toContain( - `if (((this).expr) !== 1 && ((this).expr) !== 2) { (this).default(); }`, + `if ((((this).expr) !== 1) && (((this).expr) !== 2)) { (this).default(); }`, ); });