Skip to content

[Pratt Parser] Add a fuzzer comparing ANTLR and Pratt parser outputs and fix uncovered discrepancies #1231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Pratt Parser] Add a fuzzer comparing ANTLR and Pratt parser outputs and fix uncovered discrepancies #1231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions parser/src/main/java/dev/cel/parser/Lexer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ private void consumeWhitespaceAndComments() {
case '\n':
case ' ':
case '\r':
case 11: // \v
case '\t':
position++;
break;
Expand Down Expand Up @@ -595,12 +594,6 @@ private Token consumeNumericLiteral() {
start, position, "integral literal missing digits after hexadecimal separator");
}
TokenType tokenType = consumeIntegralSuffix();
if (consumeIf(Lexer::isIdentTrailing)) {
return setError(
start,
position,
tokenType.getSymbol() + " literal has unexpected trailing characters");
}
return makeToken(tokenType, start, position);
}
consumeDigits();
Expand All @@ -622,10 +615,6 @@ && isDigit(content.get(position + 1))) {
}
}
TokenType tokenType = floatingPoint ? TokenType.FLOAT : consumeIntegralSuffix();
if (consumeIf(Lexer::isIdentTrailing)) {
return setError(
start, position, tokenType.getSymbol() + " literal has unexpected trailing characters");
}
return makeToken(tokenType, start, position);
}

Expand Down
146 changes: 54 additions & 92 deletions parser/src/main/java/dev/cel/parser/PrattParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import dev.cel.common.CelAbstractSyntaxTree;
import dev.cel.common.CelIssue;
import dev.cel.common.CelOptions;
Expand Down Expand Up @@ -524,21 +523,26 @@ private static CelExpr buildUnaryCall(long id, String function, CelExpr operand)

private CelExpr parseSelectorChain() {
Lexer.TokenType tok = peekToken.type;
CelExpr lhs =
(tok == Lexer.TokenType.EXCLAMATION || tok == Lexer.TokenType.MINUS)
? parseUnaryOps()
: parsePrimary();
if (tok == Lexer.TokenType.EXCLAMATION || tok == Lexer.TokenType.MINUS) {
return parseUnaryOps();
}
return parseMember();
}

private CelExpr parseMember() {
int memberStart = peekToken.start;
CelExpr lhs = parsePrimary();
currentLhsDepth = 0;
tok = peekToken.type;
Lexer.TokenType tok = peekToken.type;
if (tok == Lexer.TokenType.DOT
|| tok == Lexer.TokenType.LEFT_BRACKET
|| tok == Lexer.TokenType.LEFT_BRACE) {
lhs = parseSelectorChainTail(lhs);
lhs = parseSelectorChainTail(lhs, memberStart);
}
return lhs;
}

