Skip to content

Commit ec3b86c

Browse files
dmitriplotnikovcopybara-github
dmitriplotnikov
authored and
copybara-github
committed
[Pratt Parser] Add a fuzzer comparing ANTLR and Pratt parser outputs and fix uncovered discrepancies
Added `CelPrattParserFuzzer` to fuzz CEL inputs against both `AntlrParser` and `PrattParser`, asserting error parity and AST equality. Parser differences uncovered and fixed in `PrattParser` / `Lexer`: 1. **Mixed unary operator chains (e.g., `-!ll`, `!-x`)**: ANTLR requires consecutive unary operators to be homogeneous (`!`/`-`) and only allows `-` after `!` when immediately followed by an integer or floating-point literal (e.g., `!-42`), whereas Pratt previously allowed arbitrary mixtures of `!` and `-` without parentheses. 2. **Vertical tab (`\v`, ASCII 11)**: ANTLR does not treat `\v` as whitespace, whereas `Lexer` and `PrattParser` previously skipped it. 3. **Unquoted `.in` field selector**: ANTLR treats `in` as a keyword token and rejects unquoted `x.in` (requiring backtick-quoted `` x.`in` ``), whereas Pratt previously accepted unquoted `.in` after `.`. 4. **Chained optional select (`T.?a.?a`) AST positions**: ANTLR records the position of the field constant in `_?._` at the start of the `member` expression (`T`), whereas Pratt stopped at intermediate `.?`/`[]`/`()` nodes. 5. **Numeric literals immediately followed by identifier characters (e.g., `9in-x`)**: ANTLR tokenizes numeric literals (`NUM_INT`, `NUM_UINT`, `NUM_FLOAT`) without rejecting trailing identifier characters so `9in-x` parses as `9 in -x`, whereas Pratt's `Lexer` previously rejected trailing identifier characters at lexing time. 6. **Invalid quoted field selectors inside `has(...)` (e.g., `` has(a.`$b`) ``)**: When `normalizeIdent()` rejects an invalid backtick-quoted field name, `PrattParser` previously still constructed a `CelSelect` with an empty field string, causing `CelExprFactory.newSelect()` to throw `IllegalArgumentException` during `has()` macro expansion instead of returning an unset error expression like `AntlrParser`. Intentional parser differences ignored by `CelPrattParserFuzzer` (where `PrattParser` behavior is preferred): 1. **Raw byte string literal prefixes (`rb'...'`, `rB'...'`, `Rb'...'`, `RB'...'`)**: ANTLR only accepts `br`/`bR`/`Br`/`BR` prefix order, whereas Pratt accepts both `br` and `rb`. 2. **Standalone commas in empty collection literals (`[,]`, `{,}`, `Msg{,}`)**: ANTLR accepts empty collections containing only a comma, whereas Pratt requires at least one element/entry before a trailing comma. 3. **Leading-dot identifier positions (`.R`)**: Pratt records the position of leading-dot identifiers at the `.` token, whereas ANTLR records it at the identifier token after `.`. PiperOrigin-RevId: 982225263
1 parent c182a1d commit ec3b86c

11 files changed

Lines changed: 610 additions & 128 deletions

File tree

