feat: unify record access syntax - #25
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe parser replaces ChangesRecord Access Syntax
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant ExpressionText
participant src/parser.c
participant record_access
participant CorpusFixtures
ExpressionText->>src/parser.c: Parse .name, .0, ^.name, or ^.0
src/parser.c->>record_access: Build record_access nodes
record_access->>CorpusFixtures: Match expected parse trees
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/corpus/references.txt (1)
177-199: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a fixture for a bare
^with no selector.
record_accessusesrepeat1, so^alone must not parse as a valid node. No fixture pins that rule. A future change tooptionalorrepeatwould pass all current tests.Add the case to
test/corpus/invalid.txt.🤖 Prompt for 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. In `@test/corpus/references.txt` around lines 177 - 199, Add an invalid-parser fixture for a bare `^` with no selector in test/corpus/invalid.txt, ensuring it is rejected rather than producing a record_access node. Use the existing invalid fixture format and keep the record_access selector requirement enforced.test/corpus/compound_values.txt (1)
32-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the
root:andfield:labels to these expected trees.These fixtures assert node shape only. They do not assert the field mapping.
test/corpus/references.txtusesroot:andfield:in the equivalent cases.The field map is the part of
src/parser.cmost likely to regress after a regeneration. Labelled fixtures catch that regression.♻️ Proposed fixture update
- (record_access - (original_input) - (record_field_selector (named_record_field))) - (record_access - (original_input) - (record_field_selector (positional_record_field))))))) + (record_access + root: (original_input) + field: (record_field_selector (named_record_field))) + (record_access + root: (original_input) + field: (record_field_selector (positional_record_field)))))))- (record_access - (original_input) - (record_field_selector (positional_record_field)))) + (record_access + root: (original_input) + field: (record_field_selector (positional_record_field))))Also applies to: 89-91
🤖 Prompt for 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. In `@test/corpus/compound_values.txt` around lines 32 - 37, Update the expected trees in the compound-values fixtures, including the equivalent cases around the additional referenced lines, to add root: labels for the original_input nodes and field: labels for the corresponding record_field_selector nodes. Match the labeling convention used in references.txt so the fixtures validate field mapping as well as node shape.
🤖 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 `@grammar.js`:
- Around line 110-113: Update record_access in grammar.js so selector
punctuation and fields use immediate tokens, rejecting whitespace within record
references including after original_input; then regenerate src/grammar.json,
src/node-types.json, and src/parser.c. Add invalid.txt fixtures at lines 349-381
covering `. name`, `.customer . address`, and `^ .name`, documenting the
intended rejection behavior.
- Around line 110-124: Align the record-field naming rules by updating the
relevant unquoted declaration pattern to match named_record_field, including
alphanumeric names such as a1, or explicitly preserve the distinction and
document it with corpus cases covering supported quoted/unquoted forms and
rejected unquoted names. Keep record_access and positional_record_field behavior
unchanged.
---
Nitpick comments:
In `@test/corpus/compound_values.txt`:
- Around line 32-37: Update the expected trees in the compound-values fixtures,
including the equivalent cases around the additional referenced lines, to add
root: labels for the original_input nodes and field: labels for the
corresponding record_field_selector nodes. Match the labeling convention used in
references.txt so the fixtures validate field mapping as well as node shape.
In `@test/corpus/references.txt`:
- Around line 177-199: Add an invalid-parser fixture for a bare `^` with no
selector in test/corpus/invalid.txt, ensuring it is rejected rather than
producing a record_access node. Use the existing invalid fixture format and keep
the record_access selector requirement enforced.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 455ad608-b613-4aac-9ef7-fe63f736b255
📒 Files selected for processing (8)
README.mdgrammar.jssrc/grammar.jsonsrc/node-types.jsonsrc/parser.ctest/corpus/compound_values.txttest/corpus/invalid.txttest/corpus/references.txt
Summary
[name]/[0]references with unified record access nodes.name/.0and original-input^.name/^.0$n/$^nas distinct ordered-element accessValidation
npx tree-sitter test --file-name references.txt(13/13 passed)npx tree-sitter test --file-name compound_values.txt(18/18 passed)\rinstead of\ndotnet test Expressif.Syntax.sln --nologo(37/37 passed on each of .NET 8, .NET 9, and .NET 10)Close #18
Summary by CodeRabbit
New Features
.nameand.0.^navigation to access fields from the original input record.Breaking Changes
[name]and[0], with dot-based access syntax.