Implement delimiter mismatch error#55
Conversation
Reviewer's GuideThis PR enhances the CST parser to detect and report unmatched delimiters by introducing a new DelimiterError type, updating the parsing utility to accumulate errors alongside parsed name-type pairs, adjusting downstream callers to ignore the new error vector, and adding tests for mismatch handling. Sequence diagram for delimiter mismatch detection during parsingsequenceDiagram
participant Parser
participant TokenStream
participant DelimStack
participant ErrorVec as DelimiterError[]
Parser->>TokenStream: next_token()
loop for each token
Parser->>DelimStack: open/close based on token
alt Delimiter mismatch
DelimStack-->>Parser: close returns < expected
Parser->>ErrorVec: push DelimiterError
end
end
Parser-->>Parser: return (pairs, errors)
Class diagram for DelimiterError and DelimStack changesclassDiagram
class Delim {
<<enum>>
Paren
Angle
Bracket
Brace
}
class DelimStack {
+open(delim: Delim, count: usize)
+close(delim: Delim, count: usize) usize
+is_empty() bool
}
class DelimiterError {
+expected: Delim
+found: SyntaxKind
+span: TextRange
+fmt(&self, f: &mut Formatter) -> Result
}
DelimiterError -- Delim : expected
DelimiterError -- SyntaxKind : found
DelimiterError -- TextRange : span
DelimStack o-- Delim : uses
Class diagram for updated parse_name_type_pairs return typeclassDiagram
class parse_name_type_pairs {
<<function>>
+parse_name_type_pairs<I>(iter: I) : (Vec<(String, String)>, Vec<DelimiterError>)
}
parse_name_type_pairs ..> DelimiterError : returns
Class diagram for downstream callers updated to ignore errorsclassDiagram
class Relation {
+columns() : Vec<(String, String)>
}
class Function {
+parameters() : Vec<(String, String)>
}
Relation ..> parse_name_type_pairs : calls
Function ..> parse_name_type_pairs : calls
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis change introduces structured error reporting for unmatched or incorrectly closed delimiters in the parsing of name-type pairs. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant parse_name_type_pairs
participant DelimiterError
Caller->>parse_name_type_pairs: Call with token iterator
parse_name_type_pairs->>parse_name_type_pairs: Parse name-type pairs
alt Delimiter mismatch
parse_name_type_pairs->>DelimiterError: Create DelimiterError
parse_name_type_pairs->>parse_name_type_pairs: Collect errors
end
parse_name_type_pairs-->>Caller: Return (pairs, errors)
Possibly related issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)`**/*.rs`: Comment why, not what. Explain assumptions, edge cases, trade-offs, o...
📄 Source: CodeRabbit Inference Engine (AGENTS.md) List of files the instruction was applied to:
`**/*.rs`: * Seek to keep the cyclomatic complexity of functions no more than 12...
⚙️ Source: CodeRabbit Configuration File List of files the instruction was applied to:
🧬 Code Graph Analysis (1)src/parser/mod.rs (1)
⏰ Context from checks skipped due to timeout of 240000ms (1)
🔇 Additional comments (2)
✨ Finishing Touches
🪧 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 - here's some feedback:
- Extract the repeated
errors.push(...)logic into a small helper function to reduce duplication and improve readability. - Expand tests to cover unmatched
),], and}cases (not just>>) so all delimiter mismatches are validated. - Fix the
Displayimplementation to correctly interpolate theexpected/foundvalues—Rust’swrite!macros don’t support{expected}-style interpolation directly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the repeated `errors.push(...)` logic into a small helper function to reduce duplication and improve readability.
- Expand tests to cover unmatched `)`, `]`, and `}` cases (not just `>>`) so all delimiter mismatches are validated.
- Fix the `Display` implementation to correctly interpolate the `expected`/`found` values—Rust’s `write!` macros don’t support `{expected}`-style interpolation directly.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}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/parser/ast/parse_utils.rs (1)
123-163: Handle delimiter errors in all callers ofparse_name_type_pairsThe RG results show two call sites still using
.0, dropping theerrorsvector:• src/parser/mod.rs (two occurrences)
parse_name_type_pairs(self.syntax.children_with_tokens()).0Action items:
- Replace both
.0accesses with tuple destructuring:let (pairs, errors) = parse_name_type_pairs(self.syntax.children_with_tokens()); // TODO: propagate or log `errors` as appropriate- Ensure any non-empty
errorsare handled (e.g., bubbled up, logged, or converted to diagnostics).Tests in
parse_utils.rsalready destructure and assert onerrors, so no changes are needed there.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/parser/ast/parse_utils.rs(9 hunks)src/parser/mod.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.rs`: Comment why, not what. Explain assumptions, edge cases, trade-offs, o...
**/*.rs: Comment why, not what. Explain assumptions, edge cases, trade-offs, or complexity. Don't echo the obvious.
Comments must use en-GB-oxendict spelling and grammar.
Function documentation must include clear examples.
Name things precisely. Use clear, descriptive variable and function names. For booleans, prefer names with is, has, or should.
Each file should encapsulate a coherent module. Group related code (e.g., models + utilities + fixtures) close together.
Group by feature, not layer. Colocate views, logic, fixtures, and helpers related to a domain concept rather than splitting by type.
Every module must begin with a module level (//! ) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
Do not use return in single-line functions.
Prefer immutable data and avoid unnecessary mut bindings.
Handle errors with the Result type instead of panicking where feasible.
Avoid unsafe code unless absolutely necessary and document any usage clearly.
Clippy warnings MUST be disallowed.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Prefer expect over allow.
Use predicate functions for conditional criteria with more than two branches.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider using Arc to reduce the amount of data returned.
Prefer .expect() over .unwrap().
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
src/parser/mod.rssrc/parser/ast/parse_utils.rs
`**/*.rs`: * Seek to keep the cyclomatic complexity of functions no more than 12...
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments must use en-GB-oxendict spelling and grammar.
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/parser/mod.rssrc/parser/ast/parse_utils.rs
🧬 Code Graph Analysis (1)
src/parser/mod.rs (1)
src/parser/ast/parse_utils.rs (1)
parse_name_type_pairs(123-163)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (3)
src/parser/ast/parse_utils.rs (3)
26-31: LGTM! Well-structured error type.The
DelimiterErrorstruct appropriately captures all necessary information for delimiter mismatch errors. The use ofCopytrait is suitable for this small error type.
33-55: Display implementation provides clear error messages.The error message format effectively communicates delimiter mismatches. The mapping from internal
Delimenum to string representation is comprehensive.
187-223: Robust error detection for delimiter mismatches.The error detection logic correctly identifies when fewer delimiters are closed than expected, creating appropriate error objects with precise location information. The pattern is consistently applied across all delimiter types.
Docstrings generation was requested by @leynos. * #55 (comment) The following files were modified: * `src/parser/ast/parse_utils.rs` * `src/parser/mod.rs`
|
Note Generated docstrings for this pull request at #56 |
#56) * 📝 Add docstrings to `codex/implement-delimitererror-in-parse_utils.rs` Docstrings generation was requested by @leynos. * #55 (comment) The following files were modified: * `src/parser/ast/parse_utils.rs` * `src/parser/mod.rs` * Better example Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> * Add doc for DelimiterError * Fix syntax error --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Leynos <leynos@troubledskies.net> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
1 participant
Summary
DelimiterErrorfor name/type parsingTesting
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_68687ee043a083228378d71300497c4f
Summary by Sourcery
Detect and report unmatched delimiters in name/type pair parsing by introducing a DelimiterError type and extending parse_name_type_pairs to collect errors alongside parsed pairs while preserving existing API behavior.
New Features:
Enhancements:
Tests: