From e1a29308933e99ee3e0d92aae42b33834f20f50a Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Mon, 12 Apr 2021 10:22:13 -0400
Subject: [PATCH] fix(compiler): avoid parsing EmptyExpr with a backwards span
 (#41581)

`EmptyExpr` is somewhat unique, in that it's constructed in a circumstance
where the parser has been looking for a particular token or string of tokens
and has failed to find any. This means the parser state when constructing
`EmptyExpr` is fairly unique.

This gives rise to a bug where the parser constructs `EmptyExpr` with a
backwards span - a `start` value that's beyond the `end` value. This likely
happens because of the strange state the parser is in when recovering with
`EmptyExpr`.

This commit adds a backstop/workaround to avoid constructing such broken
`EmptyExpr` spans (or any other kind of span). Eventually, the parser state
should be fixed such that this does not occur, but that requires a
significant change to the parser's functionality, so a simple fix in th
interim is in order.

PR Close #41581
---
 packages/compiler/src/expression_parser/parser.ts   | 13 +++++++++++++
 .../compiler/test/expression_parser/parser_spec.ts  |  7 +++++++
 2 files changed, 20 insertions(+)

diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts
index 855b60e89c..18a516649e 100644
--- a/packages/compiler/src/expression_parser/parser.ts
+++ b/packages/compiler/src/expression_parser/parser.ts
@@ -461,6 +461,19 @@ export class _ParseAST {
     if (artificialEndIndex !== undefined && artificialEndIndex > this.currentEndIndex) {
       endIndex = artificialEndIndex;
     }
+
+    // In some unusual parsing scenarios (like when certain tokens are missing and an `EmptyExpr` is
+    // being created), the current token may already be advanced beyond the `currentEndIndex`. This
+    // appears to be a deep-seated parser bug.
+    //
+    // As a workaround for now, swap the start and end indices to ensure a valid `ParseSpan`.
+    // TODO(alxhub): fix the bug upstream in the parser state, and remove this workaround.
+    if (start > endIndex) {
+      const tmp = endIndex;
+      endIndex = start;
+      start = tmp;
+    }
+
     return new ParseSpan(start, endIndex);
   }
 
diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts
index e9f180d3d1..5e9b149c83 100644
--- a/packages/compiler/test/expression_parser/parser_spec.ts
+++ b/packages/compiler/test/expression_parser/parser_spec.ts
@@ -188,6 +188,13 @@ describe('parser', () => {
         checkAction('a.add(1, 2)');
         checkAction('fn().add(1, 2)');
       });
+
+      it('should parse an EmptyExpr with a correct span for a trailing empty argument', () => {
+        const ast = parseAction('fn(1, )').ast as MethodCall;
+        expect(ast.args[1]).toBeAnInstanceOf(EmptyExpr);
+        const sourceSpan = (ast.args[1] as EmptyExpr).sourceSpan;
+        expect([sourceSpan.start, sourceSpan.end]).toEqual([5, 6]);
+      });
     });
 
     describe('functional calls', () => {