Skip to content

fix(go-parser): key generic-type methods on base type, not "unknown"#126

Open
gadievron wants to merge 2 commits into
masterfrom
pr/parser-generic-anon-class-keys
Open

fix(go-parser): key generic-type methods on base type, not "unknown"#126
gadievron wants to merge 2 commits into
masterfrom
pr/parser-generic-anon-class-keys

Conversation

@gadievron

Copy link
Copy Markdown
Collaborator

Summary

The Go parser keys methods on generic types (func (s *Stack[T]) Push(...), func (p *Pair[K,V]) Key(...)) under a bogus class name unknown instead of the real type, because the receiver-type stringifier typeToString has no case for Go's generic-instantiation AST nodes (*ast.IndexExpr / *ast.IndexListExpr) and falls through to default: return "unknown".

Impact (all confirmed on this branch's base):

  • Wrong unit idm.go:unknown.Push instead of m.go:Stack.Push.
  • Silent data loss — distinct generic types' same-named methods (Stack[T].Len, Queue[T].Len) collapse to one id m.go:unknown.Len; the store (output.Functions[id] = info) overwrites, so one real unit is dropped.
  • Dropped call edges — generic call sites Gen[K,V]() (*ast.IndexListExpr) and generic method calls obj.M[T]() were not recovered by analyzeCallExpr.

Generic types are idiomatic Go since 1.18, so this hits modern codebases broadly.

Changes

  • extractor.go typeToString: add case *ast.IndexExpr and case *ast.IndexListExpr, each recursing into t.X so the class key is the bare base type (Stack, Pair).
  • callgraph.go analyzeCallExpr: unwrap a leading IndexExpr/IndexListExpr then reuse the existing Ident/SelectorExpr classification — fixes all four generic call shapes and recovers the receiver for generic method calls.
  • extractor.go duplicateIDWarning: emit an os.Stderr warning when a funcID collides on store, so any future class-key collapse fails loudly instead of silently overwriting. JSON output shape unchanged.

Tests

5 new Go tests: generic-receiver class keys (pointer/value/multi-param), the no-collision data-loss regression (Stack.Len + Queue.Len stay distinct), generic call recovery, and the dup-guard. go test ./... green; gofmt/go vet clean.

Scope & related work (please read before requesting "why not Zig/PHP too?")

This PR is Go-only by design and is self-contained — it touches only parsers/go/go_parser/ and has no cross-parser dependency. The Go parser uses the standard-library go/ast, whose node types are stable, so this lands cleanly on master.

The same class-key-collapse family also affects the Zig and PHP parsers. Those fixes are in a separate PR that depends on the parser-rewrite PRs #110 (Zig) and #111 (PHP) and so targets the staging/parser-fix-stack integration branch, not master. This Go PR is independent of all of that — go/ast node types are stable — so do not block it on them.

Notes

  • For the class key the [T] type arguments are intentionally dropped (we want Stack, not Stack[T]); the receiver string is a display field, not used for resolution.
  • Known pre-existing, out-of-scope edges (not introduced here): a slice/map-index call funcs[i]() reports the container identifier as the call name; a parenthesized receiver func ((T)) M() still hits default (now observable via the dup-guard).

typeToString had no case for *ast.IndexExpr / *ast.IndexListExpr, so methods on
generic types (func (s *Stack[T]) Push()) keyed under class "unknown". Distinct
generic types' same-named methods then collided onto one unit id and the store
silently overwrote one (data loss). Generic call sites (Gen[K,V](), obj.M[T]())
were likewise dropped from the call graph.

- extractor.go: add IndexExpr/IndexListExpr cases to typeToString (bare base type)
- callgraph.go: unwrap generic instantiation in analyzeCallExpr; reuse the
  Ident/SelectorExpr classification (fixes all generic call shapes + receiver)
- extractor.go: add duplicateIDWarning() - stderr signal on funcID collision so a
  future class-key collapse fails loudly instead of silently overwriting

Tests: 5 new (generic receiver keys, no-collision data-loss regression, generic
calls, dup-guard). go test ./... green; independent + judge verified, no regression.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The binary is listed in .gitignore but was tracked since the initial commit, so it
drifted stale on every source change (master's committed binary still emits
"unknown.Push"). Untrack it so it stays ignored; CI builds it on every run
(.github/workflows/test.yaml) and the dev pipeline rebuilds it when absent
(parsers/go/test_pipeline.py). Compile on demand — no committed artifact.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gadievron added a commit that referenced this pull request Jun 17, 2026
The binary is in .gitignore but was tracked since the initial commit, drifting stale on
every source change. Untrack it so it stays ignored; CI builds it every run and the dev
pipeline rebuilds when absent. Keeps the stack consistent with the same cleanup on the
go-parser PRs (#109, #126) — compile on demand, no committed artifact.

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

1 participant