feat(native): port Solidity extractor to Rust#1100
Conversation
Adds tree-sitter-solidity dependency and native extractor matching the WASM-side behavior for Solidity symbol, import, and call extraction. Part of #1071
|
Claude finished @carlos-alm's task —— View job PR Review: Solidity Native Extractor PortReviewing PR #1100 following structured analysis phases: Todo List:
Phase 1: Root Cause Analysis ✓Problem Being Solved: Adding native Rust support for Solidity parsing to achieve dual-engine parity. Currently Solidity only works in WASM mode - this PR ports the extractor to the native engine so both engines can parse Why This Approach: Follows the established dual-engine architecture pattern used by all other supported languages. The implementation mirrors WASM-side behavior and explicitly notes where it matches JS extractor quirks (e.g., struct/enum members extraction). Risk Assessment: Low risk - purely additive feature following established patterns. Phase 2: Configuration Change Detection ✅ CLEANNo concerning configuration relaxations found:
Phase 3: Test Integrity ✅ CLEANNo weakened assertions or test integrity issues:
Phase 4: Code Quality ✅ HIGH QUALITYExcellent implementation following all established patterns: Extractor Completeness (
Architectural Consistency:
Registry Integration (all required updates present):
Test Coverage: 9 comprehensive unit tests covering all symbol types and edge cases. Phase 5: Backlog ComplianceZero-dep: ✓ — Single new tree-sitter grammar dependency (standard pattern for new language support) Foundation-aligned: ✓ — Fully compliant with all principles:
Problem-fit: 4/5 — Directly improves agent accuracy by eliminating a significant language gap. Solidity is critical for web3/DeFi codebases, and dual-engine parity prevents hallucinated dependencies from engine inconsistencies. Breaking: No — Purely additive, no API changes, maintains backward compatibility Tier: Tier 1 (zero-dep ✓ + foundation-aligned ✓) Final Assessment |
Greptile SummaryThis PR ports the Solidity extractor from the WASM-side TypeScript implementation to the native Rust engine, wires
Confidence Score: 5/5Safe to merge — the Rust extractor faithfully mirrors the TS extractor, all registry wiring is complete and exhaustive, and both sides of the multi-parent inheritance fix are covered by new tests. The port is a faithful translation of the TS extractor into Rust with no divergence in logic. The multi-parent inheritance fix is correctly applied to both the Rust and TS implementations. All match arms, registry entries, file-extension lists, and AST-type maps are consistent across the codebase. Nine new Rust unit tests plus the updated TS suite give good coverage. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[".sol file input"] --> B[file_collector\nchecks SUPPORTED_EXTENSIONS]
B --> C[parser_registry\nLanguageKind::Solidity\ntree-sitter-solidity 1.2]
C --> D[parse → Tree]
D --> E[extract_symbols_with_opts\nextractors/mod.rs dispatch]
E --> F[SolidityExtractor.extract]
F --> G[walk_tree\nmatch_solidity_node]
F --> H[walk_ast_nodes_with_config\nSOLIDITY_AST_CONFIG]
G --> I1[FileSymbols.definitions]
G --> I2[FileSymbols.classes]
G --> I3[FileSymbols.imports]
G --> I4[FileSymbols.calls]
H --> I5[FileSymbols.ast_nodes]
I1 & I2 & I3 & I4 & I5 --> J[FileSymbols output]
Reviews (5): Last reviewed commit: "fix: resolve merge conflicts with main" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| symbols.definitions.push(Definition { | ||
| name: full_name, | ||
| kind: "enum".to_string(), | ||
| line: start_line(node), | ||
| end_line: Some(end_line(node)), | ||
| decorators: None, | ||
| complexity: None, | ||
| cfg: None, | ||
| children: opt_children(members), | ||
| }); | ||
| } | ||
|
|
||
| // ── Functions / modifiers / events / errors / state vars ───────────────────── | ||
|
|
||
| fn handle_function_def(node: &Node, source: &[u8], symbols: &mut FileSymbols) { | ||
| let Some(name_node) = node.child_by_field_name("name") else { | ||
| return; |
There was a problem hiding this comment.
find_child returns only the first matching sibling, so for contract A is B, C {} only the B → A relationship is pushed to symbols.classes; the relationship C → A is silently dropped. The TS extractor has the same gap (both call findChild(node, 'inheritance_specifier')), so native and WASM are in parity — but neither handles the multiple-parent case. Neither the Rust unit tests nor the TS WASM suite includes a multi-parent contract, so this edge case has never been exercised. Consider iterating all inheritance_specifier siblings instead of stopping at the first, and adding a test like contract A is B, C {}.
There was a problem hiding this comment.
Fixed in 5513d49 — confirmed via tree-sitter-solidity's grammar.js (_class_heritage: "is" commaSep1($.inheritance_specifier)) that each parent in contract A is B, C, D {} is its own inheritance_specifier sibling under the contract node, so find_child/findChild returning only the first one was indeed dropping the rest. Both the native and WASM extractors now walk every direct child of the contract node and emit a ClassRelation for every parent, and there are new multi-parent unit tests on both sides (extracts_multi_parent_inheritance in Rust, extracts multi-parent inheritance in TS) to lock the behaviour in.
Codegraph Impact Analysis37 functions changed → 25 callers affected across 3 files
|
`extract_inheritance` / `extractInheritance` previously called
`find_child(node, 'inheritance_specifier')` and stopped at the first
match, but the tree-sitter-solidity grammar models each parent in
`contract A is B, C, D { }` as a separate `inheritance_specifier`
sibling under the contract node (`_class_heritage: "is"
commaSep1($.inheritance_specifier)`). All parents past the first were
silently dropped on both the native and WASM paths.
Walk every direct child of the contract node and emit a ClassRelation
for each parent on both engines, and add multi-parent unit tests in
Rust and TypeScript to lock the behaviour in.
|
Fixed in 5513d49 — confirmed via tree-sitter-solidity's |
1 participant
Summary
crates/codegraph-core/src/extractors/solidity.rs, mirroring WASM-side behavior for symbols, imports, inheritance, calls, modifiers, events, errors, and structs/enums.tree-sitter-solidity 1.2intoparser_registry,file_collector(.sol), the extractor dispatcher, and the AST-rules registry (new/throw/string kinds)..solnative-supported inNATIVE_SUPPORTED_EXTENSIONSand update the native-drop classification test (Solidity no longer drops).Part of #1071
Test plan
cargo build --release -p codegraph-core(clean compile)cargo test -p codegraph-core --lib— 193/193 pass, including 9 new Solidity extractor testsnpx vitest run tests/parsers/solidity.test.ts— 5/5 pass (WASM parity sanity)npx vitest run tests/parsers/native-drop-classification.test.ts— 13/13 passnpx vitest run tests/benchmarks/resolution/resolution-benchmark.test.ts -t solidity— 5/5 pass