fix(compiler): type check invalid for loops

Currently if a `@for` loop doesn't have a `track` expression we don't produce an AST for it at all which means no type checking and language service support for it.

These changes make it so we produce the AST anyways since it gives the user more tools to resolve the issue (e.g. autocompletion when writing the `track` expression).

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

@@ -573,7 +573,7 @@ runInEachFileSystem(() => {
         });
 
         it('should retrieve a symbol for the track expression', () => {
-          const userSymbol = templateTypeChecker.getSymbolOfNode(forLoopNode.trackBy.ast, cmp)!;
+          const userSymbol = templateTypeChecker.getSymbolOfNode(forLoopNode.trackBy!.ast, cmp)!;
           expectUserSymbol(userSymbol);
         });
 

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

@@ -6573,6 +6573,32 @@ suppress
           `Type 'number' is not assignable to type 'string'.`,
         ]);
       });
+
+      it('should type check a @for loop without a `track` expression', () => {
+        env.write(
+          'test.ts',
+          `
+          import {Component} from '@angular/core';
+
+          @Component({
+            template: \`
+              @for (item of items) {
+                {{does_not_exist}}
+              }
+            \`,
+          })
+          export class Main {
+            items = [];
+          }
+        `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.map((d) => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
+          `Property 'does_not_exist' does not exist on type 'Main'.`,
+          `@for loop must have a "track" expression`,
+        ]);
+      });
     });
 
     describe('control flow content projection diagnostics', () => {

packages/compiler/src/render3/r3_ast.ts

@@ -489,8 +489,8 @@ export class ForLoopBlock extends BlockNode implements Node {
   constructor(
     public item: Variable,
     public expression: ASTWithSource,
-    public trackBy: ASTWithSource,
-    public trackKeywordSpan: ParseSourceSpan,
+    public trackBy: ASTWithSource | null,
+    public trackKeywordSpan: ParseSourceSpan | null,
     public contextVariables: Variable[],
     public children: Node[],
     public empty: ForLoopBlockEmpty | null,

packages/compiler/src/render3/r3_control_flow.ts

@@ -183,35 +183,40 @@ export function createForLoop(
   }
 
   if (params !== null) {
+    // The `for` block has a main span that includes the `empty` branch. For only the span of the
+    // main `for` body, use `mainSourceSpan`.
+    const endSpan = empty?.endSourceSpan ?? ast.endSourceSpan;
+    const sourceSpan = new ParseSourceSpan(
+      ast.sourceSpan.start,
+      endSpan?.end ?? ast.sourceSpan.end,
+    );
+    let trackExpression: ASTWithSource | null;
+    let trackKeywordSpan: ParseSourceSpan | null;
+
     if (params.trackBy === null) {
-      // TODO: We should not fail here, and instead try to produce some AST for the language
-      // service.
+      trackExpression = trackKeywordSpan = null;
       errors.push(new ParseError(ast.startSourceSpan, '@for loop must have a "track" expression'));
     } else {
-      // The `for` block has a main span that includes the `empty` branch. For only the span of the
-      // main `for` body, use `mainSourceSpan`.
-      const endSpan = empty?.endSourceSpan ?? ast.endSourceSpan;
-      const sourceSpan = new ParseSourceSpan(
-        ast.sourceSpan.start,
-        endSpan?.end ?? ast.sourceSpan.end,
-      );
+      trackExpression = params.trackBy.expression;
+      trackKeywordSpan = params.trackBy.keywordSpan;
       validateTrackByExpression(params.trackBy.expression, params.trackBy.keywordSpan, errors);
-      node = new t.ForLoopBlock(
-        params.itemName,
-        params.expression,
-        params.trackBy.expression,
-        params.trackBy.keywordSpan,
-        params.context,
-        html.visitAll(visitor, ast.children, ast.children),
-        empty,
-        sourceSpan,
-        ast.sourceSpan,
-        ast.startSourceSpan,
-        endSpan,
-        ast.nameSpan,
-        ast.i18n,
-      );
     }
+
+    node = new t.ForLoopBlock(
+      params.itemName,
+      params.expression,
+      trackExpression,
+      trackKeywordSpan,
+      params.context,
+      html.visitAll(visitor, ast.children, ast.children),
+      empty,
+      sourceSpan,
+      ast.sourceSpan,
+      ast.startSourceSpan,
+      endSpan,
+      ast.nameSpan,
+      ast.i18n,
+    );
   }
 
   return {node, errors};

packages/compiler/src/render3/view/t2_binder.ts

@@ -994,7 +994,7 @@ class TemplateBinder extends CombinedRecursiveAstVisitor {
     } else if (nodeOrNodes instanceof ForLoopBlock) {
       this.visitNode(nodeOrNodes.item);
       nodeOrNodes.contextVariables.forEach((v) => this.visitNode(v));
-      nodeOrNodes.trackBy.visit(this);
+      nodeOrNodes.trackBy?.visit(this);
       nodeOrNodes.children.forEach(this.visitNode);
       this.nestingLevel.set(nodeOrNodes, this.level);
     } else if (nodeOrNodes instanceof DeferredBlock) {

packages/compiler/src/template/pipeline/src/ingest.ts

@@ -935,8 +935,19 @@ function ingestForBlock(unit: ViewCompilationUnit, forBlock: t.ForLoopBlock): vo
     }
   }
 
-  const sourceSpan = convertSourceSpan(forBlock.trackBy.span, forBlock.sourceSpan);
-  const track = convertAst(forBlock.trackBy, unit.job, sourceSpan);
+  let track: o.Expression;
+
+  if (forBlock.trackBy === null) {
+    // `@for` without a `track` is invalid and it produces a parser error.
+    // Put a placeholder here so we don't need to account for it throughout the pipeline.
+    track = o.variable('$index');
+  } else {
+    track = convertAst(
+      forBlock.trackBy,
+      unit.job,
+      convertSourceSpan(forBlock.trackBy.span, forBlock.sourceSpan),
+    );
+  }
 
   ingestNodes(repeaterView, forBlock.children);
 

packages/compiler/src/typecheck/ops/for_block.ts

@@ -48,11 +48,21 @@ export class TcbForOfOp extends TcbOp {
     const expression = new TcbExpr(
       `${tcbExpression(this.block.expression, this.tcb, this.scope).print()}!`,
     );
-    const trackTranslator = new TcbForLoopTrackTranslator(this.tcb, loopScope, this.block);
-    const trackExpression = trackTranslator.translate(this.block.trackBy);
-    const block = getStatementsBlock([...loopScope.render(), trackExpression]);
+
+    let statements: TcbExpr[];
+
+    if (this.block.trackBy === null) {
+      statements = loopScope.render();
+    } else {
+      const trackTranslator = new TcbForLoopTrackTranslator(this.tcb, loopScope, this.block);
+      const trackExpression = trackTranslator.translate(this.block.trackBy);
+      statements = [...loopScope.render(), trackExpression];
+    }
+
     this.scope.addStatement(
-      new TcbExpr(`for (${initializer.print()} of ${expression.print()}) {\n${block} }`),
+      new TcbExpr(
+        `for (${initializer.print()} of ${expression.print()}) {\n${getStatementsBlock(statements)} }`,
+      ),
     );
     return null;
   }

packages/compiler/test/render3/r3_template_transform_spec.ts

@@ -119,7 +119,10 @@ class R3AstHumanizer implements t.Visitor<void> {
   }
 
   visitForLoopBlock(block: t.ForLoopBlock): void {
-    const result: any[] = ['ForLoopBlock', unparse(block.expression), unparse(block.trackBy)];
+    const result: any[] = ['ForLoopBlock', unparse(block.expression)];
+    if (block.trackBy !== null) {
+      result.push(unparse(block.trackBy));
+    }
     this.result.push(result);
     this.visitAll([[block.item], block.contextVariables, block.children]);
     block.empty?.visit(this);

packages/core/schematics/migrations/safe-optional-chaining/migration.ts

@@ -703,7 +703,7 @@ class TmplVisitor extends TmplAstRecursiveVisitor {
 
   override visitForLoopBlock(block: TmplAstForLoopBlock) {
     block.expression.visit(this.exprMigrator, false);
-    block.trackBy.visit(this.exprMigrator, false);
+    block.trackBy?.visit(this.exprMigrator, false);
     super.visitForLoopBlock(block);
   }
 

packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/template_reference_visitor.ts

@@ -175,7 +175,11 @@ export class TemplateReferenceVisitor<
 
   override visitForLoopBlock(block: TmplAstForLoopBlock): void {
     this.checkExpressionForReferencedFields(block, block.expression);
-    this.checkExpressionForReferencedFields(block, block.trackBy);
+
+    if (block.trackBy !== null) {
+      this.checkExpressionForReferencedFields(block, block.trackBy);
+    }
+
     super.visitForLoopBlock(block);
   }
 

packages/language-service/src/quick_info_built_ins.ts

@@ -127,6 +127,7 @@ export function createQuickInfoForBuiltIn(
       partSpan = node.nameSpan;
     } else if (
       node instanceof TmplAstForLoopBlock &&
+      node.trackKeywordSpan !== null &&
       isWithin(cursorPositionInTemplate, node.trackKeywordSpan)
     ) {
       partSpan = node.trackKeywordSpan;

packages/language-service/src/template_target.ts

@@ -679,7 +679,7 @@ class TemplateTargetVisitor implements TmplAstVisitor {
     this.visit(block.item);
     this.visitAll(block.contextVariables);
     this.visitBinding(block.expression);
-    this.visitBinding(block.trackBy);
+    block.trackBy && this.visitBinding(block.trackBy);
     this.visitAll(block.children);
     block.empty && this.visit(block.empty);
   }