fix(compiler): template type checking not reporting diagnostics for incompatible type comparisons (#52322)

crisbeto · dylhunn · commit dc3f7cb3bfc4 · 2023-10-23T09:27:23.000-07:00
In #52110 the compiler was changed to produce `if` statements when type checking `@switch` in order to avoid a bug in the TypeScript compiler. In order to avoid duplicate diagnostics, the main `@switch` expression was ignored in each of the `@case` comparisons. This appears to have caused a regression where comparing incompatible types wasn't being reported anymore. These changes resolve the issue by wrapping the expression in parentheses which allows the compiler to report comparison diagnostics while ignoring diagnostics in the expression itself. Fixes #52315. PR Close #52322
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
@@ -1327,12 +1327,15 @@ class TcbSwitchOp extends TcbOp {
   }
 
   override execute(): null {
-    const expression = tcbExpression(this.block.expression, this.tcb, this.scope);
-
     // Since we'll use the expression in multiple comparisons, we don't want to
     // log the same diagnostic more than once. Ignore this expression since we already
     // produced a `TcbExpressionOp`.
-    markIgnoreDiagnostics(expression);
+    const comparisonExpression = tcbExpression(this.block.expression, this.tcb, this.scope);
+    markIgnoreDiagnostics(comparisonExpression);
+
+    // Wrap the comparisson expression in parentheses so we don't ignore
+    // diagnostics when comparing incompatible types (see #52315).
+    const expression = ts.factory.createParenthesizedExpression(comparisonExpression);
     const root = this.generateCase(0, expression, null);
 
     if (root !== undefined) {
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
@@ -1468,8 +1468,8 @@ describe('type check blocks', () => {
 
       expect(tcb(TEMPLATE))
           .toContain(
-              'if (((this).expr) === 1) { "" + ((this).one()); } else if ' +
-              '(((this).expr) === 2) { "" + ((this).two()); } else { "" + ((this).default()); }');
+              'if ((((this).expr)) === 1) { "" + ((this).one()); } else if ' +
+              '((((this).expr)) === 2) { "" + ((this).two()); } else { "" + ((this).default()); }');
     });
 
     it('should generate a switch block that only has a default case', () => {
@@ -1501,9 +1501,10 @@ describe('type check blocks', () => {
 
       const result = tcb(TEMPLATE);
 
-      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();`);
+      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();`);
     });
 
     it('should generate a switch block inside a template', () => {
@@ -1526,8 +1527,8 @@ describe('type check blocks', () => {
       expect(tcb(TEMPLATE))
           .toContain(
               'var _t1: any = null!; { var _t2 = (_t1.exp); _t2(); ' +
-              'if (_t2() === "one") { "" + ((this).one()); } ' +
-              'else if (_t2() === "two") { "" + ((this).two()); } ' +
+              'if ((_t2()) === "one") { "" + ((this).one()); } ' +
+              'else if ((_t2()) === "two") { "" + ((this).two()); } ' +
               'else { "" + ((this).default()); } }');
     });
   });
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
@@ -4294,6 +4294,31 @@ suppress
         ]);
       });
 
+      it('should produce a diagnostic if @switch and @case have different types', () => {
+        env.write('test.ts', `
+          import {Component} from '@angular/core';
+
+          @Component({
+            template: \`
+              @switch (expr) {
+                @case (1) {
+                  {{expr}}
+                }
+              }
+            \`,
+            standalone: true,
+          })
+          export class Main {
+            expr = true;
+          }
+        `);
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+          `This comparison appears to be unintentional because the types 'boolean' and 'number' have no overlap.`,
+        ]);
+      });
+
       it('should narrow the type in listener inside switch cases with expressions', () => {
         env.write('test.ts', `
           import {Component} from '@angular/core';
