fix(compiler): declare for loop aliases in addition to new name (#54942)

Currently when aliasing a `for` loop variable with `let`, we replace the variable's old name with the new one. Since users have found this to be confusing, these changes switch to a model where the variable is available both under the original name and the new one.

Fixes #52528.

PR Close #54942

packages/compiler-cli/src/ngtsc/indexer/src/template.ts

@@ -271,7 +271,7 @@ class TemplateVisitor extends TmplAstRecursiveVisitor {
 
   override visitForLoopBlock(block: TmplAstForLoopBlock): void {
     block.item.visit(this);
-    this.visitAll(Object.values(block.contextVariables));
+    this.visitAll(block.contextVariables);
     this.visitExpression(block.expression);
     this.visitAll(block.children);
     block.empty?.visit(this);

packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts

@@ -195,7 +195,7 @@ class TemplateVisitor<Code extends ErrorCode> extends RecursiveAstVisitor implem
 
   visitForLoopBlock(block: TmplAstForLoopBlock): void {
     block.item.visit(this);
-    this.visitAllNodes(Object.values(block.contextVariables));
+    this.visitAllNodes(block.contextVariables);
     this.visitAst(block.expression);
     this.visitAllNodes(block.children);
     block.empty?.visit(this);

packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts

@@ -353,11 +353,13 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
       throw new Error(`Assertion failure: no SourceLocation found for property read.`);
     }
 
+    const messageVars = [block.item, ...block.contextVariables.filter(v => v.value === '$index')]
+                            .map(v => `'${v.name}'`)
+                            .join(', ');
     const message =
         `Cannot access '${access.name}' inside of a track expression. ` +
-        `Only '${block.item.name}', '${
-            block.contextVariables.$index
-                .name}' and properties on the containing component are available to this expression.`;
+        `Only ${
+            messageVars} and properties on the containing component are available to this expression.`;
 
     this._diagnostics.push(makeTemplateDiagnostic(
         templateId, this.resolver.getSourceMapping(templateId), sourceSpan,

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

@@ -1816,12 +1816,13 @@ class Scope {
       addParseSpanInfo(loopInitializer, scopedNode.item.sourceSpan);
       scope.varMap.set(scopedNode.item, loopInitializer);
 
-      for (const [name, variable] of Object.entries(scopedNode.contextVariables)) {
-        if (!this.forLoopContextVariableTypes.has(name)) {
-          throw new Error(`Unrecognized for loop context variable ${name}`);
+      for (const variable of scopedNode.contextVariables) {
+        if (!this.forLoopContextVariableTypes.has(variable.value)) {
+          throw new Error(`Unrecognized for loop context variable ${variable.name}`);
         }
 
-        const type = ts.factory.createKeywordTypeNode(this.forLoopContextVariableTypes.get(name)!);
+        const type =
+            ts.factory.createKeywordTypeNode(this.forLoopContextVariableTypes.get(variable.value)!);
         this.registerVariable(
             scope, variable, new TcbBlockImplicitVariableOp(tcb, scope, type, variable));
       }
@@ -2761,18 +2762,26 @@ class TcbEventHandlerTranslator extends TcbExpressionTranslator {
 }
 
 class TcbForLoopTrackTranslator extends TcbExpressionTranslator {
+  private allowedVariables: Set<TmplAstVariable>;
+
   constructor(tcb: Context, scope: Scope, private block: TmplAstForLoopBlock) {
     super(tcb, scope);
+
+    // Tracking expressions are only allowed to read the `$index`,
+    // the item and properties off the component instance.
+    this.allowedVariables = new Set([block.item]);
+    for (const variable of block.contextVariables) {
+      if (variable.value === '$index') {
+        this.allowedVariables.add(variable);
+      }
+    }
   }
 
   protected override resolve(ast: AST): ts.Expression|null {
     if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver) {
       const target = this.tcb.boundTarget.getExpressionTarget(ast);
 
-      // Tracking expressions are only allowed to read the `$index`,
-      // the item and properties off the component instance.
-      if (target !== null && target !== this.block.item &&
-          target !== this.block.contextVariables.$index) {
+      if (target !== null && !this.allowedVariables.has(target)) {
         this.tcb.oobRecorder.illegalForLoopTrackAccess(this.tcb.id, this.block, ast);
       }
     }

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

@@ -1710,15 +1710,16 @@ describe('type check blocks', () => {
       expect(result).toContain('"" + (_t2) + (_t3) + (_t4) + (_t5) + (_t6) + (_t7)');
     });
 
-    it('should read an implicit variable from the component scope if it is aliased', () => {
+    it('should read both implicit variables and their alias at the same time', () => {
       const TEMPLATE = `
         @for (item of items; track item; let i = $index) { {{$index}} {{i}} }
       `;
 
       const result = tcb(TEMPLATE);
       expect(result).toContain('for (const _t1 of ((this).items)!) {');
       expect(result).toContain('var _t2 = null! as number;');
-      expect(result).toContain('"" + (((this).$index)) + (_t2)');
+      expect(result).toContain('var _t3 = null! as number;');
+      expect(result).toContain('"" + (_t2) + (_t3)');
     });
 
     it('should read variable from a parent for loop', () => {

packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts

@@ -477,17 +477,18 @@ runInEachFileSystem(() => {
         });
 
         it('finds symbols for $index variable', () => {
-          const iVar = forLoopNode.contextVariables.$index;
+          const iVar = forLoopNode.contextVariables.find(v => v.name === '$index')!;
           const iSymbol = templateTypeChecker.getSymbolOfNode(iVar, cmp)!;
-          expectIndexSymbol(iSymbol);
+          expect(iVar).toBeTruthy();
+          expectIndexSymbol(iSymbol, '$index');
         });
 
         it('finds symbol when using the index in the body', () => {
           const innerElementNodes =
               onlyAstElements((forLoopNode.children[0] as TmplAstElement).children);
           const indexSymbol =
               templateTypeChecker.getSymbolOfNode(innerElementNodes[0].inputs[0].value, cmp)!;
-          expectIndexSymbol(indexSymbol);
+          expectIndexSymbol(indexSymbol, 'i');
         });
 
         function expectUserSymbol(userSymbol: Symbol) {
@@ -497,12 +498,15 @@ runInEachFileSystem(() => {
           expect((userSymbol).declaration).toEqual(forLoopNode.item);
         }
 
-        function expectIndexSymbol(indexSymbol: Symbol) {
+        function expectIndexSymbol(indexSymbol: Symbol, localName: string) {
+          const indexVar =
+              forLoopNode.contextVariables.find(v => v.value === '$index' && v.name === localName)!;
           assertVariableSymbol(indexSymbol);
+          expect(indexVar).toBeTruthy();
           expect(indexSymbol.tsSymbol)
               .toBeNull();  // implicit variable doesn't have a TS definition location
           expect(program.getTypeChecker().typeToString(indexSymbol.tsType!)).toEqual('number');
-          expect((indexSymbol).declaration).toEqual(forLoopNode.contextVariables.$index);
+          expect((indexSymbol).declaration).toEqual(indexVar);
         }
       });
     });

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js

@@ -1143,6 +1143,75 @@ export declare class MyApp {
     static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
 }
 
+/****************************************************************************************************
+ * PARTIAL FILE: for_both_aliased_and_original_variables.js
+ ****************************************************************************************************/
+import { Component } from '@angular/core';
+import * as i0 from "@angular/core";
+export class MyApp {
+    constructor() {
+        this.message = 'hello';
+        this.items = [];
+    }
+}
+MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
+MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
+    <div>
+      {{message}}
+      @for (item of items; track item; let idx = $index, f = $first; let l = $last, ev = $even, o = $odd; let co = $count) {
+        Original index: {{$index}}
+        Original first: {{$first}}
+        Original last: {{$last}}
+        Original even: {{$even}}
+        Original odd: {{$odd}}
+        Original count: {{$count}}
+        <hr>
+        Aliased index: {{idx}}
+        Aliased first: {{f}}
+        Aliased last: {{l}}
+        Aliased even: {{ev}}
+        Aliased odd: {{o}}
+        Aliased count: {{co}}
+      }
+    </div>
+  `, isInline: true });
+i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
+            type: Component,
+            args: [{
+                    template: `
+    <div>
+      {{message}}
+      @for (item of items; track item; let idx = $index, f = $first; let l = $last, ev = $even, o = $odd; let co = $count) {
+        Original index: {{$index}}
+        Original first: {{$first}}
+        Original last: {{$last}}
+        Original even: {{$even}}
+        Original odd: {{$odd}}
+        Original count: {{$count}}
+        <hr>
+        Aliased index: {{idx}}
+        Aliased first: {{f}}
+        Aliased last: {{l}}
+        Aliased even: {{ev}}
+        Aliased odd: {{o}}
+        Aliased count: {{co}}
+      }
+    </div>
+  `,
+                }]
+        }] });
+
+/****************************************************************************************************
+ * PARTIAL FILE: for_both_aliased_and_original_variables.d.ts
+ ****************************************************************************************************/
+import * as i0 from "@angular/core";
+export declare class MyApp {
+    message: string;
+    items: never[];
+    static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
+    static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
+}
+
 /****************************************************************************************************
  * PARTIAL FILE: nested_for_template_variables.js
  ****************************************************************************************************/

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json

@@ -301,6 +301,21 @@
         }
       ]
     },
+    {
+      "description": "should generate a for block with references both to original and aliased template variables",
+      "inputFiles": ["for_both_aliased_and_original_variables.ts"],
+      "expectations": [
+        {
+          "files": [
+            {
+              "expected": "for_both_aliased_and_original_variables_template.js",
+              "generated": "for_both_aliased_and_original_variables.js"
+            }
+          ],
+          "failureMessage": "Incorrect template"
+        }
+      ]
+    },
     {
       "description": "should be able to refer to aliased template variables in nested for blocks",
       "inputFiles": ["nested_for_template_variables.ts"],

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_aliased_template_variables_template.pipeline.js

@@ -3,11 +3,9 @@ function MyApp_For_3_Template(rf, ctx) {
 	  $r3$.ɵɵtext(0);
 	}
 	if (rf & 2) {
-	  const $idx_r2$ = ctx.$index;
-	  const $co_r3$ = ctx.$count;
-	  const $idx_2_r2$ = ctx.$index;
-	  const $co_2_r3$ = ctx.$count;
-	  $r3$.ɵɵtextInterpolate6(" Index: ", $idx_r2$, " First: ", $idx_2_r2$ === 0, " Last: ", $idx_2_r2$ === $co_2_r3$ - 1, " Even: ", $idx_2_r2$ % 2 === 0, " Odd: ", $idx_2_r2$ % 2 !== 0, " Count: ", $co_r3$, " ");
+		const $idx_r2$ = ctx.$index;
+    const $co_r2$ = ctx.$count;
+	  $r3$.ɵɵtextInterpolate6(" Index: ", $idx_r2$, " First: ", $idx_r2$ === 0, " Last: ", $idx_r2$ === $co_r2$ - 1, " Even: ", $idx_r2$ % 2 === 0, " Odd: ", $idx_r2$ % 2 !== 0, " Count: ", $co_r2$, " ");
 	}
   }
   …

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables.ts

@@ -0,0 +1,28 @@
+import {Component} from '@angular/core';
+
+@Component({
+  template: `
+    <div>
+      {{message}}
+      @for (item of items; track item; let idx = $index, f = $first; let l = $last, ev = $even, o = $odd; let co = $count) {
+        Original index: {{$index}}
+        Original first: {{$first}}
+        Original last: {{$last}}
+        Original even: {{$even}}
+        Original odd: {{$odd}}
+        Original count: {{$count}}
+        <hr>
+        Aliased index: {{idx}}
+        Aliased first: {{f}}
+        Aliased last: {{l}}
+        Aliased even: {{ev}}
+        Aliased odd: {{o}}
+        Aliased count: {{co}}
+      }
+    </div>
+  `,
+})
+export class MyApp {
+  message = 'hello';
+  items = [];
+}

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_both_aliased_and_original_variables_template.js

@@ -0,0 +1,31 @@
+function MyApp_For_3_Template(rf, ctx) {
+  if (rf & 1) {
+    $r3$.ɵɵtext(0);
+    $r3$.ɵɵelement(1, "hr");
+    $r3$.ɵɵtext(2);
+  }
+  if (rf & 2) {
+    const $index_r1$ = ctx.$index;
+    const $index_4_r2$ = ctx.$index;
+    const $count_r3$ = ctx.$count;
+    const $count_4_r4$ = ctx.$count;
+    $r3$.ɵɵtextInterpolate6(" Original index: ", $index_r1$, " Original first: ", $index_4_r2$ === 0, " Original last: ", $index_4_r2$ === $count_4_r4$ - 1, " Original even: ", $index_4_r2$ % 2 === 0, " Original odd: ", $index_4_r2$ % 2 !== 0, " Original count: ", $count_r3$, " ");
+    $r3$.ɵɵadvance(2);
+    $r3$.ɵɵtextInterpolate6(" Aliased index: ", $index_4_r2$, " Aliased first: ", $index_4_r2$ === 0, " Aliased last: ", $index_4_r2$ === $count_4_r4$ - 1, " Aliased even: ", $index_4_r2$ % 2 === 0, " Aliased odd: ", $index_4_r2$ % 2 !== 0, " Aliased count: ", $count_4_r4$, " ");
+  }
+}
+…
+function MyApp_Template(rf, ctx) {
+  if (rf & 1) {
+    $r3$.ɵɵelementStart(0, "div");
+    $r3$.ɵɵtext(1);
+    $r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 12, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
+    $r3$.ɵɵelementEnd();
+  }
+  if (rf & 2) {
+    $r3$.ɵɵadvance();
+    $r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
+    $r3$.ɵɵadvance();
+    $r3$.ɵɵrepeater(ctx.items);
+  }
+}

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_listener_template.pipeline.js

@@ -5,8 +5,8 @@ function MyApp_For_3_Template(rf, ctx) {
 	  $r3$.ɵɵlistener("click", function MyApp_For_3_Template_div_click_0_listener() {
 		const $restoredCtx$ = $r3$.ɵɵrestoreView($_r5$);
 		const $index_r2$ = $restoredCtx$.$index;
-		const $count_r3$ = $restoredCtx$.$count;
 		const $index_2_r2$ = $restoredCtx$.$index;
+		const $count_r3$ = $restoredCtx$.$count;
 		const $ctx_r4$ = $r3$.ɵɵnextContext();
 		return $r3$.ɵɵresetView($ctx_r4$.log($index_r2$, $index_2_r2$ % 2 === 0, $index_2_r2$ === 0, $count_r3$));
 	  });

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_scope_template.pipeline.js

@@ -4,8 +4,8 @@ function MyApp_For_2_Template(rf, ctx) {
 	}
 	if (rf & 2) {
 	  const $index_r2$ = ctx.$index;
-	  const $count_r3$ = ctx.$count;
 	  const $index_2_r2$ = ctx.$index;
+	  const $count_r3$ = ctx.$count;
 	  const $count_2_r3$ = ctx.$count;
 	  $r3$.ɵɵtextInterpolate4(" ", $index_r2$, " ", $count_r3$, " ", $index_2_r2$ === 0, " ", $index_2_r2$ === $count_2_r3$ - 1, " ");
 	}

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_variables_template.pipeline.js

@@ -4,8 +4,8 @@ function MyApp_For_3_Template(rf, ctx) {
 	}
 	if (rf & 2) {
 	  const $index_r2$ = ctx.$index;
-	  const $count_r3$ = ctx.$count;
 	  const $index_4_r2$ = ctx.$index;
+	  const $count_r3$ = ctx.$count;
 	  const $count_4_r2$ = ctx.$count;
 	  $r3$.ɵɵtextInterpolate6(" Index: ", $index_r2$, " First: ", $index_4_r2$ === 0, " Last: ", $index_4_r2$ === $count_4_r2$ - 1, " Even: ", $index_4_r2$ % 2 === 0, " Odd: ", $index_4_r2$ % 2 !== 0, " Count: ", $count_r3$, " ");
 	}

packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts

@@ -4828,23 +4828,25 @@ suppress
         ]);
       });
 
-      it('should not expose variables under their implicit name if they are aliased', () => {
-        env.write('test.ts', `
-          import {Component} from '@angular/core';
+      it('should continue exposing implicit loop variables under their old names when they are aliased',
+         () => {
+           env.write('test.ts', `
+            import {Component} from '@angular/core';
 
-          @Component({
-            template: '@for (item of items; track item; let alias = $index) { {{$index}} {{$count}} }',
-          })
-          export class Main {
-            items = [];
-          }
-        `);
+            @Component({
+              template: '@for (item of items; track item; let alias = $index) { {{acceptsString($index)}} }',
+            })
+            export class Main {
+              items = [];
+              acceptsString(str: string) {}
+            }
+          `);
 
-        const diags = env.driveDiagnostics();
-        expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
-          `Property '$index' does not exist on type 'Main'.`,
-        ]);
-      });
+           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'.`,
+           ]);
+         });
 
       it('should not be able to write to loop template variables', () => {
         env.write('test.ts', `