private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
private CelExpr parseSelectorChainTail(CelExpr initialLhs, int memberStartPosition) {
CelExpr lhs = initialLhs;
int chainDepth = 0;
while (true) {
Expand All @@ -558,9 +562,7 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
}
}
Lexer.Token idTok = nextToken();
if (idTok.type != Lexer.TokenType.IDENT
&& idTok.type != Lexer.TokenType.RESERVED_WORD
&& idTok.type != Lexer.TokenType.IN) {
if (idTok.type != Lexer.TokenType.IDENT && idTok.type != Lexer.TokenType.RESERVED_WORD) {
if (idTok.type != Lexer.TokenType.ERROR) {
reportSyntaxError(idTok, "expected identifier after '.'");
}
Expand All @@ -573,7 +575,7 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
if (optional) {
long opId = nextId(dotTok);
CelExpr field =
CelExpr.ofConstant(nextId(getLeftmostPosition(lhs)), CelConstant.ofValue(idText));
CelExpr.ofConstant(nextId(memberStartPosition), CelConstant.ofValue(idText));
lhs = buildBinaryCall(opId, Operator.OPTIONAL_SELECT.getFunction(), lhs, field);
} else if (peekToken.type == Lexer.TokenType.LEFT_PAREN) {
Lexer.Token lparen = nextToken();
Expand All @@ -584,6 +586,8 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
expanded.isPresent()
? expanded.get()
: CelExpr.ofCall(callId, Optional.of(lhs), idText, args);
} else if (idText.isEmpty()) {
lhs = CelExpr.newBuilder().setId(nextId(dotTok)).build();
} else {
lhs = CelExpr.ofSelect(nextId(dotTok), lhs, idText, /* isTestOnly= */ false);
}
Expand Down Expand Up @@ -622,83 +626,42 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
}

private CelExpr parseUnaryOps() {
Lexer.Token op = nextToken();
Lexer.TokenType opType = op.type;
if (peekToken.type == Lexer.TokenType.EXCLAMATION || peekToken.type == Lexer.TokenType.MINUS) {
return parseUnaryOpsChain(op);
}

if (opType == Lexer.TokenType.MINUS) {
if (peekToken.type == Lexer.TokenType.INT) {
return parseIntLiteral(nextId(peekToken), /* isNegative= */ true);
}
if (peekToken.type == Lexer.TokenType.FLOAT) {
return parseDoubleLiteral(nextId(peekToken), /* isNegative= */ true);
}
}

if (checkRecursion(0, op)) {
return ERROR;
}

long opId = nextId(op);
recursionDepth++;
CelExpr operand = parseSelectorChain();
recursionDepth--;
if (recursionLimitExceeded) {
return ERROR;
}

String opName =
(opType == Lexer.TokenType.EXCLAMATION)
? Operator.LOGICAL_NOT.getFunction()
: Operator.NEGATE.getFunction();
return buildUnaryCall(opId, opName, operand);
}

private CelExpr parseUnaryOpsChain(Lexer.Token firstOp) {
Lexer.Token firstOp = nextToken();
Lexer.TokenType opType = firstOp.type;
List<UnaryOp> ops = new ArrayList<>();
ops.add(new UnaryOp(firstOp));
while (peekToken.type == Lexer.TokenType.EXCLAMATION
|| peekToken.type == Lexer.TokenType.MINUS) {
while (peekToken.type == opType) {
ops.add(new UnaryOp(nextToken()));
}

boolean hasSolitaryTrailingMinus =
!ops.isEmpty()
&& Iterables.getLast(ops).token.type == Lexer.TokenType.MINUS
&& (ops.size() == 1 || ops.get(ops.size() - 2).token.type != Lexer.TokenType.MINUS);

if (!options.retainRepeatedUnaryOperators()) {
int write = 0;
for (int read = 0; read < ops.size(); ) {
int next = read;
while (next < ops.size() && ops.get(next).token.type == ops.get(read).token.type) {
next++;
}
if ((next - read) % 2 != 0) {
ops.set(write++, ops.get(read));
}
read = next;
if (opType == Lexer.TokenType.MINUS
&& ops.size() == 1
&& (peekToken.type == Lexer.TokenType.INT || peekToken.type == Lexer.TokenType.FLOAT)) {
CelExpr lhs =
(peekToken.type == Lexer.TokenType.INT)
? parseIntLiteral(nextId(peekToken), /* isNegative= */ true)
: parseDoubleLiteral(nextId(peekToken), /* isNegative= */ true);
currentLhsDepth = 0;
Lexer.TokenType tok = peekToken.type;
if (tok == Lexer.TokenType.DOT
|| tok == Lexer.TokenType.LEFT_BRACKET
|| tok == Lexer.TokenType.LEFT_BRACE) {
lhs = parseSelectorChainTail(lhs, firstOp.start);
}
ops = new ArrayList<>(ops.subList(0, write));
}

for (UnaryOp op : ops) {
op.id = nextId(op.token);
return lhs;
}

boolean isNegativeNumericLiteral =
hasSolitaryTrailingMinus
&& (peekToken.type == Lexer.TokenType.INT || peekToken.type == Lexer.TokenType.FLOAT);
long negativeLiteralOpId = 0;
if (isNegativeNumericLiteral) {
negativeLiteralOpId = Iterables.getLast(ops).id;
ops.remove(ops.size() - 1);
if (!options.retainRepeatedUnaryOperators()) {
if (ops.size() % 2 == 0) {
ops.clear();
} else {
ops = new ArrayList<>(ops.subList(0, 1));
}
}

int chainDepth = 0;
for (UnaryOp op : ops) {
op.id = nextId(op.token);
if (checkRecursion(chainDepth, op.token)) {
return ERROR;
}
Expand All @@ -707,14 +670,20 @@ private CelExpr parseUnaryOpsChain(Lexer.Token firstOp) {

recursionDepth += ops.size();
CelExpr operand;
if (isNegativeNumericLiteral) {
operand =
(peekToken.type == Lexer.TokenType.INT)
? parseIntLiteral(negativeLiteralOpId, /* isNegative= */ true)
: parseDoubleLiteral(negativeLiteralOpId, /* isNegative= */ true);
operand = parseSelectorChainTail(operand);
if (opType == Lexer.TokenType.EXCLAMATION && peekToken.type == Lexer.TokenType.MINUS) {
Lexer.Token minusTok = nextToken();
if (peekToken.type == Lexer.TokenType.INT) {
operand = parseIntLiteral(nextId(peekToken), /* isNegative= */ true);
operand = parseSelectorChainTail(operand, minusTok.start);
} else if (peekToken.type == Lexer.TokenType.FLOAT) {
operand = parseDoubleLiteral(nextId(peekToken), /* isNegative= */ true);
operand = parseSelectorChainTail(operand, minusTok.start);
} else {
reportSyntaxError(minusTok, "unexpected '-'");
operand = parseMember();
}
} else {
operand = parseSelectorChain();
operand = parseMember();
}
recursionDepth -= ops.size();

Expand Down Expand Up @@ -1066,13 +1035,6 @@ private static boolean isAsciiAlphanumeric(char c) {
return null;
}

private int getLeftmostPosition(CelExpr expr) {
while (expr.exprKind().getKind() == CelExpr.ExprKind.Kind.SELECT) {
expr = expr.select().operand();
}
return getPosition(expr.id());
}

private @Nullable CelMacro lookupMacro(String id, int argCount, boolean receiverStyle) {
if (macros.isEmpty()) {
return null;
Expand Down Expand Up @@ -1188,7 +1150,7 @@ private int countGroupingParentheses() {
int size = content.size();
while (pos < size) {
int c = content.get(pos);
if (c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\f' && c != 11) {
if (c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\f') {
if (c == '/') {
// A comment might precede another '('.
break;
Expand Down
11 changes: 10 additions & 1 deletion parser/src/test/java/dev/cel/parser/CelParserParameterizedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ public void parser_core_syntax() {
runTest("a.b");
runTest("a.b.c");
runTest("a.?b");
runTest("a.?b.?c");
runTest("a.`b-c`");
runTest("a.`b c`");
runTest("a.`b.c`");
Expand Down Expand Up @@ -334,6 +335,8 @@ public void parser_core_syntax() {
runTest("! false");
runTest("-a");
runTest("---a");
runTest("!-42");
runTest("!-4.2");

// Arithmetic operators
runTest("x * 2");
Expand All @@ -359,6 +362,7 @@ public void parser_core_syntax() {
runTest("a > b");
runTest("a >= b");
runTest("a in b");
runTest("9in-x");
runTest("\"\ud83d\ude01\" in [\"\ud83d\ude01\", \"\ud83d\ude11\", \"\ud83d\ude26\"]");
runTest("size(x) == x.size()");
runTest("x.single_nested_message != null");
Expand All @@ -379,7 +383,7 @@ public void parser_core_syntax() {
runTest("cond ? 1 : 2");
runTest("false && !true || false ? 2 : 3");
runTest(OPTIONS_MAX_RECURSION_DEPTH_32, Strings.repeat("true ? 1 : ", 31) + "1", false);
runAntlrTest(OPTIONS_MAX_RECURSION_DEPTH_32, Strings.repeat("!-", 15) + "x");
runTest(OPTIONS_MAX_RECURSION_DEPTH_32, Strings.repeat("!-", 15) + "x", false);

// Complex expressions
runTest("1 + 2 * 3 - 1 / 2 == 6 % 1");
Expand Down Expand Up @@ -455,6 +459,7 @@ public void parser_errors() {
runTest("*@a | b");
runTest("((@))");
runTest("1 + $");
runTest("1 \u000b + 2");
runTest(
"\u00f3\u00a0\u00a2\n"
+ "\t\t\u00f3\u00a00\u00a0\n"
Expand All @@ -468,6 +473,8 @@ public void parser_errors() {

// Unexpected tokens
runTest("1 + +");
runTest("-!x");
runTest("!-x");
runTest("?");
runTest("a ? b ((?))");
runTest("a ? b @");
Expand Down Expand Up @@ -547,6 +554,7 @@ public void parser_errors() {
// Member selection errors
runTest("{\"a\": 1}.\"a\"");
runTest("self.true == 1");
runTest("a.in");

// Map syntax errors
runTest("{a}");
Expand Down Expand Up @@ -594,6 +602,7 @@ public void parser_errors() {
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "`b-c`");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "`b-c`()");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "a.`$b`");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "has(a.`$b`)");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "a.`b.c`()");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "`bar`");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "foo.``");
Expand Down
9 changes: 9 additions & 0 deletions parser/src/test/java/dev/cel/parser/PrattParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ public void pratt_parser_core_syntax() {
runTest("a.b");
runTest("a.b.c");
runTest("a.?b");
runTest("a.?b.?c");
runTest("a.`b-c`");
runTest("a.`b c`");
runTest("a.`b.c`");
Expand Down Expand Up @@ -257,6 +258,8 @@ public void pratt_parser_core_syntax() {
runTest("!x");
runTest("! false");
runTest("-a");
runTest("!-42");
runTest("!-4.2");

// Arithmetic operators
runTest("x * 2");
Expand All @@ -282,6 +285,7 @@ public void pratt_parser_core_syntax() {
runTest("a > b");
runTest("a >= b");
runTest("a in b");
runTest("9in-x");
runTest("\"\ud83d\ude01\" in [\"\ud83d\ude01\", \"\ud83d\ude11\", \"\ud83d\ude26\"]");
runTest("size(x) == x.size()");
runTest("x.single_nested_message != null");
Expand Down Expand Up @@ -358,6 +362,7 @@ public void pratt_parser_errors() {
runTest("*@a | b");
runTest("((@))");
runTest("1 + $");
runTest("1 \u000b + 2");
runTest(
"\u00f3\u00a0\u00a2\n"
+ "\t\t\u00f3\u00a00\u00a0\n"
Expand All @@ -368,6 +373,8 @@ public void pratt_parser_errors() {

// Unexpected tokens
runTest("1 + +");
runTest("-!x");
runTest("!-x");
runTest("?");
runTest("a ? b ((?))");
runTest("a ? b @");
Expand Down Expand Up @@ -427,6 +434,7 @@ public void pratt_parser_errors() {
// Member selection errors
runTest("{\"a\": 1}.\"a\"");
runTest("self.true == 1");
runTest("a.in");

// Map syntax errors
runTest("{a}");
Expand Down Expand Up @@ -458,6 +466,7 @@ public void pratt_parser_errors() {
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "`b-c`");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "`b-c`()");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "a.`$b`");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "has(a.`$b`)");
runTest(OPTIONS_QUOTED_IDENTIFIER_SYNTAX, "a.`b.c`()");

// Recursion limit exceeded
Expand Down
Loading
Loading