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
This commit is contained in:
parent
34545ad2cc
commit
e1a2930893
|
@ -461,6 +461,19 @@ export class _ParseAST {
|
||||||
if (artificialEndIndex !== undefined && artificialEndIndex > this.currentEndIndex) {
|
if (artificialEndIndex !== undefined && artificialEndIndex > this.currentEndIndex) {
|
||||||
endIndex = artificialEndIndex;
|
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);
|
return new ParseSpan(start, endIndex);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -188,6 +188,13 @@ describe('parser', () => {
|
||||||
checkAction('a.add(1, 2)');
|
checkAction('a.add(1, 2)');
|
||||||
checkAction('fn().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', () => {
|
describe('functional calls', () => {
|
||||||
|
|
Loading…
Reference in New Issue