fix(compiler): avoid parsing EmptyExpr with a backwards span (#41581)

alxhub · zarend · commit e1a29308933e · 2021-04-13T12:39:17.000-07:00
`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
diff --git 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
@@ -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', () => {
