feat: Go structural interface matching and C# implements disambiguation (4.3)#522
feat: Go structural interface matching and C# implements disambiguation (4.3)#522carlos-alm merged 4 commits intomainfrom
Conversation
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewing PR #522: Go structural interface matching and C# implements disambiguation (4.3) Review Checklist
Root Cause AnalysisProblem Being Solved: This PR completes roadmap item 4.3 by filling two extractor-level gaps in interface/trait implementation tracking. The CLI commands, MCP tools, and impact analysis integration were already implemented, but Go structural interface matching and C# implements disambiguation were missing at the extractor level. Why This Approach: The implementation correctly addresses language-specific challenges:
Risk Assessment: Low risk. This is additive functionality with comprehensive test coverage, no breaking changes, and proper error handling. Backlog Compliance
Technical ReviewGo Extractor (
|
Greptile SummaryThis PR completes roadmap item 4.3 by filling the two remaining extractor-level gaps for Go structural interface matching and C# Go structural matching (
C# base-type disambiguation (
Minor observations
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Go Extractor
A[walkGoNode] --> B[handleGoTypeDecl]
B -->|struct| C[ctx.definitions: kind=struct]
B -->|interface| D[ctx.definitions: kind=interface\n+ interface methods as kind=method]
E[handleGoMethodDecl] -->|strips * from pointer receiver| F[ctx.definitions: kind=method\nname=ReceiverType.MethodName]
A --> E
G[matchGoStructuralInterfaces] -->|collects interface methods| H{ifaceMethods.size > 0?}
H -->|yes| I{struct methods ⊇ interface methods?}
I -->|yes - name-only match| J[ctx.classes.push\nname=struct, implements=iface]
I -->|no| K[skip]
H -->|no - empty interface| K
C --> G
D --> G
F --> G
end
subgraph C# Extractor
L[walkCSharpNode] --> M[handleCsClassDecl\nhandleCsStructDecl\nhandleCsRecordDecl]
M --> N[extractCSharpBaseTypes]
N -->|childForFieldName bases OR\nfindChild base_list| O{base_list found?}
O -->|yes| P[ctx.classes.push\nname=class, extends=baseName]
O -->|no| Q[return]
L --> R[handleCsInterfaceDecl]
R --> S[ctx.definitions: kind=interface]
T[reclassifyCSharpImplements] -->|builds interfaceNames from ctx.definitions| U{cls.extends in interfaceNames?}
U -->|yes - same-file interface| V[mutate: cls.implements = cls.extends\ndelete cls.extends]
U -->|no - cross-file or base class| W[keep as extends]
P --> T
S --> T
end
Last reviewed commit: "fix: add TODO tracki..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const [structName, methods] of structMethods) { | ||
| for (const [ifaceName, ifaceMethods] of interfaceMethods) { | ||
| if (ifaceMethods.size > 0 && [...ifaceMethods].every((m) => methods.has(m))) { | ||
| ctx.classes.push({ | ||
| name: structName, | ||
| implements: ifaceName, | ||
| line: structLines.get(structName) || 1, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The structural matching only compares method names — not signatures. In Go, a method set is defined by both the method name and its signature, so this can generate incorrect implements edges. For example, if a struct has Write(string) error and an interface requires Write([]byte) (int, error), the code will report the struct as an implementor even though the signatures differ.
For the file-local use case this is a pragmatic trade-off, but it's worth an explicit note in the JSDoc that the check is name-only (not signature-verified), so downstream consumers of the implements edges know to treat them as candidates rather than definitive matches.
// Match: struct satisfies interface if it has all interface methods (name-only;
// signatures are not verified — use this as a candidate match, not a definitive one)
for (const [structName, methods] of structMethods) {There was a problem hiding this comment.
Fixed — added comment noting that matching is name-only (no signature verification) and that results should be treated as candidate matches, not definitive.
| for (const def of ctx.definitions) { | ||
| if (def.kind !== 'method' || !def.name.includes('.')) continue; | ||
| const dotIdx = def.name.indexOf('.'); | ||
| const receiver = def.name.slice(0, dotIdx); | ||
| const method = def.name.slice(dotIdx + 1); | ||
|
|
||
| if (interfaceNames.has(receiver)) { | ||
| if (!interfaceMethods.has(receiver)) interfaceMethods.set(receiver, new Set()); | ||
| interfaceMethods.get(receiver).add(method); | ||
| } | ||
| if (structNames.has(receiver)) { | ||
| if (!structMethods.has(receiver)) structMethods.set(receiver, new Set()); | ||
| structMethods.get(receiver).add(method); | ||
| } |
There was a problem hiding this comment.
When a Go interface embeds another interface (e.g. type ReadWriter interface { Reader; Writer }), tree-sitter represents the embedded names as type_elem nodes, not method_elem nodes. The extractor only collects method_elem children for interface methods (see handleGoTypeDecl), so embedded interface names are never added to interfaceMethods.
As a result, ReadWriter would end up with ifaceMethods.size === 0 and be treated like an empty interface — skipped silently. A struct that implements Read() and Write() would not be matched to ReadWriter, even though it should be.
This is an edge case that could confuse users of the implementations command for composite interfaces. Consider adding a note in the JSDoc or a TODO tracking this gap.
There was a problem hiding this comment.
Fixed — added inline note documenting the embedded interface (type_elem) limitation. Composite interfaces like ReadWriter will be silently excluded from matching until this is addressed.
| file: 'Test.cs', | ||
| // Known native gap: native engine doesn't extract base_list into classes array | ||
| skip: true, |
There was a problem hiding this comment.
Marking this test skip: true is documented as a known gap (native engine doesn't extract base_list into classes array), which is a fair explanation. However, there's no mechanism to surface when the native engine eventually fixes this gap — the skip will silently remain even after the Rust engine is updated.
Consider replacing the bare skip with a tracked comment or a lightweight assertion so the test can be re-enabled without forgetting:
| file: 'Test.cs', | |
| // Known native gap: native engine doesn't extract base_list into classes array | |
| skip: true, | |
| // TODO(#NNN): re-enable once the native engine extracts base_list | |
| // (track at https://...) | |
| skip: true, |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Acknowledged — the skip already has a descriptive comment explaining the gap. A tracking issue will be filed separately for the native engine base_list extraction.
…on (4.3)
Complete the extractor-level gaps in interface/trait implementation tracking.
The query layer, CLI commands, MCP tools, impact analysis integration, and
presentation were already in place — this fills the two remaining extractor gaps.
Go extractor: add post-walk structural matching that compares struct method
sets against interface method sets within the same file, pushing implements
entries to the classes array. Empty interfaces are excluded.
C# extractor: fix extractCSharpBaseTypes to find base_list nodes (was using
childForFieldName('bases') which returns null in tree-sitter-c-sharp). Add
reclassifyCSharpImplements post-walk pass that reclassifies extends entries
as implements when the target is a known same-file interface definition.
Type definitions: add ImplementationsResult, InterfacesResult, and
Repository.findImplementors/findInterfaces to src/types.ts.
Tests: 8 new parser tests (Go structural matching, C# implements
disambiguation). Skip C# engine parity case (known native gap: native
engine does not extract base_list).
Impact: 15 functions changed, 6 affected
carlos-alm
force-pushed
the
feat/impl-tracking-4.3
branch
from
b3da2d9 to
51d99b3
Compare
…go extractor) Impact: 1 functions changed, 1 affected
1 participant
Summary
Completes roadmap item 4.3 — Interface and Trait Implementation Tracking by filling the two remaining extractor-level gaps. The query layer, CLI commands (
implementations,interfaces), MCP tools, impact analysis integration (bfsTransitiveCallerswithincludeImplementors), presentation formatting, and integration tests were already in place.Go extractor — structural interface matching:
{ name, implements, line }entries to theclassesarrayC# extractor — implements disambiguation:
extractCSharpBaseTypes:childForFieldName('bases')returns null in tree-sitter-c-sharp; addedfindChild(node, 'base_list')fallbackreclassifyCSharpImplementspost-walk pass that reclassifiesextendsentries asimplementswhen the target is a known same-file interface definitionType definitions:
ImplementationsResult,InterfacesResulttosrc/types.tsfindImplementors,findInterfacestoRepositoryinterfaceimplementations,interfacestoMcpDefaultsTest plan
base_list)implementationsData,interfacesData,contextData,fnImpactData)