‎common/src/main/java/dev/cel/common/internal/Constants.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ public final class Constants {
5151

5252
public static CelConstant parseInt(String text) throws ParseException {
5353
int base;
54-
if (text.startsWith("-0x")) {
54+
if (text.startsWith("-0x") || text.startsWith("-0X")) {
5555
base = 16;
5656
// Strip off the sign and prefix.
5757
text = text.substring(3);
5858
// Add the sign back.
5959
text = "-" + text;
60-
} else if (text.startsWith("0x")) {
60+
} else if (text.startsWith("0x") || text.startsWith("0X")) {
6161
base = 16;
6262
text = text.substring(2);
6363
if (text.startsWith("-")) {
@@ -83,7 +83,7 @@ public static CelConstant parseUint(String text) throws ParseException {
8383
throw new ParseException("Unsigned integer literal is missing trailing 'u' suffix", 0);
8484
}
8585
text = text.substring(0, text.length() - 1);
86-
if (text.startsWith("0x")) {
86+
if (text.startsWith("0x") || text.startsWith("0X")) {
8787
base = 16;
8888
text = text.substring(2);
8989
} else {

‎parser/src/main/java/dev/cel/parser/Lexer.java‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,6 @@ private void consumeWhitespaceAndComments() {
428428
case '\n':
429429
case ' ':
430430
case '\r':
431-
case 11: // \v
432431
case '\t':
433432
position++;
434433
break;
@@ -589,18 +588,12 @@ private Token consumeNumericLiteral() {
589588
}
590589
} else {
591590
advance(1);
592-
if (c == '0' && consume('x')) {
591+
if (c == '0' && (consume('x') || consume('X'))) {
593592
if (!consumeHexDigits()) {
594593
return setError(
595594
start, position, "integral literal missing digits after hexadecimal separator");
596595
}
597596
TokenType tokenType = consumeIntegralSuffix();
598-
if (consumeIf(Lexer::isIdentTrailing)) {
599-
return setError(
600-
start,
601-
position,
602-
tokenType.getSymbol() + " literal has unexpected trailing characters");
603-
}
604597
return makeToken(tokenType, start, position);
605598
}
606599
consumeDigits();
@@ -622,10 +615,6 @@ && isDigit(content.get(position + 1))) {
622615
}
623616
}
624617
TokenType tokenType = floatingPoint ? TokenType.FLOAT : consumeIntegralSuffix();
625-
if (consumeIf(Lexer::isIdentTrailing)) {
626-
return setError(
627-
start, position, tokenType.getSymbol() + " literal has unexpected trailing characters");
628-
}
629618
return makeToken(tokenType, start, position);
630619
}
631620

‎parser/src/main/java/dev/cel/parser/PrattParser.java‎

Lines changed: 82 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import com.google.common.collect.ImmutableList;
1818
import com.google.common.collect.ImmutableMap;
19-
import com.google.common.collect.Iterables;
2019
import dev.cel.common.CelAbstractSyntaxTree;
2120
import dev.cel.common.CelIssue;
2221
import dev.cel.common.CelOptions;
@@ -524,21 +523,36 @@ private static CelExpr buildUnaryCall(long id, String function, CelExpr operand)
524523

525524
private CelExpr parseSelectorChain() {
526525
Lexer.TokenType tok = peekToken.type;
527-
CelExpr lhs =
528-
(tok == Lexer.TokenType.EXCLAMATION || tok == Lexer.TokenType.MINUS)
529-
? parseUnaryOps()
530-
: parsePrimary();
526+
if (tok == Lexer.TokenType.EXCLAMATION || tok == Lexer.TokenType.MINUS) {
527+
return parseUnaryOps();
528+
}
529+
return parseMember();
530+
}
531+
532+
private CelExpr parseMember() {
533+
int memberStart = peekToken.start;
534+
boolean startedWithIdentOrDot =
535+
peekToken.type == Lexer.TokenType.DOT
536+
|| peekToken.type == Lexer.TokenType.IDENT
537+
|| peekToken.type == Lexer.TokenType.RESERVED_WORD;
538+
CelExpr lhs = parsePrimary();
539+
boolean canBeStructName =
540+
startedWithIdentOrDot
541+
&& (currentToken.type == Lexer.TokenType.IDENT
542+
|| currentToken.type == Lexer.TokenType.RESERVED_WORD)
543+
&& !isQuotedIdent(currentToken);
531544
currentLhsDepth = 0;
532-
tok = peekToken.type;
545+
Lexer.TokenType tok = peekToken.type;
533546
if (tok == Lexer.TokenType.DOT
534547
|| tok == Lexer.TokenType.LEFT_BRACKET
535548
|| tok == Lexer.TokenType.LEFT_BRACE) {
536-
lhs = parseSelectorChainTail(lhs);
549+
lhs = parseSelectorChainTail(lhs, memberStart, canBeStructName);
537550
}
538551
return lhs;
539552
}
540553

541-
private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
554+
private CelExpr parseSelectorChainTail(
555+
CelExpr initialLhs, int memberStartPosition, boolean canBeStructName) {
542556
CelExpr lhs = initialLhs;
543557
int chainDepth = 0;
544558
while (true) {
@@ -558,9 +572,7 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
558572
}
559573
}
560574
Lexer.Token idTok = nextToken();
561-
if (idTok.type != Lexer.TokenType.IDENT
562-
&& idTok.type != Lexer.TokenType.RESERVED_WORD
563-
&& idTok.type != Lexer.TokenType.IN) {
575+
if (idTok.type != Lexer.TokenType.IDENT && idTok.type != Lexer.TokenType.RESERVED_WORD) {
564576
if (idTok.type != Lexer.TokenType.ERROR) {
565577
reportSyntaxError(idTok, "expected identifier after '.'");
566578
}
@@ -570,11 +582,17 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
570582
}
571583
boolean isMemberCall = (peekToken.type == Lexer.TokenType.LEFT_PAREN);
572584
String idText = normalizeIdent(idTok, /* allowQuoted= */ !isMemberCall);
585+
if (idText.isEmpty()) {
586+
synchronizeOnDelimiter();
587+
currentLhsDepth = chainDepth;
588+
return ERROR;
589+
}
573590
if (optional) {
574591
long opId = nextId(dotTok);
575592
CelExpr field =
576-
CelExpr.ofConstant(nextId(getLeftmostPosition(lhs)), CelConstant.ofValue(idText));
593+
CelExpr.ofConstant(nextId(memberStartPosition), CelConstant.ofValue(idText));
577594
lhs = buildBinaryCall(opId, Operator.OPTIONAL_SELECT.getFunction(), lhs, field);
595+
canBeStructName = false;
578596
} else if (peekToken.type == Lexer.TokenType.LEFT_PAREN) {
579597
Lexer.Token lparen = nextToken();
580598
long callId = nextId(lparen);
@@ -584,8 +602,10 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
584602
expanded.isPresent()
585603
? expanded.get()
586604
: CelExpr.ofCall(callId, Optional.of(lhs), idText, args);
605+
canBeStructName = false;
587606
} else {
588607
lhs = CelExpr.ofSelect(nextId(dotTok), lhs, idText, /* isTestOnly= */ false);
608+
canBeStructName = canBeStructName && !isQuotedIdent(idTok);
589609
}
590610
} else if (tok == Lexer.TokenType.LEFT_BRACKET) {
591611
if (checkRecursion(chainDepth, peekToken)) {
@@ -607,12 +627,17 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
607627
String opName =
608628
optional ? Operator.OPTIONAL_INDEX.getFunction() : Operator.INDEX.getFunction();
609629
lhs = buildBinaryCall(opId, opName, lhs, index);
630+
canBeStructName = false;
610631
} else if (tok == Lexer.TokenType.LEFT_BRACE) {
632+
if (!canBeStructName) {
633+
break;
634+
}
611635
String structName = extractStructName(lhs);
612636
if (structName == null) {
613637
break;
614638
}
615639
lhs = parseStruct(nextId(peekToken.start), structName);
640+
canBeStructName = false;
616641
} else {
617642
break;
618643
}
@@ -622,83 +647,42 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) {
622647
}
623648

624649
private CelExpr parseUnaryOps() {
625-
Lexer.Token op = nextToken();
626-
Lexer.TokenType opType = op.type;
627-
if (peekToken.type == Lexer.TokenType.EXCLAMATION || peekToken.type == Lexer.TokenType.MINUS) {
628-
return parseUnaryOpsChain(op);
629-
}
630-
631-
if (opType == Lexer.TokenType.MINUS) {
632-
if (peekToken.type == Lexer.TokenType.INT) {
633-
return parseIntLiteral(nextId(peekToken), /* isNegative= */ true);
634-
}
635-
if (peekToken.type == Lexer.TokenType.FLOAT) {
636-
return parseDoubleLiteral(nextId(peekToken), /* isNegative= */ true);
637-
}
638-
}
639-
640-
if (checkRecursion(0, op)) {
641-
return ERROR;
642-
}
643-
644-
long opId = nextId(op);
645-
recursionDepth++;
646-
CelExpr operand = parseSelectorChain();
647-
recursionDepth--;
648-
if (recursionLimitExceeded) {
649-
return ERROR;
650-
}
651-
652-
String opName =
653-
(opType == Lexer.TokenType.EXCLAMATION)
654-
? Operator.LOGICAL_NOT.getFunction()
655-
: Operator.NEGATE.getFunction();
656-
return buildUnaryCall(opId, opName, operand);
657-
}
658-
659-
private CelExpr parseUnaryOpsChain(Lexer.Token firstOp) {
650+
Lexer.Token firstOp = nextToken();
651+
Lexer.TokenType opType = firstOp.type;
660652
List<UnaryOp> ops = new ArrayList<>();
661653
ops.add(new UnaryOp(firstOp));
662-
while (peekToken.type == Lexer.TokenType.EXCLAMATION
663-
|| peekToken.type == Lexer.TokenType.MINUS) {
654+
while (peekToken.type == opType) {
664655
ops.add(new UnaryOp(nextToken()));
665656
}
666657

667-
boolean hasSolitaryTrailingMinus =
668-
!ops.isEmpty()
669-
&& Iterables.getLast(ops).token.type == Lexer.TokenType.MINUS
670-
&& (ops.size() == 1 || ops.get(ops.size() - 2).token.type != Lexer.TokenType.MINUS);
671-
672-
if (!options.retainRepeatedUnaryOperators()) {
673-
int write = 0;
674-
for (int read = 0; read < ops.size(); ) {
675-
int next = read;
676-
while (next < ops.size() && ops.get(next).token.type == ops.get(read).token.type) {
677-
next++;
678-
}
679-
if ((next - read) % 2 != 0) {
680-
ops.set(write++, ops.get(read));
681-
}
682-
read = next;
658+
if (opType == Lexer.TokenType.MINUS
659+
&& ops.size() == 1
660+
&& (peekToken.type == Lexer.TokenType.INT || peekToken.type == Lexer.TokenType.FLOAT)) {
661+
CelExpr lhs =
662+
(peekToken.type == Lexer.TokenType.INT)
663+
? parseIntLiteral(nextId(peekToken), /* isNegative= */ true)
664+
: parseDoubleLiteral(nextId(peekToken), /* isNegative= */ true);
665+
currentLhsDepth = 0;
666+
Lexer.TokenType tok = peekToken.type;
667+
if (tok == Lexer.TokenType.DOT
668+
|| tok == Lexer.TokenType.LEFT_BRACKET
669+
|| tok == Lexer.TokenType.LEFT_BRACE) {
670+
lhs = parseSelectorChainTail(lhs, firstOp.start, /* canBeStructName= */ false);
683671
}
684-
ops = new ArrayList<>(ops.subList(0, write));
685-
}
686-
687-
for (UnaryOp op : ops) {
688-
op.id = nextId(op.token);
672+
return lhs;
689673
}
690674

691-
boolean isNegativeNumericLiteral =
692-
hasSolitaryTrailingMinus
693-
&& (peekToken.type == Lexer.TokenType.INT || peekToken.type == Lexer.TokenType.FLOAT);
694-
long negativeLiteralOpId = 0;
695-
if (isNegativeNumericLiteral) {
696-
negativeLiteralOpId = Iterables.getLast(ops).id;
697-
ops.remove(ops.size() - 1);
675+
if (!options.retainRepeatedUnaryOperators()) {
676+
if (ops.size() % 2 == 0) {
677+
ops.clear();
678+
} else {
679+
ops = new ArrayList<>(ops.subList(0, 1));
680+
}
698681
}
699682

700683
int chainDepth = 0;
701684
for (UnaryOp op : ops) {
685+
op.id = nextId(op.token);
702686
if (checkRecursion(chainDepth, op.token)) {
703687
return ERROR;
704688
}
@@ -707,14 +691,20 @@ private CelExpr parseUnaryOpsChain(Lexer.Token firstOp) {
707691

708692
recursionDepth += ops.size();
709693
CelExpr operand;
710-
if (isNegativeNumericLiteral) {
711-
operand =
712-
(peekToken.type == Lexer.TokenType.INT)
713-
? parseIntLiteral(negativeLiteralOpId, /* isNegative= */ true)
714-
: parseDoubleLiteral(negativeLiteralOpId, /* isNegative= */ true);
715-
operand = parseSelectorChainTail(operand);
694+
if (opType == Lexer.TokenType.EXCLAMATION && peekToken.type == Lexer.TokenType.MINUS) {
695+
Lexer.Token minusTok = nextToken();
696+
if (peekToken.type == Lexer.TokenType.INT) {
697+
operand = parseIntLiteral(nextId(peekToken), /* isNegative= */ true);
698+
operand = parseSelectorChainTail(operand, minusTok.start, /* canBeStructName= */ false);
699+
} else if (peekToken.type == Lexer.TokenType.FLOAT) {
700+
operand = parseDoubleLiteral(nextId(peekToken), /* isNegative= */ true);
701+
operand = parseSelectorChainTail(operand, minusTok.start, /* canBeStructName= */ false);
702+
} else {
703+
reportSyntaxError(minusTok, "unexpected '-'");
704+
operand = parseMember();
705+
}
716706
} else {
717-
operand = parseSelectorChain();
707+
operand = parseMember();
718708
}
719709
recursionDepth -= ops.size();
720710

@@ -1044,6 +1034,13 @@ private String normalizeIdent(Lexer.Token tok, boolean allowQuoted) {
10441034
return text;
10451035
}
10461036

1037+
private boolean isQuotedIdent(Lexer.Token tok) {
1038+
if (tok.text != null) {
1039+
return !tok.text.isEmpty() && tok.text.charAt(0) == '`';
1040+
}
1041+
return tok.start >= 0 && tok.start < tok.end && content.get(tok.start) == '`';
1042+
}
1043+
10471044
private static boolean isAsciiAlphanumeric(char c) {
10481045
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9');
10491046
}
@@ -1066,13 +1063,6 @@ private static boolean isAsciiAlphanumeric(char c) {
10661063
return null;
10671064
}
10681065

1069-
private int getLeftmostPosition(CelExpr expr) {
1070-
while (expr.exprKind().getKind() == CelExpr.ExprKind.Kind.SELECT) {
1071-
expr = expr.select().operand();
1072-
}
1073-
return getPosition(expr.id());
1074-
}
1075-
10761066
private @Nullable CelMacro lookupMacro(String id, int argCount, boolean receiverStyle) {
10771067
if (macros.isEmpty()) {
10781068
return null;
@@ -1188,7 +1178,7 @@ private int countGroupingParentheses() {
11881178
int size = content.size();
11891179
while (pos < size) {
11901180
int c = content.get(pos);
1191-
if (c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\f' && c != 11) {
1181+
if (c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\f') {
11921182
if (c == '/') {
11931183
// A comment might precede another '('.
11941184
break;

0 commit comments

Comments
 (0)