fix(compiler): incorrect spans for left side of binary operation (#62641)

crisbeto · AndrewKushnir · commit 75d2a349b4d0 · 2025-07-15T07:57:32.000-07:00
Fixes that the span for the `left` side of a `Binary` AST included the range up to and including the operator. Fixes #62617. PR Close #62641
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts
@@ -113,14 +113,14 @@ describe('type check blocks diagnostics', () => {
     it('should annotate property writes', () => {
       const TEMPLATE = `<div (click)='a.b.c = d'></div>`;
       expect(tcbWithSpans(TEMPLATE)).toContain(
-        '(((((((this).a /*14,15*/) /*14,15*/).b /*16,17*/) /*14,17*/).c /*18,19*/) /*14,21*/) = (((this).d /*22,23*/) /*22,23*/) /*14,23*/;',
+        '(((((((this).a /*14,15*/) /*14,15*/).b /*16,17*/) /*14,17*/).c /*18,19*/) /*14,19*/) = (((this).d /*22,23*/) /*22,23*/) /*14,23*/;',
       );
     });
 
-    it('should $event property writes', () => {
+    it('should annotate $event property writes', () => {
       const TEMPLATE = `<div (click)='a = $event'></div>`;
       expect(tcbWithSpans(TEMPLATE)).toContain(
-        '(((this).a /*14,15*/) /*14,17*/) = ($event /*18,24*/) /*14,24*/;',
+        '(((this).a /*14,15*/) /*14,15*/) = ($event /*18,24*/) /*14,24*/; }) /*5,25*/;',
       );
     });
 
@@ -134,7 +134,7 @@ describe('type check blocks diagnostics', () => {
     it('should annotate keyed property writes', () => {
       const TEMPLATE = `<div (click)="a[b] = c"></div>`;
       expect(tcbWithSpans(TEMPLATE)).toContain(
-        '((((this).a /*14,15*/) /*14,15*/)[((this).b /*16,17*/) /*16,17*/] /*14,20*/) = (((this).c /*21,22*/) /*21,22*/) /*14,22*/;',
+        '((((this).a /*14,15*/) /*14,15*/)[((this).b /*16,17*/) /*16,17*/] /*14,18*/) = (((this).c /*21,22*/) /*21,22*/) /*14,22*/;',
       );
     });
 
diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts
@@ -1257,9 +1257,9 @@ class _ParseAST {
     } else {
       if (this.isAssignmentOperator(this.next)) {
         const operation = this.next.strValue;
-        this.advance();
 
         if (!(this.parseFlags & ParseFlags.Action)) {
+          this.advance();
           this.error('Bindings cannot contain assignments');
           return new EmptyExpr(this.span(start), this.sourceSpan(start));
         }
@@ -1270,6 +1270,7 @@ class _ParseAST {
           readReceiver,
           id,
         );
+        this.advance();
         const value = this.parseConditional();
         return new Binary(this.span(start), this.sourceSpan(start), operation, receiver, value);
       } else {
@@ -1398,9 +1399,9 @@ class _ParseAST {
       this.expectCharacter(chars.$RBRACKET);
       if (this.isAssignmentOperator(this.next)) {
         const operation = this.next.strValue;
-        this.advance();
 
         if (isSafe) {
+          this.advance();
           this.error("The '?.' operator cannot be used in the assignment");
         } else {
           const binaryReceiver = new KeyedRead(
@@ -1409,6 +1410,7 @@ class _ParseAST {
             receiver,
             key,
           );
+          this.advance();
           const value = this.parseConditional();
           return new Binary(
             this.span(start),
diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts
@@ -18,6 +18,7 @@ import {
   TemplateBinding,
   VariableBinding,
   BindingPipeType,
+  Binary,
 } from '../../src/expression_parser/ast';
 import {ParseError} from '../../src/parse_util';
 import {Lexer} from '../../src/expression_parser/lexer';
@@ -631,6 +632,33 @@ describe('parser', () => {
       ]);
     });
 
+    it('should record spans for binary assignment operations', () => {
+      expect(unparseWithSpan(parseAction('a.b ??= c'))).toEqual([
+        ['a.b ??= c', 'a.b ??= c'],
+        ['a.b', 'a.b'],
+        ['a.b', '[nameSpan] b'],
+        ['a', 'a'],
+        ['a', '[nameSpan] a'],
+        ['', ''],
+        ['c', 'c'],
+        ['c', '[nameSpan] c'],
+        ['', ' '],
+      ]);
+      expect(unparseWithSpan(parseAction('a[b] ||= c'))).toEqual([
+        ['a[b] ||= c', 'a[b] ||= c'],
+        ['a[b]', 'a[b]'],
+        ['a', 'a'],
+        ['a', '[nameSpan] a'],
+        ['', ''],
+        ['b', 'b'],
+        ['b', '[nameSpan] b'],
+        ['', ''],
+        ['c', 'c'],
+        ['c', '[nameSpan] c'],
+        ['', ' '],
+      ]);
+    });
+
     it('should include parenthesis in spans', () => {
       // When a LHS expression is parenthesized, the parenthesis on the left used to be
       // excluded from the span. This test verifies that the parenthesis are properly included
