feat(runtime)!: expose resolved byte spans to error listeners - #257
Merged
Conversation
Replace the positional ErrorListener callback with SyntaxErrorEvent so consumers can use resolved parser and lexer spans without retaining source text or scraping diagnostic messages. Record UTF-8 byte ranges on TokenSourceError and preserve them through generated lexer dispatch while keeping the Send + Sync listener guarantee. Closes #251
📝 WalkthroughWalkthroughThe runtime introduces ChangesStructured error spans
Lexer span resolution
Recognizer and parser dispatch
Integration and generated-code migration
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Lexer
participant Parser
participant Recognizer
participant ErrorListener
Lexer->>Lexer: resolve scalar range to byte span
Lexer->>Recognizer: notify_error_listeners(source_error.into())
Parser->>Recognizer: notify_error_listeners(SyntaxErrorEvent)
Recognizer->>ErrorListener: forward structured event
ErrorListener->>ErrorListener: consume line, column, message, and span
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review in progress
|
Contributor
Author
|
@codex review |
📊 Source Code Metrics (this PR vs
|
| File | Cyclomatic | Cognitive | Functions | LLOC | MI |
|---|---|---|---|---|---|
| src/parser.rs | 2170 (main: 2169) 🔴 | 1422 (main: 1423) 🟢 | 702 (main: 699) 🔴 | 4715 (main: 4704) 🔴 | 0 ⚪ |
| src/lexer.rs | 336 (main: 326) 🔴 | 76 (main: 70) 🔴 | 206 (main: 201) 🔴 | 570 (main: 547) 🔴 | 0 ⚪ |
| src/atn/lexer.rs | 467 (main: 464) 🔴 | 347 ⚪ | 120 (main: 118) 🔴 | 1030 (main: 1017) 🔴 | 0 ⚪ |
| src/token.rs | 182 (main: 178) 🔴 | 49 (main: 44) 🔴 | 94 (main: 91) 🔴 | 202 (main: 189) 🔴 | 0 ⚪ |
| src/bin_support/grammar/frontend.rs | 227 ⚪ | 91 ⚪ | 68 ⚪ | 311 (main: 309) 🔴 | 0 ⚪ |
| src/recognizer.rs | 53 ⚪ | 2 ⚪ | 39 ⚪ | 77 ⚪ | 6.87 (main: 6.17) 🟢 |
| src/errors.rs | 6 (main: 4) 🔴 | 0 ⚪ | 2 (main: 1) 🔴 | 2 (main: 1) 🔴 | 33.06 (main: 36.55) 🔴 |
| src/lib.rs | 4 ⚪ | 3 ⚪ | 1 ⚪ | 7 ⚪ | 32.47 (main: 32.49) 🔴 |
Generated by mehen v1.7.0 — the code quality watcher.
tinovyatkin
changed the title
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: decd497e01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/parser.rs`:
- Around line 5271-5283: Extract the shared SyntaxErrorEvent construction used
by report_unrecovered_parser_error and dispatch_parser_diagnostic into a private
helper on the same parser type. Have the helper resolve the optional offending
token through token_store(), derive span from the resolved token, and accept the
line, column, message, and error values; update both callers to delegate to it
while preserving their existing event behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9e8170fd-5ffb-4858-880c-f8ae9d8857a1
⛔ Files ignored due to path filters (8)
src/bin_support/grammar/generated/antlr_v4_lexer.rsis excluded by!**/generated/**src/snapshots/antlr4_runtime__parser__tests__failed_interpreted_parse_notifies_error_listener.snapis excluded by!**/*.snapsrc/snapshots/antlr4_runtime__parser__tests__parser_dispatches_recovery_diagnostics_through_registered_listeners.snapis excluded by!**/*.snapsrc/snapshots/antlr4_runtime__parser__tests__recovery_diagnostics_expose_the_offending_token_to_listeners.snapis excluded by!**/*.snapsrc/snapshots/antlr4_runtime__recognizer__tests__recognizers_replace_the_default_console_error_listener.snapis excluded by!**/*.snapsrc/xpath/generated/x_path_lexer.rsis excluded by!**/generated/**tests/snapshots/antlr4_rust_gen_cli__fatal_entry_preserves_prior_recovery_diagnostics.snapis excluded by!**/*.snaptests/snapshots/antlr4_rust_gen_cli__semantic_override_does_not_leak_prior_recovery_diagnostics.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
README.mdsrc/atn/lexer.rssrc/bin/antlr4-rust-gen.rssrc/bin_support/grammar/frontend.rssrc/errors.rssrc/lexer.rssrc/lib.rssrc/parser.rssrc/recognizer.rssrc/token.rstests/antlr4_rust_gen_cli.rs
Reserve the compact token boundary sentinel for missing byte positions so parser diagnostics do not expose fabricated spans from custom token sources or synthetic recovery tokens. Make Token::byte_span optional, retain exact ByteStream ranges, and centralize parser event construction.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab93f785ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Custom CharStream implementations may expose scalar positions without a UTF-8 byte mapping. Store the reserved unknown boundary on synthetic EOF tokens instead of treating the scalar index as a byte offset. Cover a non-ASCII scalar-only stream so parser diagnostics at EOF continue to report no span.
Separate scalar and byte TokenSpec builders so custom token sources cannot fabricate UTF-8 ranges. Keep compact sentinels internal while exposing optional byte boundaries and spans from Token and TokenStore. Document the listener and token migration, make diagnostic event structs non-exhaustive, and expose explicit scalar-span lexer diagnostics. BREAKING CHANGE: ErrorListener::syntax_error now receives SyntaxErrorEvent; Token byte accessors return Option values; TokenSourceError includes span and diagnostic structs are non-exhaustive.
Contributor
Author
|
Addressed the review in
Validation on the new head:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/antlr4_rust_gen_cli.rs`:
- Around line 1285-1292: Add #[allow(clippy::disallowed_methods)] to the
combined_literal_tokens_are_public_and_lexable test or its enclosing test scope
in tests/antlr4_rust_gen_cli.rs (anchor site 1285-1292), and to the relevant mod
tests or affected test function in src/tree.rs (sibling site 1558). Ensure both
scopes using insta assertion macros carry the allowance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2b349a65-47b8-47c7-9169-7065173aa39a
⛔ Files ignored due to path filters (1)
docs/migration.mdis excluded by!**/docs/**
📒 Files selected for processing (10)
src/atn/lexer.rssrc/atn/lexer_dfa.rssrc/bin_support/grammar/atn/interp_test.rssrc/bin_support/grammar/frontend.rssrc/errors.rssrc/lexer.rssrc/parser.rssrc/token.rssrc/tree.rstests/antlr4_rust_gen_cli.rs
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
1 participant
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.
Summary
ErrorListener::syntax_errorcallback with a non-exhaustiveSyntaxErrorEventTokenandTokenStore, keeping compact sentinels internalTokenSpec::with_spandata from explicitly resolvedwith_byte_spandataWhy
Rich-diagnostic consumers currently have to retain a second copy of the source,
convert line/column pairs back to byte offsets, and scrape formatted error
messages to recover lexer-error widths. The runtime already knows these
locations.
TokenSourceErrornow carries the span captured byBaseLexer, while parserdiagnostics derive it from the offending
TokenView. Streams and token sourcesthat cannot provide exact byte offsets return
Nonerather than a fabricatedrange.
ByteStreamretains exact byte ranges.This intentionally changes the listener and token byte-offset APIs in the
pre-1.0 crate instead of retaining parallel legacy paths. The migration guide
documents the callback and custom-token-source changes, and the durable commit
message carries a
BREAKING CHANGEfooter.The listener's
Send + 'staticbound and theRecognizerData: Send + Syncguarantee remain unchanged.
Validation
cargo fmt --all -- --checkcargo clippy --locked --all-targets --all-features -- -D warningscargo test --locked --all-features --workspace(345 + 3 + 835 + 48 passed)RUSTDOCFLAGS='-D warnings -A rustdoc::private-intra-doc-links' cargo doc --locked --all-features --no-deps --libCloses #251
Summary by CodeRabbit
New Features
Bug Fixes
Documentation