fix(groovy): emit ClassRelation for interface inheritance in both engines#1158
fix(groovy): emit ClassRelation for interface inheritance in both engines#1158carlos-alm wants to merge 1 commit into
Conversation
…ines `interface X extends Y, Z` produced no class-graph edges because both extractors pushed only a Definition for the interface and never inspected the unnamed `extends_interfaces` child. The Rust handler now reuses `collect_interfaces` (which already recurses into `type_list`); the TS handler gains an equivalent helper that mirrors the same traversal. Closes #1115
|
Claude finished @carlos-alm's task —— View job Reviewing PR: fix(groovy): emit ClassRelation for interface inheritance in both enginesTodo List
Root Cause AnalysisProblem Being Solved: Why This Approach:
Risk Assessment: LOW - Purely additive change to extraction logic, no behavioral modifications to existing functionality. Backlog Compliance
Code Quality AssessmentImplementation Strengths
Technical AnalysisRust Implementation ( for i in 0..node.child_count() {
let Some(child) = node.child(i) else { continue };
if child.kind() == "extends_interfaces" {
collect_interfaces(&child, &iface_name, source, symbols);
break;
}
}✅ Efficient early termination with TypeScript Implementation ( for (let i = 0; i < node.childCount; i++) {
const child = node.child(i);
if (child && child.type === 'extends_interfaces') {
collectGroovyParentInterfaces(child, ifaceName, node.startPosition.row + 1, ctx);
break;
}
}✅ Mirrors Rust pattern exactly Minor Observations
Critical ConcernsNone found. This is a well-executed bug fix with proper testing and engine parity. Final Recommendation
Excellent work - this demonstrates deep understanding of both the tree-sitter grammar and the codegraph extraction pipeline. |
Greptile SummaryThis PR fixes a missing-edge bug in both the Rust and TypeScript Groovy extractors:
Confidence Score: 4/5Safe to merge; the change is well-scoped and the emitted relations are currently gated by a downstream filter that does not yet include interface, so there is no production impact from the minor TS/Rust line-number inconsistency. Both engines correctly identify parent interfaces and emit ClassRelation entries. The only divergence is that the TS helper always forwards the interface declaration's start line through recursive type_list calls, while the Rust helper re-evaluates the current node's line at each recursion level. src/extractors/groovy.ts — the collectGroovyParentInterfaces line-forwarding behavior should be aligned with the Rust engine before interface is added to HIERARCHY_SOURCE_KINDS. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[interface_declaration node] --> B{Find extends_interfaces child}
B -- found --> C[collect_interfaces / collectGroovyParentInterfaces]
B -- not found --> D[No ClassRelation emitted]
C --> E{Child node type}
E -- type_identifier / identifier / scoped_type_identifier --> F[emit ClassRelation]
E -- generic_type --> G[extract first child text, emit ClassRelation]
E -- type_list --> H[recurse into type_list]
H --> E
Reviews (1): Last reviewed commit: "fix(groovy): emit ClassRelation for inte..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function collectGroovyParentInterfaces( | ||
| parent: TreeSitterNode, | ||
| name: string, | ||
| line: number, | ||
| ctx: ExtractorOutput, | ||
| ): void { | ||
| for (let i = 0; i < parent.childCount; i++) { | ||
| const child = parent.child(i); | ||
| if (!child) continue; | ||
| switch (child.type) { | ||
| case 'type_identifier': | ||
| case 'identifier': | ||
| case 'scoped_type_identifier': { | ||
| ctx.classes.push({ name, implements: child.text, line }); | ||
| break; | ||
| } | ||
| case 'generic_type': { | ||
| const inner = child.child(0)?.text; | ||
| if (inner) ctx.classes.push({ name, implements: inner, line }); | ||
| break; | ||
| } | ||
| case 'type_list': | ||
| collectGroovyParentInterfaces(child, name, line, ctx); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
collectGroovyParentInterfaces receives line once (the interface declaration's startPosition.row + 1) and forwards the same value through every recursive call into type_list children. The Rust collect_interfaces instead evaluates start_line(interfaces) at each recursive invocation, so it tracks the actual start line of whatever node (extends_interfaces → type_list) is being processed at that moment.
For a multi-line interface — e.g. interface X\n extends Y, Z {} — the Rust engine would emit line 2 for the relation, while the TS engine always emits line 1. This is currently benign (the downstream HIERARCHY_SOURCE_KINDS filter doesn't include interface), but it means the two engines will produce divergent ClassRelation.line values once that filter is extended.
Codegraph Impact Analysis4 functions changed → 4 callers affected across 2 files
|
1 participant
Summary
interface X extends Y, Zproduced no class-graph edges because both Groovy extractors pushed a singleDefinitionfor the interface but never inspected the unnamedextends_interfaceschild.collect_interfaceshelper (which already recurses intotype_list); the TS handler gains an equivalentcollectGroovyParentInterfaceshelper that mirrors the same traversal.ClassRelation { name: X, implements: Y, line }for each parent interface — consistent with the existingcollect_interfaceshelper, with class-implements behavior, and with the downstreamIMPLEMENTS_TARGET_KINDSfilter that already accepts interface targets.Notes
tree-sitter-groovy0.1.x exposes parent interfaces as an unnamedextends_interfaceschild ofinterface_declaration(not a field), distinct from class declarations which use theinterfacesfield that wrapssuper_interfaces → type_list.HIERARCHY_SOURCE_KINDSset insrc/domain/graph/builder/stages/build-edges.tsdoes not currently includeinterface, so emitted relations still won't materialize as graph edges until that filter is extended — that's a broader cross-language change worth tracking separately.Closes #1115
Test plan
cargo test -p codegraph-core --lib extractors::groovy— newextracts_interface_inheritancetest passes alongside existing 8 testsnpx vitest run tests/parsers/groovy.test.ts— newextracts interface inheritance (extends_interfaces)test passes alongside existing 6 testsnpx biome check src/extractors/groovy.ts tests/parsers/groovy.test.ts— clean