Parser rework#9
Open
sean-parent wants to merge 33 commits into
Open
Conversation
Introduces a new op_table module with a scope-based operation lookup system for dynamic operator dispatch in cel-parser. The parser now supports custom operation scopes, efficient built-in operator tables using phf, and exposes op_lookup_mut for extensibility. Includes new tests and documentation, and updates dependencies to add phf and once_cell.
Enhanced literal parsing in LexLexer to use syn::parse2 for better span preservation and added the 'parsing' feature to the syn dependency. Expanded documentation for parser structures and methods in lib.rs, including detailed doc comments for primary expressions, probe results, error formatting, and parsing entry points. Improved test assertions and error reporting in parser tests.
Refactored conditional logic in cel-parser to use && let chains for improved readability and conciseness. Expanded .vscode/tasks.json with comprehensive build, test, documentation, clippy, and sanitizer tasks for multiple platforms to streamline development workflows.
Added and reorganized tasks in .vscode/tasks.json for improved Rust workflow. Expanded sanitizer support for Linux/WSL2 and Windows, clarified platform-specific notes, and added a comprehensive test task for Windows. This update enhances cross-platform development and testing capabilities.
Updated .vscode/tasks.json to streamline task definitions by changing command types to 'cargo' for clippy and adjusting environment variables for better clarity. Removed experimental Windows tasks to focus on macOS and Linux/WSL2 support, enhancing the overall development experience.
Updated the error handling logic in the expression function to use the is_err() method for improved clarity and conciseness. This change enhances the readability of the parser code.
The lexer (`LexLexer`) no longer returns `Result` and is now infallible, as all input is pre-validated by `proc_macro2`. The parser (`CELParser`) is updated to remove error handling for lexer errors, simplifying token peeking and advancement. Documentation is updated to clarify error handling in both lexer and parser. Macro now only reports errors for non-matching expressions, not parse errors.
Move the CEL parser implementation from the separate cel-parser crate into cel-runtime::parser, remove the cel-parser crate and its example docs, and update workspace/Cargo manifests. CELParser API changed: constructor now takes an OpLookup, tokens are set via set_tokens/parse_tokens/parse_str, postfix/call parsing added, and error/reporting helpers adjusted. Updated lex_lexer to use a concrete TokenStreamIter type and relocated files under cel-runtime/src/parser; cel-rs-macros now depends on cel-runtime and uses CELParser/OpLookup accordingly. Also export parser types and implement FromStr for DynSegment to allow parsing from strings; tests updated to the new API.
Add a .cursor rule encouraging no-heap allocations and implement several refactors to reduce temporaries and allocations. Introduce PunctOp (no-heap punctuation token) and replace String-based punct tokens. Extend StackInfo with Cow<'static, str> type_name, padding flag, dropper, and AssociatedType to record nested types; store argument_names in DynSegment. Replace peek_types_vec with peek_stack_infos(&[StackInfo]) returning a slice to avoid allocating Vec<TypeId>. Change ScopeFn/OpLookup APIs to accept &mut DynSegment and a num_operands so scopes can inspect the stack without extra allocations; update builtin lookup and parser call sites and tests accordingly. Improve error messages to include human-readable type names.
Add a dedicated CELError type (with message and proc_macro2::Span) and a rustc-style formatter, and refactor the parser to return CELError instead of anyhow::Error. Removed the parser's compile_error token-output plumbing and helper extraction/formatting methods; replaced report_error with error_at that constructs CELError at the current span. Updated CEL macro to emit compile_error! using CELError spans and literals. Re-export CELError from runtime, adjusted FromStr and parser APIs/Result type, and updated tests to work with the new error type and formatting.
Introduce a Send+Sync SourceSpan and switch CELError to store it (with helper from_proc_macro2) so parse errors can be used across threads. Adjust proc-macro handling to emit compile_error! at call_site and use SourceSpan defaults. Add fallible operation support: DynSegment.op1r/op2r with stack-unwinding droppers and RawSegment::raw2 helper to run fallible binary ops. Update op_table to use op1r/op2r for checked integer arithmetic and shifts (returning errors on overflow/invalid shifts) and add tests for overflow and unwind behavior.
Drop manual unsafe impl Send/Sync for SourceSpan and CELError in parser/error.rs. Reformat cel-runtime/src/parser/op_table.rs: reorder anyhow import, reflow many long closure expressions across multiple lines for readability, compact some signature arrays onto single lines, and tidy minor test formatting. These are mostly non-functional formatting/import changes; only behavioral change is removal of the explicit unsafe Send/Sync impls.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Replace standalone matches! calls with assert!(matches!(...)) in cel-runtime/src/parser/lex_lexer.rs tests. The previous matches! usages were no-ops and did not fail the test on mismatch; wrapping them with assert! ensures the tests actually verify token shapes and will fail when tokens differ from expectations.
Use integer checked_div/checked_rem wrapped with op2r for all integer division and modulo signatures so division-by-zero returns an error instead of panicking; float behavior remains unchanged. Add explanatory comments and unit tests (division_by_zero and modulo_by_zero) that assert the error contains "division by zero". This prevents runtime panics and surfaces a clear error to callers.
Added a workflow_dispatch trigger to the CI configuration for manual workflow execution. Included a new job to check Rust code formatting using `cargo fmt --check`. Additionally, refactored several function signatures in the `cel-runtime` module for improved readability by removing unnecessary line breaks and ensuring consistent formatting across the codebase.
There was a problem hiding this comment.
Pull request overview
This PR consolidates CEL parsing into cel-runtime by removing the standalone cel-parser crate, updating macro validation to use the new runtime parser, and expanding developer tooling/docs to match the new workspace structure.
Changes:
- Remove
cel-parserfrom the workspace and route parsing APIs throughcel-runtime(including new parser modules andDynSegment: FromStr). - Refactor
cel-rs-macrosto validate expressions viacel-runtimeparser/error types. - Add/expand developer workflow assets (VSCode tasks, CI fmt check, and Cursor/CLAUDE guidance).
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib.rs |
Updates crate-level docs/re-exports to reflect parser living in cel-runtime. |
notes.txt |
Adds a cargo clippy --fix --workspace workflow note. |
CLAUDE.md |
Documents workspace architecture, parser pipeline, and workflow commands. |
cel-runtime/src/segment.rs |
Aligns type-id tracking with updated StackInfo shape. |
cel-runtime/src/raw_segment.rs |
Adds fallible stack-aware binary op support (raw2). |
cel-runtime/src/parser/op_table.rs |
Introduces operator lookup/dispatch table with built-in ops and custom scopes. |
cel-runtime/src/parser/mod.rs |
Adds the recursive-descent parser producing executable segments. |
cel-runtime/src/parser/lex_lexer.rs |
Adds lexer layer that flattens/normalizes token streams for the parser. |
cel-runtime/src/parser/error.rs |
Adds CELError + rustc-style diagnostic formatting. |
cel-runtime/src/lib.rs |
Exposes parser APIs and implements DynSegment: FromStr. |
cel-runtime/src/dyn_segment.rs |
Enhances stack metadata, adds fallible ops (op1r/op2r), improves error reporting. |
cel-runtime/Cargo.toml |
Adds dependencies needed for parser/diagnostics and removes old macro dependency coupling. |
cel-rs-macros/src/lib.rs |
Switches expression! validation to the runtime parser and updates docs. |
cel-rs-macros/Cargo.toml |
Depends on cel-runtime instead of removed cel-parser. |
cel-parser/src/lib.rs |
Removed (parser implementation moved into cel-runtime). |
cel-parser/Cargo.toml |
Removed (crate removed from workspace). |
Cargo.toml |
Removes cel-parser from workspace members/dependencies. |
Cargo.lock |
Updates lockfile for removed/added crates. |
.vscode/tasks.json |
Adds comprehensive build/test/lint/sanitizer tasks (needs update for removed crate). |
.vscode/settings.json |
Updates spellchecker dictionary. |
.github/workflows/ci.yml |
Adds workflow_dispatch and cargo fmt --check. |
.cursor/rules/doc-comments.mdc |
Expands/enforces Rust doc comment conventions. |
.cursor/rules/avoid-heap-allocations.mdc |
Adds allocation-avoidance guidelines. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Updated the push_literal function to return a Result type, allowing for better error handling when parsing integer and float literals with invalid suffixes. Added unit tests to verify correct parsing of valid suffixes and proper error reporting for invalid ones. This change improves the robustness of the parser by ensuring that suffix-related errors are caught and reported accurately.
Comment on lines
47
to
65
| pub fn expression(input: ProcMacroTokenStream) -> ProcMacroTokenStream { | ||
| let input = TokenStream::from(input); | ||
| let mut parser = CELParser::new(input.into_iter()); | ||
| if !parser.is_expression() { | ||
| parser.report_error("Expected expression"); | ||
| let mut parser = CELParser::new(OpLookup::new()); | ||
| parser.set_tokens(input.into_iter()); | ||
| match parser.is_expression() { | ||
| Ok(true) => ProcMacroTokenStream::new(), | ||
| Ok(false) => { | ||
| let e = CELError::new( | ||
| "Expected expression", | ||
| cel_runtime::parser::SourceSpan::default(), | ||
| ); | ||
| let msg_lit = Literal::string(&e.to_string()); | ||
| quote_spanned!(proc_macro2::Span::call_site() => compile_error!(#msg_lit)).into() | ||
| } | ||
| Err(e) => { | ||
| let msg_lit = Literal::string(&e.to_string()); | ||
| quote_spanned!(proc_macro2::Span::call_site() => compile_error!(#msg_lit)).into() | ||
| } | ||
| } |
Comment on lines
+728
to
+760
| /// `postfix_expression = primary_expression { "(" parameter_list ")" }.` | ||
| fn is_postfix_expression(&mut self) -> Result<bool> { | ||
| if !self.is_primary_expression()? { | ||
| return Ok(false); | ||
| } | ||
| while matches!( | ||
| self.peek_token(), | ||
| Some(Token::OpenDelim { | ||
| delimiter: Delimiter::Parenthesis, | ||
| .. | ||
| }) | ||
| ) { | ||
| self.advance(); // consume "(" | ||
| let arg_count = self.parameter_list()?; | ||
| match self.peek_token() { | ||
| Some(Token::CloseDelim { | ||
| delimiter: Delimiter::Parenthesis, | ||
| .. | ||
| }) => { | ||
| self.advance(); // consume ")" | ||
| } | ||
| _ => return Err(self.error_at("expected closing parenthesis")), | ||
| } | ||
| // Push the call operation: pops argument(s) then callee, invokes callee, pushes result. | ||
| // Stack order is [callee, arg1, arg2, ...]; lookup peeks top (arg_count + 1) entries. | ||
| if self.context.stack_ids.len() > arg_count | ||
| && let Err(e) = self | ||
| .op_lookup | ||
| .lookup("()", &mut self.context, arg_count + 1) | ||
| { | ||
| return Err(self.error_at(&format!("call: {}", e))); | ||
| } | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Added overloads for shift operators. - Updated several dependencies in Cargo.lock to their latest versions, including `anyhow`, `is-terminal`, `libc`, `once_cell`, `owo-colors`, `proc-macro2`, `quote`, `rand`, `siphasher`, `syn`, `typenum`, and `unicode-ident`. - Added new features and lints in Cargo.toml for both the main project and `cel-runtime`, including a `playground` feature and workspace linting. - Updated VSCode settings to enable the `playground` feature for Rust Analyzer. - Removed unnecessary documentation warnings in `cel-rs-macros` and `cel-runtime` source files. - Cleaned up commented-out code and improved code readability in various modules. These changes aim to keep the project up-to-date with the latest library improvements and enhance the development experience.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 298905b. Configure here.
| Err(e) => { | ||
| let msg_lit = Literal::string(&e.to_string()); | ||
| quote_spanned!(proc_macro2::Span::call_site() => compile_error!(#msg_lit)).into() | ||
| } |
There was a problem hiding this comment.
Proc macro loses precise error span location
Medium Severity
Both the Ok(false) and Err(e) branches use Span::call_site() for the compile_error! span. The old parser used the offending token's actual proc_macro2::Span via quote_spanned!, so IDE diagnostics pointed to the specific token causing the error. Now all errors point to the macro invocation site, a regression in diagnostic precision. The CELError carries a SourceSpan with the correct line/column but it's not used to reconstruct a span for compile_error!.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 298905b. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request makes several improvements to Rust development workflows and code quality standards for the project. The most significant changes include the removal of the
cel-parsercrate, enhancements to documentation and allocation guidelines, expanded VSCode build/test tasks, and updates to dependencies and macro usage to reflect the new crate structure.Project structure and dependencies:
cel-parsercrate from the workspace and dependencies, consolidating parsing functionality intocel-runtime. All references and dependencies oncel-parserhave been eliminated fromCargo.tomlfiles and the workspace configuration. ([[1]](https://github.com/stlab/cel-rs/pull/9/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542L9-R15),[[2]](https://github.com/stlab/cel-rs/pull/9/files#diff-b9957dc1b6aedbcdeff4fbce5b4221da59ef2dd8884b80c051f4fe6ee3108f8dL1-L12),[[3]](https://github.com/stlab/cel-rs/pull/9/files#diff-3823bfb8141007195f06dc4ffabcc9e2db3fd93d37624332d46e88bcfd20c48bL14-R14))cel-rs-macroscrate to depend oncel-runtimeinstead ofcel-parser, and refactored macro code to use parsing and error types fromcel-runtime. ([[1]](https://github.com/stlab/cel-rs/pull/9/files#diff-3823bfb8141007195f06dc4ffabcc9e2db3fd93d37624332d46e88bcfd20c48bL14-R14),[[2]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L33-R36),[[3]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L48-L52))Development tooling:
[.vscode/tasks.jsonR4-R352](https://github.com/stlab/cel-rs/pull/9/files#diff-7d76d7533653c23b753fc7ce638cf64bdb5e419927d276af836d3a03fdf1745aR4-R352))[.vscode/settings.jsonL10-R11](https://github.com/stlab/cel-rs/pull/9/files#diff-a5de3e5871ffcc383a2294845bd3df25d3eeff6c29ad46e3a396577c413bf357L10-R11))Code quality and documentation standards:
[.cursor/rules/avoid-heap-allocations.mdcR1-R87](https://github.com/stlab/cel-rs/pull/9/files#diff-d13685b400c5d4957f64a3d3644543afc23c9f80bff059aa3542cfe5adc431aeR1-R87))[.cursor/rules/doc-comments.mdcL2-R154](https://github.com/stlab/cel-rs/pull/9/files#diff-bc71edfe8cc02c0facfc56fc6371c9bf2d90055cb84af398575c9f3dc2c204ddL2-R154))Dependency updates:
cel-runtime(e.g.,proc-macro2,quote,owo-colors,syn,phf,once_cell) to support the consolidated parsing and macro infrastructure. ([cel-runtime/Cargo.tomlL10-R15](https://github.com/stlab/cel-rs/pull/9/files#diff-b401c530f9e812ac243c304a31f8c72e20f1a285e1fdbd95fdb804581cdd1bf2L10-R15))Macro and parsing improvements:
expressionmacro incel-rs-macrosto use the new parser interface fromcel-runtime, improving error reporting and aligning with the new crate structure. Also updated theprint_tokens!macro documentation example. ([[1]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L33-R36),[[2]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L48-L52),[[3]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L61-R74))Detailed list of changes:
Project structure and dependencies
cel-parsercrate from the workspace and all related dependencies, consolidating parsing functionality intocel-runtime. ([[1]](https://github.com/stlab/cel-rs/pull/9/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542L9-R15),[[2]](https://github.com/stlab/cel-rs/pull/9/files#diff-b9957dc1b6aedbcdeff4fbce5b4221da59ef2dd8884b80c051f4fe6ee3108f8dL1-L12))cel-rs-macrosto depend oncel-runtimeinstead ofcel-parser, and refactored macro code accordingly. ([[1]](https://github.com/stlab/cel-rs/pull/9/files#diff-3823bfb8141007195f06dc4ffabcc9e2db3fd93d37624332d46e88bcfd20c48bL14-R14),[[2]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L33-R36),[[3]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L48-L52))Development tooling
[.vscode/tasks.jsonR4-R352](https://github.com/stlab/cel-rs/pull/9/files#diff-7d76d7533653c23b753fc7ce638cf64bdb5e419927d276af836d3a03fdf1745aR4-R352))[.vscode/settings.jsonL10-R11](https://github.com/stlab/cel-rs/pull/9/files#diff-a5de3e5871ffcc383a2294845bd3df25d3eeff6c29ad46e3a396577c413bf357L10-R11))Code quality and documentation
[.cursor/rules/avoid-heap-allocations.mdcR1-R87](https://github.com/stlab/cel-rs/pull/9/files#diff-d13685b400c5d4957f64a3d3644543afc23c9f80bff059aa3542cfe5adc431aeR1-R87))[.cursor/rules/doc-comments.mdcL2-R154](https://github.com/stlab/cel-rs/pull/9/files#diff-bc71edfe8cc02c0facfc56fc6371c9bf2d90055cb84af398575c9f3dc2c204ddL2-R154))Dependencies
cel-runtimeto support parsing, macro expansion, and colorized output. ([cel-runtime/Cargo.tomlL10-R15](https://github.com/stlab/cel-rs/pull/9/files#diff-b401c530f9e812ac243c304a31f8c72e20f1a285e1fdbd95fdb804581cdd1bf2L10-R15))Macros and parsing
expressionmacro to use the newCELParserinterface fromcel-runtime, improving error handling and reporting. Updated documentation for theprint_tokens!macro. ([[1]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L33-R36),[[2]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L48-L52),[[3]](https://github.com/stlab/cel-rs/pull/9/files#diff-c187b9b22359c2452046a640b14f4e3748f18cc98c62ce1ed00bfc82ca709c20L61-R74))Note
Medium Risk
Large parser/runtime refactor (new op table, fallible arithmetic, macro API) affects expression evaluation and compile-time validation; behavior changes vs old token-only parser are possible.
Overview
Removes the standalone
cel-parsercrate and folds CEL parsing, lexing, operation dispatch, and rustc-style diagnostics intocel-runtime(parser/withCELParser,LexLexer,CELError, andOpLookup).The parser now builds executable
DynSegments (not just token validation): literals with typed suffixes, postfix calls viaOpLookup, and built-in ops fromphftables with checked signed math, division/modulo-by-zero, and shift semantics.DynSegmentgains stack type names for errors,peek_stack_infos, andop1r/op2rwith unwind-on-failure;DynSegment: FromStrparses strings end-to-end.cel-rs-macrosvalidates expressions throughCELParser+OpLookupand emitscompile_error!fromCELErrortext. Workspace addsplaygroundfeature,missing_docswarn, and CIcargo fmt --check. Remaining diff is mostly dev tooling (VSCode tasks/sanitizers, Cursor/Claude rules,CLAUDE.md).Reviewed by Cursor Bugbot for commit 298905b. Bugbot is set up for automated code reviews on this repo. Configure here.