Add Dart support to codebase indexing#941
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughDart is added to tree-sitter’s supported extensions, parser loading, query exports, and definition extraction tests. A Dart fixture covers classes, methods, mixins, enums, extensions, typedefs, and top-level functions. ChangesDart tree-sitter support
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SourceFile
participant loadRequiredLanguageParsers
participant DartWasm
participant Query
participant DefinitionParser
SourceFile->>loadRequiredLanguageParsers: provide .dart extension
loadRequiredLanguageParsers->>DartWasm: load Dart grammar
loadRequiredLanguageParsers->>Query: construct query from dartQuery
Query-->>loadRequiredLanguageParsers: return Dart query
DefinitionParser->>Query: extract Dart definitions
Query-->>DefinitionParser: return captured declarations
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
edelauna
left a comment
There was a problem hiding this comment.
Thanks so much for this contribution, had some comments regarding the queries.
| it("should capture common Dart declarations", async () => { | ||
| const result = await testParseSourceCodeDefinitions("/test/file.dart", sampleDartContent, dartOptions) | ||
|
|
||
| expect(result).toMatch(/abstract class Animal/) |
There was a problem hiding this comment.
The assertions here match substrings of the echoed source line (the pipeline outputs lines[startLine], the raw source text). That means a mutation breaking every @name capture in the query would still pass all 19 assertions — the @definition.* captures alone are enough to echo the source line.
Sibling specs address this with two additions:
- The
\d+--\d+ \|line-range prefix, which at least confirms the output came through the pipeline in the right format:expect(result).toMatch(/\d+--\d+ \| abstract class Animal/) - A count assertion that catches dropped or duplicated captures:
expect(result!.split("\n").filter(l => l.includes(" | ")).length).toBe(N)
One thing I wasn't sure about: getMinComponentLines() defaults to 4, so single-line declarations like const Point() and int get x => 0 would normally be filtered by the lineCount < 4 check. Are assertions like toMatch(/const Point\(\)/) expected to pass, and if so through which code path?
| name: (identifier) @name)) @definition.method | ||
|
|
||
| (method_signature | ||
| (operator_signature)) @definition.method |
There was a problem hiding this comment.
The upstream tags.scm this is adapted from has a bare (method_signature) @definition.method catch-all (their line 34) that fires for any method_signature not matched by the more-specific rules above. Without it, abstract method declarations and any future grammar node types added to the Dart grammar would produce zero captures rather than a fallback @definition.method.
Was this intentionally omitted, or an oversight from the adaptation?
| (operator_signature)) @definition.method | ||
|
|
||
| (constructor_signature | ||
| name: (identifier) @name) @definition.method |
There was a problem hiding this comment.
There is also a (method_signature (constructor_signature ...)) pattern at line 29–31. For a constructor inside a class body, a method_signature wraps it, so both patterns could match the same constructor.
For the single-line constructors in the fixture this is fine — both nodes share the same startLine/endLine so the lineKey dedup in processCaptures suppresses the duplicate. But for a multi-line constructor in real Dart code, the two nodes have different line spans → different lineKey values → both get output, and the constructor could appear twice in the index.
Is there a case where a constructor_signature appears outside a method_signature that this standalone pattern is meant to catch?
e63a286 to
5f71794
Compare
Related GitHub Issue
Closes: #940
Description
Adds Dart support to Tree-sitter-based codebase indexing.
.dartas a supported source extension and loadstree-sitter-dart.wasm..gitignoreor.rooignorewhen desired.Reviewers should pay particular attention to the query patterns for constructors and top-level accessors, which differ structurally from class method signatures in the Dart grammar.
Test Procedure
Automated tests:
cd src npx vitest run services/tree-sitter/__tests__/languageParser.spec.ts services/tree-sitter/__tests__/parseSourceCodeDefinitions.dart.spec.tsExpected result: 2 test files and 8 tests pass.
The repository pre-commit and pre-push hooks were also run successfully:
Manual verification:
.dartfiles.Pre-Submission Checklist
Screenshots / Videos
Not applicable — this change has no UI impact.
Documentation Updates
Additional Notes
No generated-file suffixes such as
*.g.dartor*.freezed.dartare hard-coded into the exclusion logic. Existing.gitignoreand.rooignorebehavior remains the source of truth, preserving access to generated APIs when projects intentionally index them.Get in Touch
GitHub: @WebMad
Summary by CodeRabbit
New Features
.dart) support for source-code definition parsing.Tests