fix(compiler-cli): incorrect inferred type of for loop implicit variables (#52732)

crisbeto · thePunderWoman · commit 645447daff46 · 2023-11-09T15:47:56.000Z
Fixes that all implicit variables in `@for` loops were inferred to be numbers, even though most are actually boolean. Note that I also had to work around a weird TypeScript behavior in `tsDeclareVariable` where usually we declare variables in the following format: ``` var _t1: <type> = null!; ``` This works in most cases, but if the type is a `boolean`, TypeScript infers the variable as `never`, instead of `boolean`. I've worked around it by adding an `as boolean` to the initializer. Fixes #52730. PR Close #52732
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts
@@ -77,11 +77,21 @@ export function tsCreateElement(tagName: string): ts.Expression {
  * Unlike with `tsCreateVariable`, the type of the variable is explicitly specified.
  */
 export function tsDeclareVariable(id: ts.Identifier, type: ts.TypeNode): ts.VariableStatement {
+  let initializer: ts.Expression = ts.factory.createNonNullExpression(ts.factory.createNull());
+
+  // When we create a variable like `var _t1: boolean = null!`, TypeScript actually infers `_t1`
+  // to be `never`, instead of a `boolean`. To work around it, we cast the value to boolean again
+  // in the initializer, e.g. `var _t1: boolean = null! as boolean;`.
+  if (type.kind === ts.SyntaxKind.BooleanKeyword) {
+    initializer = ts.factory.createAsExpression(
+        initializer, ts.factory.createKeywordTypeNode(ts.SyntaxKind.BooleanKeyword));
+  }
+
   const decl = ts.factory.createVariableDeclaration(
       /* name */ id,
       /* exclamationToken */ undefined,
       /* type */ type,
-      /* initializer */ ts.factory.createNonNullExpression(ts.factory.createNull()));
+      /* initializer */ initializer);
   return ts.factory.createVariableStatement(
       /* modifiers */ undefined,
       /* declarationList */[decl]);
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
@@ -1572,6 +1572,18 @@ class Scope {
    */
   private statements: ts.Statement[] = [];
 
+  /**
+   * Names of the for loop context variables and their types.
+   */
+  private static readonly forLoopContextVariableTypes = new Map<string, ts.KeywordTypeSyntaxKind>([
+    ['$first', ts.SyntaxKind.BooleanKeyword],
+    ['$last', ts.SyntaxKind.BooleanKeyword],
+    ['$even', ts.SyntaxKind.BooleanKeyword],
+    ['$odd', ts.SyntaxKind.BooleanKeyword],
+    ['$index', ts.SyntaxKind.NumberKeyword],
+    ['$count', ts.SyntaxKind.NumberKeyword],
+  ]);
+
   private constructor(
       private tcb: Context, private parent: Scope|null = null,
       private guard: ts.Expression|null = null) {}
@@ -1628,9 +1640,12 @@ class Scope {
           new TcbBlockVariableOp(
               tcb, scope, ts.factory.createIdentifier(scopedNode.item.name), scopedNode.item));
 
-      for (const variable of Object.values(scopedNode.contextVariables)) {
-        // Note: currently all context variables are assumed to be number types.
-        const type = ts.factory.createKeywordTypeNode(ts.SyntaxKind.NumberKeyword);
+      for (const [name, variable] of Object.entries(scopedNode.contextVariables)) {
+        if (!this.forLoopContextVariableTypes.has(name)) {
+          throw new Error(`Unrecognized for loop context variable ${name}`);
+        }
+
+        const type = ts.factory.createKeywordTypeNode(this.forLoopContextVariableTypes.get(name)!);
         this.registerVariable(
             scope, variable, new TcbBlockImplicitVariableOp(tcb, scope, type, variable));
       }
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
@@ -1560,10 +1560,10 @@ describe('type check blocks', () => {
       const result = tcb(TEMPLATE);
       expect(result).toContain('for (const item of ((this).items)!) { var _t1 = item;');
       expect(result).toContain('var _t2: number = null!;');
-      expect(result).toContain('var _t3: number = null!;');
-      expect(result).toContain('var _t4: number = null!;');
-      expect(result).toContain('var _t5: number = null!;');
-      expect(result).toContain('var _t6: number = null!;');
+      expect(result).toContain('var _t3: boolean = null! as boolean;');
+      expect(result).toContain('var _t4: boolean = null! as boolean;');
+      expect(result).toContain('var _t5: boolean = null! as boolean;');
+      expect(result).toContain('var _t6: boolean = null! as boolean;');
       expect(result).toContain('var _t7: number = null!;');
       expect(result).toContain('"" + (_t2) + (_t3) + (_t4) + (_t5) + (_t6) + (_t7)');
     });
@@ -1578,10 +1578,10 @@ describe('type check blocks', () => {
       const result = tcb(TEMPLATE);
       expect(result).toContain('for (const item of ((this).items)!) { var _t1 = item;');
       expect(result).toContain('var _t2: number = null!;');
-      expect(result).toContain('var _t3: number = null!;');
-      expect(result).toContain('var _t4: number = null!;');
-      expect(result).toContain('var _t5: number = null!;');
-      expect(result).toContain('var _t6: number = null!;');
+      expect(result).toContain('var _t3: boolean = null! as boolean;');
+      expect(result).toContain('var _t4: boolean = null! as boolean;');
+      expect(result).toContain('var _t5: boolean = null! as boolean;');
+      expect(result).toContain('var _t6: boolean = null! as boolean;');
       expect(result).toContain('var _t7: number = null!;');
       expect(result).toContain('"" + (_t2) + (_t3) + (_t4) + (_t5) + (_t6) + (_t7)');
     });
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
@@ -4477,10 +4477,10 @@ suppress
         const diags = env.driveDiagnostics();
         expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
           `Argument of type 'number' is not assignable to parameter of type 'string'.`,
-          `Argument of type 'number' is not assignable to parameter of type 'string'.`,
-          `Argument of type 'number' is not assignable to parameter of type 'string'.`,
-          `Argument of type 'number' is not assignable to parameter of type 'string'.`,
-          `Argument of type 'number' is not assignable to parameter of type 'string'.`,
+          `Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
+          `Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
+          `Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
+          `Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
           `Argument of type 'number' is not assignable to parameter of type 'string'.`,
         ]);
       });
@@ -4513,10 +4513,10 @@ suppress
         const diags = env.driveDiagnostics();
         expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
           `Argument of type 'number' is not assignable to parameter of type 'string'.`,
-          `Argument of type 'number' is not assignable to parameter of type 'string'.`,
-          `Argument of type 'number' is not assignable to parameter of type 'string'.`,
-          `Argument of type 'number' is not assignable to parameter of type 'string'.`,
-          `Argument of type 'number' is not assignable to parameter of type 'string'.`,
+          `Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
+          `Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
+          `Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
+          `Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
           `Argument of type 'number' is not assignable to parameter of type 'string'.`,
         ]);
       });
