Skip to content

Replace ANTLR parser with CongoCC - #958

Open
snazy wants to merge 1 commit into
projectnessie:mainfrom
snazy:congocc
Open

Replace ANTLR parser with CongoCC#958
snazy wants to merge 1 commit into
projectnessie:mainfrom
snazy:congocc

Conversation

@snazy

@snazy snazy commented Aug 18, 2026

Copy link
Copy Markdown
Member

Move CEL grammar generation into cel-core using CongoCC and remove the generated-antlr module, shadow jar dependency, and ANTLR runtime wiring.

The generated parser now builds internal AST nodes with typed CEL expression conversion hooks, keeping parser construction close to the grammar while preserving the existing parser API and source-location handling.

This simplifies dependency management for downstream consumers, removes the relocated ANTLR artifact from the build graph, and makes future grammar changes easier to review in the core module.

Move CEL grammar generation into cel-core using CongoCC and remove the generated-antlr module, shadow jar dependency, and ANTLR runtime wiring.

The generated parser now builds internal AST nodes with typed CEL expression conversion hooks, keeping parser construction close to the grammar while preserving the existing parser API and source-location handling.

This simplifies dependency management for downstream consumers, removes the relocated ANTLR artifact from the build graph, and makes future grammar changes easier to review in the core module.
@snazy
snazy requested a review from XN137 August 18, 2026 08:38
} else if (ctx instanceof Token) {
Token token = (Token) ctx;
location = source.newLocation(token.getLine(), token.getCharPositionInLine());
if (ctx instanceof Node) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: arent we on java 17 now?

if (e.isEmpty()) {
assertThat(actualErr).isEmpty();
} else {
assertThat(actualErr).isNotEmpty();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isnt this a serious loss of coverage i.e. the expected errors now dont get asserted?

.isInstanceOf(ScriptCreateException.class)
.hasMessageStartingWith(
"parse failed: ERROR: <input>:1:3: Syntax error: mismatched input ',' expecting IDENTIFIER");
"parse failed: ERROR: <input>:1:3: Syntax error: Encountered an error");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there no way to keep providing more context to the user?


parser.addParseListener(parserListener);

parser.setErrorHandler(new RecoveryLimitErrorStrategy(options.getErrorRecoveryLimit()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.getErrorRecoveryLimit() still exists but is no longer respected ?
are we missing test coverage here ?

return reportError(node, "invalid literal");
}

private boolean isNegativeNumericLiteral(Node operand) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pardon my ignorance, but where is the "negative" property checked in the method body?

return text;
}

private Node firstExpressionChild(List<Node> children, Node ctx) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cant many of these helper methods be static?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants