Conversation
Reviewer's GuideRefactor parser by extracting all span scanning logic into a dedicated Class diagram for parser module refactor with span_scanner extractionclassDiagram
class ParserMod {
+parse(src: &str) Parsed
+parse_tokens -- moved --> SpanScanner::parse_tokens
}
class SpanScanner {
+parse_tokens(tokens: &[(SyntaxKind, Span)], src: &str) -> (ParsedSpans, Vec<Simple<SyntaxKind>>)
+collect_import_spans(tokens, src)
+collect_typedef_spans(tokens, src)
+collect_relation_spans(tokens, src)
+collect_index_spans(tokens, src)
+collect_function_spans(tokens, src)
+collect_transformer_spans(tokens, src)
+collect_rule_spans(tokens, src)
}
ParserMod ..> SpanScanner : uses
class ParsedSpans
class SyntaxKind
class Span
class Simple
ParserMod --> ParsedSpans
SpanScanner --> ParsedSpans
SpanScanner --> SyntaxKind
SpanScanner --> Span
SpanScanner --> Simple
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughReplace all internal span collection and token scanning logic in Changes
Sequence Diagram(s)sequenceDiagram
participant Parser as parser/mod.rs
participant Scanner as span_scanner.rs
Parser->>Scanner: parse_tokens(tokens, src)
Scanner->>Scanner: Collect import, typedef, relation, index, function, transformer, rule spans
Scanner-->>Parser: (ParsedSpans, Errors)
Parser->>Parser: Continue CST construction with returned spans
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.rsInstructions used from: Sources: 🧬 Code Graph Analysis (1)src/parser/mod.rs (1)
🔇 Additional comments (11)
✨ 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 (
|
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/span_scanner.rs:52` </location>
<code_context>
-/// let (import_spans, errors) = collect_import_spans(&tokens, src);
-/// assert!(!import_spans.is_empty());
-/// ```
-fn collect_import_spans(
- tokens: &[(SyntaxKind, Span)],
- src: &str,
</code_context>
<issue_to_address>
Consider extracting the common span-collection logic in the `collect_*_spans` functions into a generic helper to reduce duplication.
```markdown
Most of the `collect_*_spans` functions share the same “instantiate a `SpanCollector`, hook up a single‐token dispatch, run `parse_span`, then either push or skip on error” pattern. You can factor that out into a small generic helper and then each `collect_*_spans` becomes just “call `collect_with_parser` with my keyword and parser builder.”
Example:
```rust
fn collect_with_parser<F, P>(
tokens: &[(SyntaxKind, Span)],
src: &str,
kind: SyntaxKind,
mk_parser: F,
) -> (Vec<Span>, Vec<Simple<SyntaxKind>>)
where
F: Fn() -> P,
P: Parser<SyntaxKind, Span, Error = Simple<SyntaxKind>>,
{
type State<'a> = SpanCollector<'a, Vec<Simple<SyntaxKind>>>;
let mut st = State::new(tokens, src, Vec::new());
let handler = move |st: &mut State<'_>, span: Span| {
let (res, errs) = st.parse_span(mk_parser(), span.start);
if let Some(sp) = res {
st.spans.push(sp.clone());
st.stream.skip_until(sp.end);
} else {
st.extra.extend(errs);
st.skip_line();
}
};
token_dispatch!(st, { kind => handler });
st.into_parts()
}
```
Then rewrite `collect_import_spans` as:
```rust
fn collect_import_spans(
tokens: &[(SyntaxKind, Span)],
src: &str,
) -> (Vec<Span>, Vec<Simple<SyntaxKind>>) {
collect_with_parser(tokens, src, SyntaxKind::K_IMPORT, || {
let ws = inline_ws().repeated();
let ident = ident();
let module_path = ident.clone()
.then(just(SyntaxKind::T_COLON_COLON)
.padded_by(ws.clone())
.ignore_then(ident.clone())
.repeated())
.ignored();
let alias = just(SyntaxKind::K_AS).padded_by(ws.clone()).ignore_then(ident);
just(SyntaxKind::K_IMPORT)
.padded_by(ws.clone())
.ignore_then(module_path)
.then(alias.or_not())
.padded_by(ws)
.map_with_span(|_, sp| sp)
})
}
```
You can apply the same pattern to `collect_index_spans`, `collect_transformer_spans`, `collect_function_spans`, etc., collapsing ~120 lines of duplicated span‐collection boilerplate while keeping each grammar fragment intact.
</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 review |
✅ Actions performedReview triggered.
|
Summary
span_scannermoduleTesting
make fmtmake lintmake testmake markdownlinthttps://chatgpt.com/codex/tasks/task_e_687820ef46488322beef1654161c7a8f
Summary by Sourcery
Extract span scanning logic from the main parser module into its own
span_scannermodule and update the parser entry point to use the new module.Enhancements:
span_scannermodule undersrc/parserparser/mod.rsto reference theparse_tokensfunction from the new module