diff --git a/parser/src/main/java/dev/cel/parser/BUILD.bazel b/parser/src/main/java/dev/cel/parser/BUILD.bazel index 848209380..55183446f 100644 --- a/parser/src/main/java/dev/cel/parser/BUILD.bazel +++ b/parser/src/main/java/dev/cel/parser/BUILD.bazel @@ -118,6 +118,7 @@ java_library( "//common:source_location", "//common/ast", "//common/internal", + "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", "@maven//:org_jspecify_jspecify", ], diff --git a/parser/src/main/java/dev/cel/parser/Lexer.java b/parser/src/main/java/dev/cel/parser/Lexer.java index 894cda9ce..32b6ebfab 100644 --- a/parser/src/main/java/dev/cel/parser/Lexer.java +++ b/parser/src/main/java/dev/cel/parser/Lexer.java @@ -29,8 +29,6 @@ final class Lexer { enum TokenType { ERROR("error"), END("end"), - WHITESPACE("whitespace"), - COMMENT("comment"), // Keywords NULL("null"), @@ -98,11 +96,17 @@ static final class Token { final TokenType type; final int start; final int end; + final @Nullable String text; Token(TokenType type, int start, int end) { + this(type, start, end, null); + } + + Token(TokenType type, int start, int end, @Nullable String text) { this.type = type; this.start = start; this.end = end; + this.text = text; } @Override @@ -149,35 +153,28 @@ static final class LexerError { .buildOrThrow(); private final CelCodePointArray content; + private final int size; private int position; private LexerError error; Lexer(CelCodePointArray content) { this.content = content; + this.size = content.size(); this.position = 0; this.error = null; } Token lex() { + consumeWhitespaceAndComments(); int start = position; - if (position >= content.size()) { + if (position >= size) { return makeToken(TokenType.END, start, start); } int c = content.get(position); switch (c) { - case '\f': - case '\n': - case ' ': - case '\r': - case 0x0B: // \v (vertical tab) - case '\t': - { - consumeWhitespace(); - return makeToken(TokenType.WHITESPACE, start, position); - } case '.': { - if (position + 1 < content.size() && isDigit(content.get(position + 1))) { + if (position + 1 < size && isDigit(content.get(position + 1))) { return consumeNumericLiteral(); } advance(1); @@ -283,10 +280,6 @@ Token lex() { case '/': { advance(1); - if (consume('/')) { - consumeLine(); - return makeToken(TokenType.COMMENT, start, position); - } return makeToken(TokenType.SLASH, start, position); } case '&': @@ -381,6 +374,10 @@ private Token makeToken(TokenType type, int start, int end) { return new Token(type, start, end); } + private Token makeToken(TokenType type, int start, int end, @Nullable String text) { + return new Token(type, start, end, text); + } + private Token setError(int start, int end, String message) { this.error = new LexerError(start, end, message); return new Token(TokenType.ERROR, start, end); @@ -391,7 +388,7 @@ private void advance(int n) { } private boolean match(int c) { - return position < content.size() && content.get(position) == c; + return position < size && content.get(position) == c; } private boolean consume(int c) { @@ -403,7 +400,7 @@ private boolean consume(int c) { } private boolean consumeIf(IntPredicate predicate) { - if (position < content.size()) { + if (position < size) { int cp = content.get(position); if (predicate.test(cp)) { advance(1); @@ -414,7 +411,7 @@ private boolean consumeIf(IntPredicate predicate) { } private void consumeLine() { - while (position < content.size()) { + while (position < size) { if (content.get(position) == '\n') { advance(1); return; @@ -423,8 +420,8 @@ private void consumeLine() { } } - private void consumeWhitespace() { - while (position < content.size()) { + private void consumeWhitespaceAndComments() { + while (position < size) { int c = content.get(position); switch (c) { case '\f': @@ -433,8 +430,15 @@ private void consumeWhitespace() { case '\r': case 11: // \v case '\t': - advance(1); + position++; break; + case '/': + if (position + 1 < size && content.get(position + 1) == '/') { + consumeLine(); + break; + } else { + return; + } default: return; } @@ -442,29 +446,19 @@ private void consumeWhitespace() { } private boolean consumeDigits() { - boolean advanced = false; - while (position < content.size()) { - int c = content.get(position); - if (!isDigit(c)) { - break; - } - advance(1); - advanced = true; + int start = position; + while (position < size && isDigit(content.get(position))) { + position++; } - return advanced; + return position > start; } private boolean consumeHexDigits() { - boolean advanced = false; - while (position < content.size()) { - int c = content.get(position); - if (!isHexDigit(c)) { - break; - } - advance(1); - advanced = true; + int start = position; + while (position < size && isHexDigit(content.get(position))) { + position++; } - return advanced; + return position > start; } private TokenType consumeIntegralSuffix() { @@ -486,7 +480,7 @@ private Token consumeQuotedIdent() { private boolean consumeUntilAfter(int c, boolean isRaw) { int pos = position; boolean escaped = false; - while (pos < content.size()) { + while (pos < size) { int cc = content.get(pos); if (cc == '\n' || cc == '\r') { position = pos; @@ -503,20 +497,20 @@ private boolean consumeUntilAfter(int c, boolean isRaw) { } pos++; } - position = content.size(); + position = size; return false; } private boolean consumeUntilAfterTripleQuote(int quote, boolean isRaw) { int pos = position; boolean escaped = false; - while (pos < content.size()) { + while (pos < size) { int cc = content.get(pos); if (!isRaw && cc == '\\') { escaped = !escaped; } else { if ((isRaw || !escaped) - && pos + 2 < content.size() + && pos + 2 < size && cc == quote && content.get(pos + 1) == quote && content.get(pos + 2) == quote) { @@ -527,16 +521,14 @@ private boolean consumeUntilAfterTripleQuote(int quote, boolean isRaw) { } pos++; } - position = content.size(); + position = size; return false; } private Token consumeStringLiteral(int start, int quote, boolean isBytes, boolean isRaw) { advance(1); boolean isTripleQuote = - position + 1 < content.size() - && content.get(position) == quote - && content.get(position + 1) == quote; + position + 1 < size && content.get(position) == quote && content.get(position + 1) == quote; if (isTripleQuote) { advance(2); if (!consumeUntilAfterTripleQuote(quote, isRaw)) { @@ -556,7 +548,7 @@ private Token consumeStringLiteral(int start, int quote, boolean isBytes, boolea private @Nullable Token consumePrefixedStringLiteral() { int start = position; - if (position >= content.size()) { + if (position >= size) { return null; } int c = content.get(position); @@ -566,7 +558,7 @@ private Token consumeStringLiteral(int start, int quote, boolean isBytes, boolea return null; } int lookahead = 1; - if (position + 1 < content.size()) { + if (position + 1 < size) { int c2 = content.get(position + 1); if (isBytes ? (c2 == 'r' || c2 == 'R') : (c2 == 'b' || c2 == 'B')) { isBytes = true; @@ -574,7 +566,7 @@ private Token consumeStringLiteral(int start, int quote, boolean isBytes, boolea lookahead = 2; } } - if (position + lookahead < content.size()) { + if (position + lookahead < size) { int quote = content.get(position + lookahead); if (quote == '"' || quote == '\'') { advance(lookahead); @@ -612,9 +604,9 @@ private Token consumeNumericLiteral() { return makeToken(tokenType, start, position); } consumeDigits(); - if (position < content.size() + if (position < size && content.get(position) == '.' - && position + 1 < content.size() + && position + 1 < size && isDigit(content.get(position + 1))) { floatingPoint = true; advance(1); @@ -639,12 +631,8 @@ && isDigit(content.get(position + 1))) { private Token consumeIdent() { int start = position; - while (position < content.size()) { - int c = content.get(position); - if (!isIdentTrailing(c)) { - break; - } - advance(1); + while (position < size && isIdentTrailing(content.get(position))) { + position++; } int end = position; String word = content.slice(start, end).toString(); @@ -652,6 +640,6 @@ private Token consumeIdent() { if (keywordType != null) { return makeToken(keywordType, start, end); } - return makeToken(TokenType.IDENT, start, end); + return makeToken(TokenType.IDENT, start, end, word); } } diff --git a/parser/src/main/java/dev/cel/parser/PrattParser.java b/parser/src/main/java/dev/cel/parser/PrattParser.java index 17ce514d5..3ddb0baad 100644 --- a/parser/src/main/java/dev/cel/parser/PrattParser.java +++ b/parser/src/main/java/dev/cel/parser/PrattParser.java @@ -30,6 +30,7 @@ import java.text.ParseException; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -41,6 +42,9 @@ final class PrattParser { private static final String ACCUMULATOR_NAME = "@result"; private static final CelExpr ERROR = CelExpr.newBuilder().setConstant(Constants.ERROR).build(); + private static final Lexer.Token END_TOKEN = new Lexer.Token(Lexer.TokenType.END, 0, 0); + /** Most logical chains are short; 8 avoids resizing for the overwhelming majority. */ + private static final int INITIAL_CHAIN_CAPACITY = 8; private static final class BinaryOpInfo { final int precedence; @@ -56,72 +60,49 @@ private static final class BinaryOpInfo { } } - private static final BinaryOpInfo LOGICAL_OR_OP = - new BinaryOpInfo(1, Operator.LOGICAL_OR.getFunction(), true, Lexer.TokenType.LOGICAL_OR); - private static final BinaryOpInfo LOGICAL_AND_OP = - new BinaryOpInfo(2, Operator.LOGICAL_AND.getFunction(), true, Lexer.TokenType.LOGICAL_AND); - private static final BinaryOpInfo LESS_OP = - new BinaryOpInfo(3, Operator.LESS.getFunction(), false, Lexer.TokenType.LESS); - private static final BinaryOpInfo LESS_EQUAL_OP = - new BinaryOpInfo(3, Operator.LESS_EQUALS.getFunction(), false, Lexer.TokenType.LESS_EQUAL); - private static final BinaryOpInfo GREATER_OP = - new BinaryOpInfo(3, Operator.GREATER.getFunction(), false, Lexer.TokenType.GREATER); - private static final BinaryOpInfo GREATER_EQUAL_OP = - new BinaryOpInfo( - 3, Operator.GREATER_EQUALS.getFunction(), false, Lexer.TokenType.GREATER_EQUAL); - private static final BinaryOpInfo EQUAL_EQUAL_OP = - new BinaryOpInfo(3, Operator.EQUALS.getFunction(), false, Lexer.TokenType.EQUAL_EQUAL); - private static final BinaryOpInfo EXCLAMATION_EQUAL_OP = - new BinaryOpInfo( - 3, Operator.NOT_EQUALS.getFunction(), false, Lexer.TokenType.EXCLAMATION_EQUAL); - private static final BinaryOpInfo IN_OP = - new BinaryOpInfo(3, Operator.IN.getFunction(), false, Lexer.TokenType.IN); - private static final BinaryOpInfo PLUS_OP = - new BinaryOpInfo(4, Operator.ADD.getFunction(), false, Lexer.TokenType.PLUS); - private static final BinaryOpInfo MINUS_OP = - new BinaryOpInfo(4, Operator.SUBTRACT.getFunction(), false, Lexer.TokenType.MINUS); - private static final BinaryOpInfo ASTERISK_OP = - new BinaryOpInfo(5, Operator.MULTIPLY.getFunction(), false, Lexer.TokenType.ASTERISK); - private static final BinaryOpInfo SLASH_OP = - new BinaryOpInfo(5, Operator.DIVIDE.getFunction(), false, Lexer.TokenType.SLASH); - private static final BinaryOpInfo PERCENT_OP = - new BinaryOpInfo(5, Operator.MODULO.getFunction(), false, Lexer.TokenType.PERCENT); - private static final BinaryOpInfo DEFAULT_OP = - new BinaryOpInfo(0, "", false, Lexer.TokenType.ERROR); - - private static BinaryOpInfo getBinaryOpInfo(Lexer.TokenType type) { - switch (type) { - case LOGICAL_OR: - return LOGICAL_OR_OP; - case LOGICAL_AND: - return LOGICAL_AND_OP; - case LESS: - return LESS_OP; - case LESS_EQUAL: - return LESS_EQUAL_OP; - case GREATER: - return GREATER_OP; - case GREATER_EQUAL: - return GREATER_EQUAL_OP; - case EQUAL_EQUAL: - return EQUAL_EQUAL_OP; - case EXCLAMATION_EQUAL: - return EXCLAMATION_EQUAL_OP; - case IN: - return IN_OP; - case PLUS: - return PLUS_OP; - case MINUS: - return MINUS_OP; - case ASTERISK: - return ASTERISK_OP; - case SLASH: - return SLASH_OP; - case PERCENT: - return PERCENT_OP; - default: - return DEFAULT_OP; - } + private static final BinaryOpInfo[] binaryOps = initBinaryOps(); + + // Safe and desirable to use .ordinal() here: + // 1. Safe: This lookup table is strictly private and internal to PrattParser, never serialized or + // persisted. The array is sized to TokenType.values().length, so indexing by ordinal is + // guaranteed to be within bounds even if enum members change. + // 2. Desirable: Expression parsing checks binary operator info on every token in the input; + // direct array indexing by ordinal provides O(1) lookup with zero hashing, indirection, + // or boxing overhead on this critical hot path. + @SuppressWarnings("EnumOrdinal") + private static BinaryOpInfo[] initBinaryOps() { + BinaryOpInfo[] ops = new BinaryOpInfo[Lexer.TokenType.values().length]; + ops[Lexer.TokenType.LOGICAL_OR.ordinal()] = + new BinaryOpInfo(1, Operator.LOGICAL_OR.getFunction(), true, Lexer.TokenType.LOGICAL_OR); + ops[Lexer.TokenType.LOGICAL_AND.ordinal()] = + new BinaryOpInfo(2, Operator.LOGICAL_AND.getFunction(), true, Lexer.TokenType.LOGICAL_AND); + ops[Lexer.TokenType.LESS.ordinal()] = + new BinaryOpInfo(3, Operator.LESS.getFunction(), false, Lexer.TokenType.LESS); + ops[Lexer.TokenType.LESS_EQUAL.ordinal()] = + new BinaryOpInfo(3, Operator.LESS_EQUALS.getFunction(), false, Lexer.TokenType.LESS_EQUAL); + ops[Lexer.TokenType.GREATER.ordinal()] = + new BinaryOpInfo(3, Operator.GREATER.getFunction(), false, Lexer.TokenType.GREATER); + ops[Lexer.TokenType.GREATER_EQUAL.ordinal()] = + new BinaryOpInfo( + 3, Operator.GREATER_EQUALS.getFunction(), false, Lexer.TokenType.GREATER_EQUAL); + ops[Lexer.TokenType.EQUAL_EQUAL.ordinal()] = + new BinaryOpInfo(3, Operator.EQUALS.getFunction(), false, Lexer.TokenType.EQUAL_EQUAL); + ops[Lexer.TokenType.EXCLAMATION_EQUAL.ordinal()] = + new BinaryOpInfo( + 3, Operator.NOT_EQUALS.getFunction(), false, Lexer.TokenType.EXCLAMATION_EQUAL); + ops[Lexer.TokenType.IN.ordinal()] = + new BinaryOpInfo(3, Operator.IN.getFunction(), false, Lexer.TokenType.IN); + ops[Lexer.TokenType.PLUS.ordinal()] = + new BinaryOpInfo(4, Operator.ADD.getFunction(), false, Lexer.TokenType.PLUS); + ops[Lexer.TokenType.MINUS.ordinal()] = + new BinaryOpInfo(4, Operator.SUBTRACT.getFunction(), false, Lexer.TokenType.MINUS); + ops[Lexer.TokenType.ASTERISK.ordinal()] = + new BinaryOpInfo(5, Operator.MULTIPLY.getFunction(), false, Lexer.TokenType.ASTERISK); + ops[Lexer.TokenType.SLASH.ordinal()] = + new BinaryOpInfo(5, Operator.DIVIDE.getFunction(), false, Lexer.TokenType.SLASH); + ops[Lexer.TokenType.PERCENT.ordinal()] = + new BinaryOpInfo(5, Operator.MODULO.getFunction(), false, Lexer.TokenType.PERCENT); + return ops; } private static final class UnaryOp { @@ -138,10 +119,9 @@ private static final class UnaryOp { private final ImmutableMap macros; private final Lexer lexer; private final Map positions; - private final Map macroCalls; + private Map macroCalls = ImmutableMap.of(); + private PrattMacroExprFactory macroExprFactory; private final List issues; - private final PrattMacroExprFactory macroExprFactory; - private Lexer.Token currentToken; private Lexer.Token peekToken; private int recursionDepth; @@ -184,11 +164,9 @@ private PrattParser(CelSource source, CelOptions options, Map this.macros = ImmutableMap.copyOf(macros); this.lexer = new Lexer(source.getContent()); this.positions = new HashMap<>(); - this.macroCalls = new HashMap<>(); this.issues = new ArrayList<>(); - this.macroExprFactory = new PrattMacroExprFactory(); this.nextId = 1; - initTokenStream(); + peekToken = nextSignificantToken(true); } CelExpr run() { @@ -215,11 +193,10 @@ private boolean isRecoveryLimitExceeded() { return errorCount > options.maxParseErrorRecoveryLimit(); } - private void initTokenStream() { - peekToken = nextSignificantToken(true); - } - private String getTokenText(Lexer.Token tok) { + if (tok.text != null) { + return tok.text; + } if (tok.start >= 0 && tok.end >= tok.start && tok.end <= source.getContent().size()) { return source.getContent().slice(tok.start, tok.end).toString(); } @@ -227,28 +204,20 @@ private String getTokenText(Lexer.Token tok) { } private Lexer.Token nextSignificantToken(boolean reportError) { - if (isRecoveryLimitExceeded()) { - return new Lexer.Token(Lexer.TokenType.END, 0, 0); - } - while (true) { - Lexer.Token tok = lexer.lex(); - if (tok.type == Lexer.TokenType.WHITESPACE || tok.type == Lexer.TokenType.COMMENT) { - continue; + Lexer.Token tok = lexer.lex(); + if (tok.type == Lexer.TokenType.ERROR && reportError) { + reportSyntaxError(tok, lexer.getError().message); + if (isRecoveryLimitExceeded()) { + return END_TOKEN; } - if (tok.type == Lexer.TokenType.ERROR && reportError) { - reportSyntaxError(tok, lexer.getError().message); - if (isRecoveryLimitExceeded()) { - return new Lexer.Token(Lexer.TokenType.END, 0, 0); - } - } - return tok; } + return tok; } private Lexer.Token nextToken() { currentToken = peekToken; if (isRecoveryLimitExceeded()) { - peekToken = new Lexer.Token(Lexer.TokenType.END, 0, 0); + peekToken = END_TOKEN; return currentToken; } if (peekToken.type != Lexer.TokenType.END) { @@ -283,7 +252,7 @@ private boolean expect(Lexer.TokenType type, String msg) { private void synchronizeOnDelimiter() { if (isRecoveryLimitExceeded()) { - peekToken = new Lexer.Token(Lexer.TokenType.END, 0, 0); + peekToken = END_TOKEN; return; } while (peekToken.type != Lexer.TokenType.END) { @@ -356,7 +325,7 @@ private void reportError(CelSourceLocation loc, String msg) { CelIssue.formatError( CelSourceLocation.NONE, String.format("More than %d parse errors.", options.maxParseErrorRecoveryLimit()))); - peekToken = new Lexer.Token(Lexer.TokenType.END, 0, 0); + peekToken = END_TOKEN; } if (errorCount <= options.maxParseErrorRecoveryLimit()) { issues.add(CelIssue.formatError(loc, msg)); @@ -369,33 +338,37 @@ private void reportSyntaxError(Lexer.Token token, String msg) { private boolean checkRecursion(int chainDepth, Lexer.Token token) { if (recursionDepth + chainDepth > options.maxParseRecursionDepth()) { - if (!recursionLimitExceeded) { - recursionLimitExceeded = true; - reportError( - token.start, - String.format( - "Expression recursion limit exceeded. limit: %d", - options.maxParseRecursionDepth())); - } + reportRecursionLimit(token.start); return true; } return false; } + private void reportRecursionLimit(int position) { + if (!recursionLimitExceeded) { + recursionLimitExceeded = true; + reportError( + position, + String.format( + "Expression recursion limit exceeded. limit: %d", options.maxParseRecursionDepth())); + } + } + private CelExpr parseExpr() { - if (recursionLimitExceeded || isRecoveryLimitExceeded()) { + if (recursionLimitExceeded || errorCount > options.maxParseErrorRecoveryLimit()) { return ERROR; } - recursionDepth++; - if (checkRecursion(0, peekToken)) { - recursionDepth--; + if (recursionDepth >= options.maxParseRecursionDepth()) { + reportRecursionLimit(peekToken.start); return ERROR; } + recursionDepth++; CelExpr expr = parseBinaryAndTernary(0); recursionDepth--; return expr; } + @SuppressWarnings("EnumOrdinal") // Using ordinal for O(1) binary operator lookup table private CelExpr parseBinaryAndTernary(int minPrec) { CelExpr lhs = parseSelectorChain(); int chainDepth = currentLhsDepth; @@ -406,8 +379,8 @@ private CelExpr parseBinaryAndTernary(int minPrec) { continue; } - BinaryOpInfo opInfo = getBinaryOpInfo(tok); - if (opInfo.precedence < minPrec || opInfo.precedence == 0) { + BinaryOpInfo opInfo = binaryOps[tok.ordinal()]; + if (opInfo == null || opInfo.precedence < minPrec) { break; } @@ -417,7 +390,8 @@ private CelExpr parseBinaryAndTernary(int minPrec) { } Lexer.Token opTok = nextToken(); - if (checkRecursion(chainDepth, opTok)) { + if (recursionDepth + chainDepth > options.maxParseRecursionDepth()) { + reportRecursionLimit(opTok.start); return ERROR; } chainDepth++; @@ -449,51 +423,60 @@ private CelExpr parseTernary(CelExpr lhs) { .build(); } - private CelExpr buildBinaryCall(long opId, String opName, CelExpr lhs, CelExpr rhs) { - return CelExpr.newBuilder() - .setId(opId) - .setCall(CelExpr.CelCall.newBuilder().setFunction(opName).addArgs(lhs).addArgs(rhs).build()) - .build(); - } - private CelExpr parseBalancedLogicalChain(CelExpr lhs, BinaryOpInfo opInfo) { - List terms = new ArrayList<>(); - List ops = new ArrayList<>(); - terms.add(lhs); + Lexer.Token opTok = nextToken(); + long opId = nextId(opTok.start); + CelExpr rhs = parseBinaryAndTernary(opInfo.precedence + 1); + if (peekToken.type != opInfo.type) { + return buildBinaryCall(opId, opInfo.name, lhs, rhs); + } + + CelExpr[] terms = new CelExpr[INITIAL_CHAIN_CAPACITY]; + long[] ops = new long[INITIAL_CHAIN_CAPACITY]; + terms[0] = lhs; + terms[1] = rhs; + ops[0] = opId; + int opsCount = 1; + int termsCount = 2; + while (peekToken.type == opInfo.type) { - Lexer.Token opTok = nextToken(); - long opId = nextId(opTok); - CelExpr rhs = parseBinaryAndTernary(opInfo.precedence + 1); - ops.add(opId); - terms.add(rhs); + opTok = nextToken(); + opId = nextId(opTok.start); + rhs = parseBinaryAndTernary(opInfo.precedence + 1); + if (termsCount == terms.length) { + int newCapacity = terms.length * 2; + ops = Arrays.copyOf(ops, newCapacity); + terms = Arrays.copyOf(terms, newCapacity); + } + ops[opsCount++] = opId; + terms[termsCount++] = rhs; } - return balancedTree(opInfo.name, terms, ops, 0, ops.size() - 1); + return balancedTree(opInfo.name, terms, ops, 0, opsCount - 1); } - private CelExpr balancedTree(String op, List terms, List ops, int lo, int hi) { + private CelExpr balancedTree(String op, CelExpr[] terms, long[] ops, int lo, int hi) { int mid = (lo + hi + 1) / 2; - CelExpr left; - if (mid == lo) { - left = terms.get(mid); - } else { - left = balancedTree(op, terms, ops, lo, mid - 1); - } - CelExpr right; - if (mid == hi) { - right = terms.get(mid + 1); - } else { - right = balancedTree(op, terms, ops, mid + 1, hi); - } + CelExpr left = (mid == lo) ? terms[mid] : balancedTree(op, terms, ops, lo, mid - 1); + CelExpr right = (mid == hi) ? terms[mid + 1] : balancedTree(op, terms, ops, mid + 1, hi); + return buildBinaryCall(ops[mid], op, left, right); + } + + private static CelExpr buildBinaryCall(long id, String function, CelExpr lhs, CelExpr rhs) { return CelExpr.newBuilder() - .setId(ops.get(mid)) - .setCall(CelExpr.CelCall.newBuilder().setFunction(op).addArgs(left).addArgs(right).build()) + .setId(id) + .setCall( + CelExpr.CelCall.newBuilder().setFunction(function).addArgs(lhs).addArgs(rhs).build()) .build(); } private CelExpr parseSelectorChain() { - CelExpr lhs = parseUnary(); - currentLhsDepth = 0; Lexer.TokenType tok = peekToken.type; + CelExpr lhs = + (tok == Lexer.TokenType.EXCLAMATION || tok == Lexer.TokenType.MINUS) + ? parseUnaryOps() + : parsePrimary(); + currentLhsDepth = 0; + tok = peekToken.type; if (tok == Lexer.TokenType.DOT || tok == Lexer.TokenType.LEFT_BRACKET || tok == Lexer.TokenType.LEFT_BRACE) { @@ -538,10 +521,7 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) { long opId = nextId(dotTok); CelExpr arg1 = lhs; CelExpr arg2 = - CelExpr.newBuilder() - .setId(nextId(getLeftmostPosition(lhs))) - .setConstant(CelConstant.ofValue(idText)) - .build(); + CelExpr.ofConstant(nextId(getLeftmostPosition(lhs)), CelConstant.ofValue(idText)); lhs = CelExpr.newBuilder() .setId(opId) @@ -572,12 +552,7 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) { .build(); } } else { - lhs = - CelExpr.newBuilder() - .setId(nextId(dotTok)) - .setSelect( - CelExpr.CelSelect.newBuilder().setOperand(lhs).setField(idText).build()) - .build(); + lhs = CelExpr.ofSelect(nextId(dotTok), lhs, idText, /* isTestOnly= */ false); } } else if (tok == Lexer.TokenType.LEFT_BRACKET) { if (checkRecursion(chainDepth, peekToken)) { @@ -609,7 +584,7 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) { .build()) .build(); } else if (tok == Lexer.TokenType.LEFT_BRACE) { - String structName = extractStructName(lhs).orElse(null); + String structName = extractStructName(lhs); if (structName == null) { break; } @@ -622,14 +597,6 @@ private CelExpr parseSelectorChainTail(CelExpr initialLhs) { return lhs; } - private CelExpr parseUnary() { - Lexer.TokenType tok = peekToken.type; - if (tok == Lexer.TokenType.EXCLAMATION || tok == Lexer.TokenType.MINUS) { - return parseUnaryOps(); - } - return parsePrimary(); - } - private CelExpr parseUnaryOps() { Lexer.Token op = nextToken(); Lexer.TokenType opType = op.type; @@ -783,10 +750,7 @@ private CelExpr parseIdentOrCall() { .build(); } long id = nextId(leadingDot ? firstTok : idTok); - return CelExpr.newBuilder() - .setId(id) - .setIdent(CelExpr.CelIdent.newBuilder().setName(name).build()) - .build(); + return CelExpr.ofIdent(id, name); } private CelExpr parsePrimary() { @@ -807,15 +771,13 @@ private CelExpr parsePrimary() { return expr; } case NULL: - return CelExpr.newBuilder().setId(nextId(nextToken())).setConstant(Constants.NULL).build(); + return CelExpr.ofConstant(nextId(nextToken()), Constants.NULL); case TRUE: case FALSE: { Lexer.Token tok = nextToken(); - return CelExpr.newBuilder() - .setId(nextId(tok)) - .setConstant(tok.type == Lexer.TokenType.TRUE ? Constants.TRUE : Constants.FALSE) - .build(); + return CelExpr.ofConstant( + nextId(tok), tok.type == Lexer.TokenType.TRUE ? Constants.TRUE : Constants.FALSE); } case INT: return parseIntLiteral(/* nodeId= */ -1, /* isNegative= */ false); @@ -990,7 +952,7 @@ private CelExpr parseIntLiteral(long nodeId, boolean isNegative) { long id = nodeId == -1 ? nextId(tok) : nodeId; try { CelConstant constExpr = Constants.parseInt(text); - return CelExpr.newBuilder().setId(id).setConstant(constExpr).build(); + return CelExpr.ofConstant(id, constExpr); } catch (ParseException e) { reportSyntaxError(tok, "invalid int literal: " + text); return CelExpr.newBuilder().setId(nextId(tok)).build(); @@ -1002,7 +964,7 @@ private CelExpr parseUintLiteral() { String value = getTokenText(tok); try { CelConstant constExpr = Constants.parseUint(value); - return CelExpr.newBuilder().setId(nextId(tok)).setConstant(constExpr).build(); + return CelExpr.ofConstant(nextId(tok), constExpr); } catch (ParseException e) { reportSyntaxError(tok, "invalid uint literal: " + value); return CelExpr.newBuilder().setId(nextId(tok)).build(); @@ -1015,7 +977,7 @@ private CelExpr parseDoubleLiteral(long nodeId, boolean isNegative) { long id = nodeId == -1 ? nextId(tok) : nodeId; try { CelConstant constExpr = Constants.parseDouble(text); - return CelExpr.newBuilder().setId(id).setConstant(constExpr).build(); + return CelExpr.ofConstant(id, constExpr); } catch (ParseException e) { reportSyntaxError(tok, "invalid double literal: " + text); return CelExpr.newBuilder().setId(nextId(tok)).build(); @@ -1027,7 +989,7 @@ private CelExpr parseStringLiteral() { String value = getTokenText(tok); try { CelConstant constExpr = Constants.parseString(value); - return CelExpr.newBuilder().setId(nextId(tok)).setConstant(constExpr).build(); + return CelExpr.ofConstant(nextId(tok), constExpr); } catch (ParseException e) { reportError(tok.start, e.getMessage()); return CelExpr.newBuilder().setId(nextId(tok)).build(); @@ -1039,7 +1001,7 @@ private CelExpr parseBytesLiteral() { String value = getTokenText(tok); try { CelConstant constExpr = Constants.parseBytes(value); - return CelExpr.newBuilder().setId(nextId(tok)).setConstant(constExpr).build(); + return CelExpr.ofConstant(nextId(tok), constExpr); } catch (ParseException e) { reportError(tok.start, e.getMessage()); return CelExpr.newBuilder().setId(nextId(tok)).build(); @@ -1084,52 +1046,53 @@ private static boolean isAsciiAlphanumeric(char c) { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9'); } - private Optional extractStructName(CelExpr expr) { + private @Nullable String extractStructName(CelExpr expr) { if (expr.exprKind().getKind() == CelExpr.ExprKind.Kind.IDENT) { String name = expr.ident().name(); eraseId(expr.id()); - return Optional.of(name); + return name; } if (expr.exprKind().getKind() == CelExpr.ExprKind.Kind.SELECT) { if (expr.select().testOnly()) { - return Optional.empty(); + return null; } CelExpr operand = expr.select().operand(); eraseId(expr.id()); - return extractStructName(operand).map(prefix -> prefix + "." + expr.select().field()); + String prefix = extractStructName(operand); + return prefix != null ? prefix + "." + expr.select().field() : null; } - return Optional.empty(); + return null; } private int getLeftmostPosition(CelExpr expr) { - if (expr.exprKind().getKind() == CelExpr.ExprKind.Kind.IDENT) { - return positions.getOrDefault(expr.id(), 0); - } - if (expr.exprKind().getKind() == CelExpr.ExprKind.Kind.SELECT) { - return getLeftmostPosition(expr.select().operand()); + while (expr.exprKind().getKind() == CelExpr.ExprKind.Kind.SELECT) { + expr = expr.select().operand(); } return positions.getOrDefault(expr.id(), 0); } - private Optional lookupMacro(String id, int argCount, boolean receiverStyle) { + private @Nullable CelMacro lookupMacro(String id, int argCount, boolean receiverStyle) { + if (macros.isEmpty()) { + return null; + } String key = CelMacro.formatKey(id, argCount, receiverStyle); CelMacro macro = macros.get(key); if (macro != null) { - return Optional.of(macro); + return macro; } key = CelMacro.formatVarArgKey(id, receiverStyle); - return Optional.ofNullable(macros.get(key)); + return macros.get(key); } private Optional tryExpandMacro( long exprId, String function, @Nullable CelExpr target, ImmutableList args) { - if (function.isEmpty()) { + if (function.isEmpty() || macros.isEmpty()) { return Optional.empty(); } boolean isReceiver = (target != null); int argCount = args.size(); - Optional macro = lookupMacro(function, argCount, isReceiver); - if (!macro.isPresent()) { + CelMacro macro = lookupMacro(function, argCount, isReceiver); + if (macro == null) { return Optional.empty(); } if (nodeLimitExceeded) { @@ -1139,15 +1102,14 @@ private Optional tryExpandMacro( return Optional.empty(); } - Optional errorArg = args.stream().filter(ERROR::equals).findAny(); - if (errorArg.isPresent() || (target != null && target.equals(ERROR))) { + if ((target != null && target.equals(ERROR)) || hasError(args)) { eraseId(exprId); return Optional.of(ERROR); } int macroPosition = positions.getOrDefault(exprId, 0); CelExpr targetExpr = (target != null ? target : CelExpr.newBuilder().build()); - Optional expandedExpr = expandMacro(macroPosition, macro.get(), targetExpr, args); + Optional expandedExpr = expandMacro(macroPosition, macro, targetExpr, args); if (expandedExpr.isPresent()) { if (options.populateMacroCalls()) { @@ -1159,8 +1121,20 @@ private Optional tryExpandMacro( return Optional.empty(); } + private static boolean hasError(List args) { + for (int i = 0; i < args.size(); i++) { + if (args.get(i).equals(ERROR)) { + return true; + } + } + return false; + } + private Optional expandMacro( int position, CelMacro macro, CelExpr target, ImmutableList arguments) { + if (macroExprFactory == null) { + macroExprFactory = new PrattMacroExprFactory(); + } macroExprFactory.pushPosition(position); try { return macro.getExpander().expandMacro(macroExprFactory, target, arguments); @@ -1171,6 +1145,9 @@ private Optional expandMacro( private void recordMacroCall( long macroId, String function, CelExpr target, ImmutableList args) { + if (!(macroCalls instanceof HashMap)) { + macroCalls = new HashMap<>(); + } CelExpr.CelCall.Builder callExpr = CelExpr.CelCall.newBuilder().setFunction(function); if (target != null) { if (macroCalls.containsKey(target.id())) { @@ -1205,6 +1182,24 @@ private int countGroupingParentheses() { return 0; } + // Fast path: if the next non-whitespace character is not '(', leading open parens is 1. + int pos = peekToken.end; + while (pos < source.getContent().size()) { + int c = source.getContent().get(pos); + if (c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\f' && c != 11) { + if (c == '/') { + // A comment might precede another '('. + break; + } + if (c == '(') { + break; + } + // Next significant token is definitely not '('. + return 1; + } + pos++; + } + int savedPos = lexer.savePosition(); try { int leadingOpenParens = 1; diff --git a/parser/src/test/java/dev/cel/parser/CelParserImplTest.java b/parser/src/test/java/dev/cel/parser/CelParserImplTest.java index 37501ec29..5b7f9defb 100644 --- a/parser/src/test/java/dev/cel/parser/CelParserImplTest.java +++ b/parser/src/test/java/dev/cel/parser/CelParserImplTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; @@ -27,6 +28,7 @@ import dev.cel.common.CelValidationException; import dev.cel.common.CelValidationResult; import dev.cel.common.ast.CelExpr; +import java.util.Collections; import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -396,4 +398,14 @@ public void toParserBuilder_collectionProperties_copied() { assertThat(newParserBuilder.getMacros()).hasSize(1); assertThat(newParserBuilder.getParserLibraries().build()).hasSize(1); } + + @Test + public void parse_logicalChainLongerThanInitialCapacity_succeeds() { + CelParser parser = newParserBuilder().build(); + for (int operands = 2; operands <= 64; operands++) { + String expr = Joiner.on(" || ").join(Collections.nCopies(operands, "true")); + CelValidationResult result = parser.parse(expr); + assertThat(result.hasError()).isFalse(); + } + } } diff --git a/parser/src/test/java/dev/cel/parser/CelParserParameterizedTest.java b/parser/src/test/java/dev/cel/parser/CelParserParameterizedTest.java index 7e19e24f8..0f9fb36b7 100644 --- a/parser/src/test/java/dev/cel/parser/CelParserParameterizedTest.java +++ b/parser/src/test/java/dev/cel/parser/CelParserParameterizedTest.java @@ -372,6 +372,7 @@ public void parser_core_syntax() { runTest("a || b || c || d || e || f"); runTest("a < 5 || a > 10"); runTest("a && b && c && d || e && f && g && h"); + runTest("a || b && c || d && e || f && g || h && i || j && k || l"); // Conditional operator runTest("a?b:c"); diff --git a/parser/src/test/java/dev/cel/parser/PrattParserTest.java b/parser/src/test/java/dev/cel/parser/PrattParserTest.java index e9394b362..710ff2fcf 100644 --- a/parser/src/test/java/dev/cel/parser/PrattParserTest.java +++ b/parser/src/test/java/dev/cel/parser/PrattParserTest.java @@ -295,6 +295,7 @@ public void pratt_parser_core_syntax() { runTest("a || b || c || d || e || f"); runTest("a < 5 || a > 10"); runTest("a && b && c && d || e && f && g && h"); + runTest("a || b && c || d && e || f && g || h && i || j && k || l"); // Conditional operator runTest("a?b:c"); diff --git a/parser/src/test/resources/parser_core_syntax.baseline b/parser/src/test/resources/parser_core_syntax.baseline index 7c05685f3..34997c9d8 100644 --- a/parser/src/test/resources/parser_core_syntax.baseline +++ b/parser/src/test/resources/parser_core_syntax.baseline @@ -1061,6 +1061,77 @@ L: _||_( )^#12[1,27]# )^#8[1,17]# +I: a || b && c || d && e || f && g || h && i || j && k || l +=====> +P: _||_( + _||_( + _||_( + a^#1:Expr.Ident#, + _&&_( + b^#3:Expr.Ident#, + c^#5:Expr.Ident# + )^#4:Expr.Call# + )^#2:Expr.Call#, + _||_( + _&&_( + d^#7:Expr.Ident#, + e^#9:Expr.Ident# + )^#8:Expr.Call#, + _&&_( + f^#11:Expr.Ident#, + g^#13:Expr.Ident# + )^#12:Expr.Call# + )^#10:Expr.Call# + )^#6:Expr.Call#, + _||_( + _||_( + _&&_( + h^#15:Expr.Ident#, + i^#17:Expr.Ident# + )^#16:Expr.Call#, + _&&_( + j^#19:Expr.Ident#, + k^#21:Expr.Ident# + )^#20:Expr.Call# + )^#18:Expr.Call#, + l^#23:Expr.Ident# + )^#22:Expr.Call# +)^#14:Expr.Call# +L: _||_( + _||_( + _||_( + a^#1[1,0]#, + _&&_( + b^#3[1,5]#, + c^#5[1,10]# + )^#4[1,7]# + )^#2[1,2]#, + _||_( + _&&_( + d^#7[1,15]#, + e^#9[1,20]# + )^#8[1,17]#, + _&&_( + f^#11[1,25]#, + g^#13[1,30]# + )^#12[1,27]# + )^#10[1,22]# + )^#6[1,12]#, + _||_( + _||_( + _&&_( + h^#15[1,35]#, + i^#17[1,40]# + )^#16[1,37]#, + _&&_( + j^#19[1,45]#, + k^#21[1,50]# + )^#20[1,47]# + )^#18[1,42]#, + l^#23[1,55]# + )^#22[1,52]# +)^#14[1,32]# + I: a?b:c =====> P: _?_:_( diff --git a/parser/src/test/resources/pratt_parser_core_syntax.baseline b/parser/src/test/resources/pratt_parser_core_syntax.baseline index 02f44e87c..fb9d94e58 100644 --- a/parser/src/test/resources/pratt_parser_core_syntax.baseline +++ b/parser/src/test/resources/pratt_parser_core_syntax.baseline @@ -1043,6 +1043,77 @@ L: _||_( )^#12[1,27]# )^#8[1,17]# +I: a || b && c || d && e || f && g || h && i || j && k || l +=====> +P: _||_( + _||_( + _||_( + a^#1:Expr.Ident#, + _&&_( + b^#3:Expr.Ident#, + c^#5:Expr.Ident# + )^#4:Expr.Call# + )^#2:Expr.Call#, + _||_( + _&&_( + d^#7:Expr.Ident#, + e^#9:Expr.Ident# + )^#8:Expr.Call#, + _&&_( + f^#11:Expr.Ident#, + g^#13:Expr.Ident# + )^#12:Expr.Call# + )^#10:Expr.Call# + )^#6:Expr.Call#, + _||_( + _||_( + _&&_( + h^#15:Expr.Ident#, + i^#17:Expr.Ident# + )^#16:Expr.Call#, + _&&_( + j^#19:Expr.Ident#, + k^#21:Expr.Ident# + )^#20:Expr.Call# + )^#18:Expr.Call#, + l^#23:Expr.Ident# + )^#22:Expr.Call# +)^#14:Expr.Call# +L: _||_( + _||_( + _||_( + a^#1[1,0]#, + _&&_( + b^#3[1,5]#, + c^#5[1,10]# + )^#4[1,7]# + )^#2[1,2]#, + _||_( + _&&_( + d^#7[1,15]#, + e^#9[1,20]# + )^#8[1,17]#, + _&&_( + f^#11[1,25]#, + g^#13[1,30]# + )^#12[1,27]# + )^#10[1,22]# + )^#6[1,12]#, + _||_( + _||_( + _&&_( + h^#15[1,35]#, + i^#17[1,40]# + )^#16[1,37]#, + _&&_( + j^#19[1,45]#, + k^#21[1,50]# + )^#20[1,47]# + )^#18[1,42]#, + l^#23[1,55]# + )^#22[1,52]# +)^#14[1,32]# + I: a?b:c =====> P: _?_:_(