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 182d54feb17..2eb7a912bdd 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 @@ -1208,6 +1208,8 @@ class TcbBlockImplicitVariableOp extends TcbOp { * Executing this operation returns nothing. */ class TcbIfOp extends TcbOp { + private expressionScopes = new Map(); + constructor(private tcb: Context, private scope: Scope, private block: TmplAstIfBlock) { super(); } @@ -1231,29 +1233,81 @@ class TcbIfOp extends TcbOp { // If the expression is null, it means that it's an `else` statement. if (branch.expression === null) { - const branchScope = Scope.forNodes(this.tcb, this.scope, null, branch.children, null); + const branchScope = Scope.forNodes( + this.tcb, this.scope, null, branch.children, this.generateBranchGuard(index)); return ts.factory.createBlock(branchScope.render()); } - let branchParentScope: Scope; + // We need to process the expression first so it gets its own scope that the body of the + // conditional will inherit from. We do this, because we need to declare a separate variable + // for the case where the expression has an alias _and_ because we need the processed + // expression when generating the guard for the body. + const expressionScope = Scope.forNodes(this.tcb, this.scope, branch, [], null); + expressionScope.render().forEach(stmt => this.scope.addStatement(stmt)); + this.expressionScopes.set(branch, expressionScope); - if (branch.expressionAlias === null) { - branchParentScope = this.scope; - } else { - // If the expression is aliased, we create a scope with a variable containing the expression. - // Further down we'll use the variable instead of the expression itself in the `if` statement. - // This allows for the type of the alias to be narrowed. - branchParentScope = Scope.forNodes(this.tcb, this.scope, branch, [], null); - branchParentScope.render().forEach(stmt => this.scope.addStatement(stmt)); - } - - const branchScope = Scope.forNodes(this.tcb, branchParentScope, null, branch.children, null); const expression = branch.expressionAlias === null ? - tcbExpression(branch.expression, this.tcb, branchScope) : - branchScope.resolve(branch.expressionAlias); + tcbExpression(branch.expression, this.tcb, expressionScope) : + expressionScope.resolve(branch.expressionAlias); + + const bodyScope = Scope.forNodes( + this.tcb, expressionScope, null, branch.children, this.generateBranchGuard(index)); return ts.factory.createIfStatement( - expression, ts.factory.createBlock(branchScope.render()), this.generateBranch(index + 1)); + expression, ts.factory.createBlock(bodyScope.render()), this.generateBranch(index + 1)); + } + + private generateBranchGuard(index: number): ts.Expression|null { + let guard: ts.Expression|null = null; + + // Since event listeners are inside callbacks, type narrowing doesn't apply to them anymore. + // To recreate the behavior, we generate an expression that negates all the values of the + // branches _before_ the current one, and then we add the current branch's expression on top. + // For example `@if (expr === 1) {} @else if (expr === 2) {} @else if (expr === 3)`, the guard + // for the last expression will be `!(expr === 1) && !(expr === 2) && expr === 3`. + for (let i = 0; i <= index; i++) { + const branch = this.block.branches[i]; + + // Skip over branches without an expression. + if (branch.expression === null) { + continue; + } + + // This shouldn't happen since all the state is handled + // internally, but we have the check just in case. + if (!this.expressionScopes.has(branch)) { + throw new Error(`Could not determine expression scope of branch at index ${i}`); + } + + const expressionScope = this.expressionScopes.get(branch)!; + let expression: ts.Expression; + + if (branch.expressionAlias === null) { + // We need to recreate the expression and mark it to be ignored for diagnostics, + // because it was already checked as a part of the block's condition and we don't + // want it to produce a duplicate diagnostic. + expression = tcbExpression(branch.expression, this.tcb, expressionScope); + markIgnoreDiagnostics(expression); + } else { + expression = expressionScope.resolve(branch.expressionAlias); + } + + // The expressions of the preceeding branches have to be negated + // (e.g. `expr` becomes `!(expr)`) when comparing in the guard, except + // for the branch's own expression which is preserved as is. + const comparisonExpression = i === index ? + expression : + ts.factory.createPrefixUnaryExpression( + ts.SyntaxKind.ExclamationToken, ts.factory.createParenthesizedExpression(expression)); + + // Finally add the expression to the guard with an && operator. + guard = guard === null ? + comparisonExpression : + ts.factory.createBinaryExpression( + guard, ts.SyntaxKind.AmpersandAmpersandToken, comparisonExpression); + } + + return guard; } } 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 628708d17ea..6d67b4399ad 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 @@ -1418,6 +1418,30 @@ describe('type check blocks', () => { 'else { "" + ((this).other()); }'); }); + it('should generate a guard expression for listener inside conditional', () => { + const TEMPLATE = ` + @if (expr === 0) { + + } @else if (expr === 1) { + + } @else if (expr === 2) { + + } @else { + + } + `; + + const result = tcb(TEMPLATE); + + expect(result).toContain(`if ((((this).expr)) === (0)) (this).zero();`); + expect(result).toContain( + `if (!((((this).expr)) === (0)) && (((this).expr)) === (1)) (this).one();`); + expect(result).toContain( + `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();`); + }); + it('should generate an if block with an `as` expression', () => { const TEMPLATE = `@if (expr === 1; as alias) { {{alias}} diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 58dd19a4342..7d3e7b2550e 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -3831,7 +3831,7 @@ suppress ]); }); - it('should check narrow the type in the alias', () => { + it('should narrow the type of the if alias', () => { env.write('test.ts', ` import {Component} from '@angular/core'; @@ -3856,6 +3856,31 @@ suppress ]); }); + it('should narrow the type of the if alias used in a listener', () => { + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: \`@if (value; as alias) { + + }\`, + standalone: true, + }) + export class Main { + value: 'one' | 0 = 0; + + acceptsNumber(value: number) { + return value; + } + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([ + `Argument of type 'string' is not assignable to parameter of type 'number'.`, + ]); + }); + it('should not expose the aliased expression outside of the main block', () => { env.write('test.ts', ` import {Component} from '@angular/core'; @@ -3939,6 +3964,93 @@ suppress ]); }); + it('should narrow the type in listeners inside if blocks', () => { + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: \` + @if (expr === 'hello') { + + } + \`, + standalone: true, + }) + export class Main { + expr: 'hello' | 1 = 'hello'; + + acceptsNumber(value: number) { + return value; + } + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([ + `Argument of type 'string' is not assignable to parameter of type 'number'.`, + ]); + }); + + it('should narrow the type in listeners inside else if blocks', () => { + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: \` + @if (expr === 1) { + One + } @else if (expr === 'hello') { + + } + \`, + standalone: true, + }) + export class Main { + expr: 'hello' | 1 | 2 = 'hello'; + + acceptsNumber(value: number) { + return value; + } + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([ + `Argument of type 'string' is not assignable to parameter of type 'number'.`, + ]); + }); + + it('should narrow the type in listeners inside else blocks', () => { + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: \` + @if (expr === 1) { + One + } @else if (expr === 2) { + Two + } @else { + + } + \`, + standalone: true, + }) + export class Main { + expr: 'hello' | 1 | 2 = 'hello'; + + acceptsNumber(value: number) { + return value; + } + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([ + `Argument of type 'string' is not assignable to parameter of type 'number'.`, + ]); + }); + it('should check bindings inside switch blocks', () => { env.write('test.ts', ` import {Component} from '@angular/core'; @@ -4126,6 +4238,30 @@ suppress `Argument of type 'string' is not assignable to parameter of type 'number'.` ]); }); + + it('should produce a single diagnostic for an invalid expression of a block containing a event listener', + () => { + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: \` + @if (does_not_exist) { + + } + \`, + standalone: true, + }) + export class Main { + test() {} + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([ + `Property 'does_not_exist' does not exist on type 'Main'.`, + ]); + }); }); describe('for loop blocks', () => {