fix(compiler): produce less noisy errors when parsing control flow (#57711)

crisbeto · thePunderWoman · commit b619d6987efe · 2024-09-09T14:10:02.000Z
Currently several parsing errors in the new control flow (e.g. missing `track` expression) produce errors whose span targets the entire block. This can be really noisy in the IDE where the error can span many lines in the template. These changes switch to highlighting just the start of the block. PR Close #57711
diff --git a/packages/compiler/src/render3/r3_control_flow.ts b/packages/compiler/src/render3/r3_control_flow.ts
@@ -183,7 +183,7 @@ export function createForLoop(
     if (params.trackBy === null) {
       // TODO: We should not fail here, and instead try to produce some AST for the language
       // service.
-      errors.push(new ParseError(ast.sourceSpan, '@for loop must have a "track" expression'));
+      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`.
@@ -284,7 +284,7 @@ function parseForLoopParameters(
   bindingParser: BindingParser,
 ) {
   if (block.parameters.length === 0) {
-    errors.push(new ParseError(block.sourceSpan, '@for loop does not have an expression'));
+    errors.push(new ParseError(block.startSourceSpan, '@for loop does not have an expression'));
     return null;
   }
 
@@ -372,7 +372,9 @@ function parseForLoopParameters(
       } else {
         const expression = parseBlockParameterToBinding(param, bindingParser, trackMatch[1]);
         if (expression.ast instanceof EmptyExpr) {
-          errors.push(new ParseError(param.sourceSpan, '@for loop must have a "track" expression'));
+          errors.push(
+            new ParseError(block.startSourceSpan, '@for loop must have a "track" expression'),
+          );
         }
         const keywordSpan = new ParseSourceSpan(
           param.sourceSpan.start,
@@ -481,18 +483,20 @@ function validateIfConnectedBlocks(connectedBlocks: html.Block[]): ParseError[]
 
     if (block.name === 'else') {
       if (hasElse) {
-        errors.push(new ParseError(block.sourceSpan, 'Conditional can only have one @else block'));
+        errors.push(
+          new ParseError(block.startSourceSpan, 'Conditional can only have one @else block'),
+        );
       } else if (connectedBlocks.length > 1 && i < connectedBlocks.length - 1) {
         errors.push(
-          new ParseError(block.sourceSpan, '@else block must be last inside the conditional'),
+          new ParseError(block.startSourceSpan, '@else block must be last inside the conditional'),
         );
       } else if (block.parameters.length > 0) {
-        errors.push(new ParseError(block.sourceSpan, '@else block cannot have parameters'));
+        errors.push(new ParseError(block.startSourceSpan, '@else block cannot have parameters'));
       }
       hasElse = true;
     } else if (!ELSE_IF_PATTERN.test(block.name)) {
       errors.push(
-        new ParseError(block.sourceSpan, `Unrecognized conditional block @${block.name}`),
+        new ParseError(block.startSourceSpan, `Unrecognized conditional block @${block.name}`),
       );
     }
   }
@@ -506,7 +510,9 @@ function validateSwitchBlock(ast: html.Block): ParseError[] {
   let hasDefault = false;
 
   if (ast.parameters.length !== 1) {
-    errors.push(new ParseError(ast.sourceSpan, '@switch block must have exactly one parameter'));
+    errors.push(
+      new ParseError(ast.startSourceSpan, '@switch block must have exactly one parameter'),
+    );
     return errors;
   }
 
@@ -530,14 +536,16 @@ function validateSwitchBlock(ast: html.Block): ParseError[] {
     if (node.name === 'default') {
       if (hasDefault) {
         errors.push(
-          new ParseError(node.sourceSpan, '@switch block can only have one @default block'),
+          new ParseError(node.startSourceSpan, '@switch block can only have one @default block'),
         );
       } else if (node.parameters.length > 0) {
-        errors.push(new ParseError(node.sourceSpan, '@default block cannot have parameters'));
+        errors.push(new ParseError(node.startSourceSpan, '@default block cannot have parameters'));
       }
       hasDefault = true;
     } else if (node.name === 'case' && node.parameters.length !== 1) {
-      errors.push(new ParseError(node.sourceSpan, '@case block must have exactly one parameter'));
+      errors.push(
+        new ParseError(node.startSourceSpan, '@case block must have exactly one parameter'),
+      );
     }
   }
 
@@ -586,7 +594,9 @@ function parseConditionalBlockParameters(
   bindingParser: BindingParser,
 ) {
   if (block.parameters.length === 0) {
-    errors.push(new ParseError(block.sourceSpan, 'Conditional block does not have an expression'));
+    errors.push(
+      new ParseError(block.startSourceSpan, 'Conditional block does not have an expression'),
+    );
     return null;
   }
 
