Conversation
Reviewer's GuideThis PR refactors various inline lexer helper functions and the token dispatch macro into a dedicated File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughRefactor the parser helpers by moving token parsing utilities, macros, and balanced block parsers from Changes
Sequence Diagram(s)sequenceDiagram
participant Parser
participant LexerHelpers
Parser->>LexerHelpers: Call atom(), ident(), inline_ws(), balanced_block()
LexerHelpers-->>Parser: Return parsed tokens or errors
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.rsInstructions used from: Sources:
⚙️ CodeRabbit Configuration File 🧬 Code Graph Analysis (2)src/parser/ast/parse_utils.rs (1)
src/parser/mod.rs (1)
🔇 Additional comments (8)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/parser/lexer_helpers.rs:72` </location>
<code_context>
+/// SyntaxKind::K_EXTERN => handle_kw,
+/// });
+/// ```
+macro_rules! token_dispatch {
+ ( $ctx:ident, {
+ $( $kind:path => $handler:ident ),* $(,)?
</code_context>
<issue_to_address>
Consider replacing the macro-based token dispatch and manual stateful block parsing with simpler, more idiomatic methods and recursive combinators.
```markdown
Two spots stand out as adding disproportionate complexity:
1) the `token_dispatch!` macro
2) the `balanced_block_with_min` + `Cell` state machine
Both can be simplified without losing any functionality.
---
### 1) Remove `token_dispatch!`, add a `dispatch` method on `TokenStream`
Instead of a macro, put the loop into a small method:
```rust
// in parser/token_stream.rs
impl<'a> TokenStream<'a> {
/// Drive a user‐supplied closure over every peeked token;
/// the closure must advance the stream itself.
pub fn dispatch<F>(&mut self, mut f: F)
where
F: FnMut(&mut Self, SyntaxKind, Span),
{
while let Some(&(kind, ref span_ref)) = self.peek() {
let span = span_ref.clone();
f(self, kind, span);
}
}
}
```
Call‐site change:
```rust
// before:
// token_dispatch!(st, {
// SyntaxKind::T_LPAREN => lp,
// SyntaxKind::T_RPAREN => rp,
// });
// after:
st.stream.dispatch(|stream, kind, span| {
match kind {
SyntaxKind::T_LPAREN => lp(&mut st, span),
SyntaxKind::T_RPAREN => rp(&mut st, span),
_ => stream.advance(),
}
});
```
---
### 2) Replace `balanced_block_with_min` + `Cell` with a recursive combinator
Drop the manual `Cell`-based depth counter and use `chumsky::recursive`:
```rust
use chumsky::recursive;
// flat, no Cell needed:
pub(super) fn balanced_block(
open: SyntaxKind,
close: SyntaxKind,
) -> impl Parser<SyntaxKind, (), Error = Simple<SyntaxKind>> + Clone {
recursive(|block| {
just(open)
.padded_by(inline_ws().repeated())
.ignore_then(
// either a nested block or any other token
block
.or(filter(|k: &SyntaxKind| *k != open && *k != close).ignored())
.padded_by(inline_ws().repeated())
.repeated(),
)
.then_ignore(just(close))
.ignored()
})
}
// If you need “nonempty”, just wrap the inner `.repeated()` in `.at_least(1)`:
pub(super) fn balanced_block_nonempty(
open: SyntaxKind,
close: SyntaxKind,
) -> impl Parser<SyntaxKind, (), Error = Simple<SyntaxKind>> + Clone {
recursive(|block| {
just(open)
.padded_by(inline_ws().repeated())
.ignore_then(
block
.or(filter(|k: &SyntaxKind| *k != open && *k != close).ignored())
.padded_by(inline_ws().repeated())
.repeated()
.at_least(1),
)
.then_ignore(just(close))
.ignored()
})
}
```
This eliminates the `filter_map` with internal mutable `Cell` and yields a much flatter, easier‐to‐read parser.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
Testing
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_68780af7ac8c8322b76eddf80d7bb05b
Summary by Sourcery
Extract and centralize lexer helper utilities into a new parser/lexer_helpers module, update the parser module to use these shared helpers, and add focused unit tests for all helper functions.
Enhancements:
Tests: