Skip to content

fix(go-extractor): attach methods on generic receivers and handle grouped type declarations#445

Open
tirth8205 wants to merge 3 commits into
Egonex-AI:mainfrom
tirth8205:fix/go-extractor-generics-grouped-types
Open

fix(go-extractor): attach methods on generic receivers and handle grouped type declarations#445
tirth8205 wants to merge 3 commits into
Egonex-AI:mainfrom
tirth8205:fix/go-extractor-generics-grouped-types

Conversation

@tirth8205

Copy link
Copy Markdown
Contributor

Problem

Two distinct gaps in the Go extractor caused real-world Go constructs to be silently dropped from the structural graph:

  1. Methods on generic receivers were never attached to their struct. extractReceiverType only handled a bare type_identifier receiver or a pointer_type directly wrapping a type_identifier. For a generic type the receiver is a generic_type node — func (s Stack[T]) ... parses as parameter_declaration -> generic_type -> {type_identifier 'Stack', type_arguments} and func (s *Stack[T]) ... as pointer_type -> generic_type -> type_identifier. Neither matched, so the function returned undefined, nothing was recorded in methodsByReceiver, and the struct's methods array stayed empty. A Stack[T] struct with Push/Len methods produced classes[0].methods === [].

  2. Grouped type declarations dropped every type after the first. extractTypeDeclaration used findChild(node, "type_spec"), which returns only the first type_spec. Go allows grouping types under one type ( ... ) block, which parses as one type_declaration with several type_spec children. So type ( Foo struct{...}; Bar struct{...} ) produced classes: ['Foo'] and exports: ['Foo'], with Bar missing entirely.

Fix

  • extractReceiverType: iteratively unwrap pointer_type and generic_type layers (descending generic_type via its type field) before grabbing the base type_identifier. Non-generic receivers are unaffected.
  • extractTypeDeclaration: iterate over all type_spec children via findChildren (already imported), and pass each typeSpec as the decl node so line ranges are per-type rather than the whole group.

Both changes are minimal and localized to go-extractor.ts.

Testing

Added two tests to go-extractor.test.ts:

  • attaches methods on generic receivers to their struct — a Stack[T] struct with *Stack[T] and Stack[T] receivers; expects classes[0].methods to contain Push and Len.
  • handles grouped type declarations — a type ( Foo struct{...}; Bar struct{...} ) block; expects classes to be ['Foo','Bar'] and exports to contain both.

Both tests fail before the fix (methods is []; only Foo is captured) and pass after. The full core Vitest suite is green (694 tests), tsc --noEmit exits 0, and ESLint passes on both changed files.

🤖 Generated with Claude Code

…uped type declarations

extractReceiverType only unwrapped a bare type_identifier or a
pointer_type directly wrapping one. Generic receivers like
`(s *Stack[T])` or `(s Stack[T])` are parsed as generic_type nodes
(optionally pointer-wrapped), so the base type name was never found
and the method was never attached to its struct (methods stayed []).
The fix iteratively unwraps pointer_type and generic_type layers
before grabbing the base type_identifier.

extractTypeDeclaration used findChild(node, "type_spec"), which only
returns the first type_spec. Grouped declarations
(`type ( Foo struct{...}; Bar struct{...} )`) contain multiple
type_spec children, so every type after the first was silently
dropped from classes and exports. The fix iterates over all type_spec
children via findChildren and passes each typeSpec as the decl node
for accurate per-type line ranges.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@thejesh23 thejesh23 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. Grouped type aliases / non-struct-non-interface specs are silently dropped.
extractTypeDeclaration now iterates all type_spec children but still only branches on struct_type / interface_type. A type ( Foo struct{...}; MyID = string; Count int ) block records Foo and drops MyID and Count without any signal — the same silent-drop pattern the PR is fixing for grouped structs. Worth either handling type aliases/named primitives or at least a comment noting the gap.

2. Test coverage for grouped declarations is shallow.
Only an all-struct group is exercised. A grouped block mixing struct and interface (e.g. type ( Foo struct{...}; Bar interface{...} )), or two grouped interfaces, would catch regressions in the interface branch of the new loop — currently nothing pins extractInterface(typeSpec, ...) against the per-spec node change.

3. Multi-type-parameter and constrained generic receivers are untested.
The new test covers Stack[T] only. func (s *Box[K, V]) ... and func (s Result[T comparable]) ... are common Go 1.18+ patterns; both should fall out of the same generic_type -> type field traversal, but without a test there's nothing guarding it. (Same class of issue you'll hit in #435 for Dart generic type parameters on methods — worth keeping the receiver-unwrap pattern consistent across extractors.)

Nit: extractTypeDeclaration now passes typeSpec (not the outer type_declaration) as declNode, so for non-grouped type Foo struct{...} the recorded lineRange[0] is the name line rather than the type keyword line. Existing tests happen to pass because type and the name are on the same line, but the semantics shifted silently.

…er cases

Add tests pinning the interface branch inside grouped type blocks
(mixed struct+interface and two-interface groups) and multi-parameter /
constrained generic method receivers (Box[K, V], Result[T comparable]).
Document in extractTypeDeclaration that non-struct/non-interface specs
(named-primitive type_spec and type_alias nodes) are intentionally not
modeled as classes, and that the per-spec declNode lets grouped types
report their own line ranges.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tirth8205

Copy link
Copy Markdown
Contributor Author

1. Grouped non-struct/non-interface specs. You're right that type ( Count int ) (a type_spec whose type field is a type_identifier) and type ( MyID = string ) (parsed as a separate type_alias node, never visited by the findChildren(node, "type_spec") loop) fall through without a signal. This is the same silent-drop class the PR fixes for grouped structs, but it's also a pre-existing gap — the original code only ever branched on struct_type/interface_type, so nothing regresses here. Whether aliases and named/defined types should be modeled as graph nodes at all is a broader type-model question, so per your suggested minimal alternative I've added an explicit comment in extractTypeDeclaration documenting that these forms are intentionally not captured as classes (and which grammar node each maps to), leaving alias modeling for a follow-up.

2. Shallow grouped-declaration coverage. Added two tests: a grouped block mixing struct + interface (type ( Foo struct{ A int }; Bar interface { Read() error } )) asserting Foo.properties == ["A"] / Foo.methods == [] and Bar.methods contains Read / Bar.properties == [], plus a two-interface group asserting both Read and Write resolve. These pin extractInterface(typeSpec, ...) against the new per-spec declNode argument inside a group.

3. Multi-param / constrained generic receivers. Added tests for func (b *Box[K, V]) ... and func (r Result[T comparable]) ..., asserting the methods attach to Box and Result. As you anticipated, these already fall out of the existing generic_typetype field traversal (the type field is the base identifier; constraints live in the type arguments), so no production change was needed — the tests lock that behavior against future regressions.

Nit (declNode → type_spec). The shift to the per-spec node is deliberate and, for grouped declarations, more correct: each grouped type now reports its own line range instead of all sharing the opening type ( line. For non-grouped type Foo struct{...} the spec and the type keyword are on the same physical line, so the range is unchanged. I documented this in the extractTypeDeclaration comment rather than reverting.

Full core suite: 698 passed. tsc clean.

… exports

Previously only struct_type / interface_type specs were captured; exported
defined types (`type Count int`) and type aliases (`type MyID = string`, which
parse as a separate `type_alias` node) were silently dropped from `exports` —
both in grouped `type ( ... )` blocks and as standalone declarations. These are
public package symbols and now appear in `exports` (still not in `classes`,
since they have no fields/methods). Unexported (lowercase) names remain
excluded. Addresses review concern #1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tirth8205

Copy link
Copy Markdown
Contributor Author

Follow-up on concern 1 — promoted from documented-gap to an actual fix in a9eba94.

On a closer look the silent-drop was broader than the grouped block: exported defined types (type Count int) and type aliases (type MyID = string, which the grammar parses as a separate type_alias node, not a type_spec) were dropped from exports whether grouped or standalone — only struct_type/interface_type were ever surfaced. Both are public package symbols, so they now appear in exports (verified the type_alias name field and the defined-type type_spec shape against the real tree-sitter-go parse first). They're intentionally not added to classes since they have no fields/methods to model, and unexported (lowercase) names stay excluded. Added tests for standalone defined-type/alias export + a mixed grouped block (struct/interface/alias/defined). Full core suite green (700 passed); tsc --noEmit clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants